Purpose
You never stated why you need an object pool, but I'm going to assume that it is for performance reasons, as it most often is. If you don't have a solid reason for using an object pool, you shouldn't. It just complicates things for no gain.
With that in mind the performance gain from an object pool is for two reasons:
- You have already pre-allocated all the objects in one big block, allocating objects from the pool thus bypasses new/delete (or malloc/free) and the risk of page allocations and all that fun stuff as we have already forced the page allocations on initialisation of the object pool. Hence allocation and de-allocation is typically faster but more importantly more consistent in time.
- All the objects are contiguous in memory. This reduces memory fragmentation and improves cache coherency somewhat if you access the objects frequently.
Your design with a trivial factory that calls new for each invocation will achieve the first benefit above, but not the second. To obtain the second benefit you would need to create non-trivial factory and deleter which to me seems like unnecessary work.
For this reason I believe that your overall design is poor as you can get both benefits for no additional hassle for the user with a better design.
Better design
To obtain all the performance gains we need to use a contiguous memory region for our objects. I'm going to show an untested example (without the blocking part in your implementation) of how you could do it:
template<typename T>
class ObjectPool{
private:
// This class makes sure we respect the alignment of T and that
// the ctor and dtor of vector don't call ctor and dtor of T
// as we have to decide when and how to do this ourselves.
class alignas(T) Object{
std::array<byte, sizeof(T)> data;
};
class Deleter{
private:
std::stack<size_t>& freeStack;
size_t index;
public:
Deleter(std::stack<size_t>& s, size_t i)
: freeStack(s), index(i)
{}
void operator () (T* p){
p->~(); // Call dtor
freeStack.push(index); // Do appropriate synchronisation here
}
}
public:
ObjectPool(size_t n)
: pool(n)
{
while(n > 0){
freeIndicies.push(n-1);
n--;
}
}
// Copy Ctor and assignment are not sensible for a pool.
ObjectPool(const ObjectPool&) = delete;
ObjectPool& operator = (const ObjectPool&) = delete;
~ObjectPool(){
if(freeIndecies.size() != pool.size()){
// Pool destroyed before all objects died!
}
// Otherwise no cleanup as each unique_ptr cleans up after themselves
}
template<typename... Args>
std::unique_ptr<Object, Deleter> alloc(Args&&... args){
if(freeIndicies.empty()){
// Handle no free objects
}
int index = freeIndicies.top();
freeIndicies.pop();
// Use placement new and forward all arguments to the constructor of T
auto object = new (&pool[index]) T(std::forward<Args>(args)...);
return std::unique_ptr<Object, Deleter>(object, Deleter(freeIndicies, index);
}
private:
std::stack<size_t> freeIndicies;
std::vector<T> pool;
};
The above simply creates a vector of raw arrays of appropriate size and alignment. This means that when the vector is created, no objects of type T have been constructed (i.e. you don't need a default constructor). It also means that upon destruction of the vector the destructors of T will not be called, this avoids double destruction as we make sure to manually call the destructor in the deleter of the unique_ptr.
In the deleter we also return the object to the pool without actually freeing the memory but we do let the object destruct properly (and release any memory it may hold internally). This is an improvement over the original code which would not properly destruct or construct the objects between uses (without releasing the memory).
We also use variadic templates to allow all constructors of T to be used for constructing the object directly which is much more powerful than the factory method you used.
I hope this helps!