std::list, object pool, leaks and performance

[brianbathorycpp]

Hello everyone!..
I made a very simple Object pool exmaple using std::list. I think the implementation is fine and I'm almost positive that it doesn't generate any kind of leaks. However, I still have a couple of questions:

1. Over at line 116 I create a std::vector that will retrieve instances of the objects inside the object pool (list).
So, isn't it a bit redundant in terms of performance to create another list, or in this case another vector to retrieve the objects inside the pool?.. What I mean by this is that I don't really know if by creating this vector with empty unique pointers and THEN assigning them using std::move to objects from the pool is having the same performance impact as if I was just using new/delete straight up.

2. If I try to retrieve more objects than the list has my program crashes, not sure how to make it so that It simply states that there are no more available objects and that's it

CODE:

#include <iostream>
#include <memory>
#include <list>
#include <vector>

    /* This object has only an int data type (value).
       One method to se it and another one to print it. */
class Object
{
    public: 
        Object() = default;
        
        Object( int val )
        {
            mValue = val;
            std::cout << "Constructed." << std::endl;
        }

            // Copy constructor
        Object( Object& other )
        {
            this->mValue = other.mValue;
            std::cout << "Constructed." << std::endl;
        }

        ~Object()
        {
            std::cout << "Destroyed." << std::endl;
        }

        void SetValue( int val )
        {
            mValue = val;
        }

        void PrintValue()
        {
            std::cout << this->mValue << std::endl;
        }

    private:
        int mValue;

};

    // Object pool using std::list
class Pool
{
    public:
        Pool() = default;
        ~Pool() = default;

            /* Fills the pool (list) with a given size using an Object class (previously defined)
               as a reference using its copy constructor.*/
        void Fill( int size, Object* obj )
        {
            for( int i = 0; i < size; i++ )
            {
                std::unique_ptr<Object> object = std::make_unique<Object>( *obj );
                pool.push_back( std::move( object ) );
            }
        }

            // Clears all items in the list.
        void PoolIsClosed()
        {
            pool.clear();
        }

            // Returns an instance of an object within the list.
        std::unique_ptr<Object> GetObject()
        {
            if( !pool.empty() )
            {
                std::unique_ptr<Object> object = std::move( pool.front() );
                pool.pop_front();
                
                std::cout << "Object retrieved." << "\nObjects in pool: "
                          << pool.size() << std::endl;
                
                return object;
            }
            else
            {
                /* "WARNING: NOT ALL CONTROL PATHS RETURN A VALUE." */
                std::cout << "Pool is empty." << std::endl;
            }
        }

            // Returns an instance back to the object list.
        void returnToPool( std::unique_ptr<Object> returningObject )
        {
            pool.push_back( std::move( returningObject ) );
            
            std::cout << "Object returned." << "\nObjects in pool: "
                      << pool.size() << std::endl;
        }

    private:
        std::list<std::unique_ptr<Object>> pool;
};

int main()
{
        /* The object used as "reference"/"base" that will fill the object pool */
    std::unique_ptr<Object> mObj = std::make_unique<Object>();
    mObj->SetValue( 500 );

        /* Create an instance of Pool as unique pointer and fill it with 50 objects
           using mObj (previously defined) as reference/base. */
    std::unique_ptr<Pool> mPool = std::make_unique<Pool>();
    mPool->Fill( 50, mObj.get() );

        /* Vector of unique pointers that will retrieve from the object pool.
           ( I used vectors here because it's easier to access its members )*/
    std::vector<std::unique_ptr<Object>> moreObjects(10);
    
        /* Assign an object from the pool to a pointer in the vector and re-set its value
           the original 500 to i + 1 ( 1 - whatever the limit of the vector ). */
    for( size_t i = 0; i < moreObjects.size(); i++ )
    {
        moreObjects[i] = std::move( mPool->GetObject() );
        moreObjects[i]->SetValue( i + 1 );
    }

        /* Print the value of the objects in the vector.*/
    for( size_t i = 0; i < moreObjects.size(); i++ )
    {
        moreObjects[i]->PrintValue();
    }

        /* Return the objects from the vector back to the object pool. */
    for( size_t i = 0; i < moreObjects.size(); i++ )
    {
        mPool->returnToPool( std::move( moreObjects[i] ) );
    }

    std::cin.get();
    return 0;
}

I'm sorry this is such a long question but I'm just worried that any leaks may ocurr or the way I'm trying to implement the object pool is worthless.
Anyway, thanks a lot, really, for taking the time!..

[helios]

What exactly are you trying to achieve? The typical usage of object pools is to prevent allocations by recycling previously allocated objects. Using std::list defeats the purpose because pushing into the std::list necessarily involves allocating the node. Object pools are typically implemented using manually managed intrusive lists (i.e. you add a "next" member that serves as a singly-linked list link).

isn't it a bit redundant in terms of performance to create another list, or in this case another vector to retrieve the objects inside the pool?

It doesn't really matter. The object pool is an optimization of the allocator, but how you use the allocated pointer is completely independent. If the design of your program calls for storing the objects in a vector, then there's no much you can do, object pool or not. If you can redesign your program to avoid a vector, it might be worthwhile to do so, again object pool or not.

If I try to retrieve more objects than the list has my program crashes, not sure how to make it so that It simply states that there are no more available objects and that's it

An object pool should behave as a replacement for the allocator. If it runs out of objects to recycle it should just allocate a new object and return it immediately. The only reasons to return an error on an allocation request are if there's no more memory or if the allocator has hit a specified memory limit.

[brianbathorycpp]

Oh I see. I thought that by filling the list initially would take care of the initial allocations and thus, I wouldn't need to make more at all. But yeah, I wasn't sure if it was doing what it was supposed to because of how lists work. I think I might try to implement the pool as a vector and check if the item I need to retrieve is available... Maybe that's the way to go?

It doesn't really matter. The object pool is an optimization of the allocator, but how you use the allocated pointer is completely independent. If the design of your program calls for storing the objects in a vector, then there's no much you can do, object pool or not. If you can redesign your program to avoid a vector, it might be worthwhile to do so, again object pool or not.

Ok I understand now, it just felt kind of weird having to make another container
Thanks a lot for the input, I'll try to improve the code!

[brianbathorycpp]

So... I've been reading a book; Game Development Patterns and Best Practices and it seems that their implementation of an object pool is pretty much the same, lists and all. The book however, uses regular pointers instead of smart pointers... So, is the list not really the problem here or why is it that it defeats the purpose I don't really understand what's going on. To me it makes sense the way its implemented but maybe I'm overlooking something?

[helios]

The book's implementation is dumb. Nobody implements object pools with std::list because the point of an object pool is to prevent allocations when possible. By using std::list for a pool you're guaranteeing that every time you return an object to the pool you will be allocating memory for a node. In fact you end up increasing the total number of allocations, rather than decreasing them.

The simplified typical implementation is like this:

class Object{
    //...
    friend class ObjectPool;
    Object *next = nullptr;
    //...
};

class ObjectPool{
    Object *head = nullptr;
public:
    ~ObjectPool(){
        while (auto temp = this->head){
            this->head = temp->next;
            delete temp;
        }
    }
    Object *create(){
        if (!this->head)
            return new Object;
        auto ret = this->head;
        this->head = ret->next;
        ret->next = nullptr;
        return ret;
    }
    void release(Object *o){
        o->next = this->head;
        this->head = o;
    }
};

This is what's known as an intrusive list. It's called "intrusive" because the data object we want to store in the list has its structure modified to accommodate the list. An unintrusive list such as std::list<Object *> has a helper class node<Object *> that looks like this:

class node<Object *>{
    Object *data;
    node<Object *> *prev;
    node<Object *> *next;
};

and the list itself looks like this:

class std::list<Object *>{
    node<Object *> *head;
    node<Object *> *tail;
};

Obviously, anytime you want to insert an Object * anywhere into the list, a node<Object *> needs to be allocated, an a node<Object *> needs to be released every time you want to remove an Object * from anywhere in the list.

Conversely, all the unobtrusive list allocates is the Object instances themselves. It allocates only when absolutely necessary, and if the pool is used appropriately most of the time it doesn't allocate anything at all.

[brianbathorycpp]

Right!.. I understand what you're saying now. Pretty much every example I've found of object pools in c++ use std::list or even std::stack. I think they all run into the same issue you mention that whenever an items gets returned memory needs to be allocated for it. Also, I'm starting to understand a bit better with the example you gave me, I'm going to have to think about it a little bit more. In the meantime, I found this implementation of a pool ready to use:

https://github.com/mrDIMAS/SmartPool

It looks simple enough to use although I'm not quite sure how it works under the hood, but it looks pretty cool

[jonnin]

you can use list or any of the containers, if you mark the items deleted and never actually delete them. Then it comes down to how you need to access them vs memory fragmentation ideas.
I mostly use vector for this.

[helios]

That's not what OP is asking. OP is asking about minimizing allocations, not about how to avoid removing elements from the middle of containers.

[jonnin]

I understand that, but recycling items is a key piece to reducing allocations. A list is kinda cool, in that you can pull the deleted ones into a side-list just by arranging pointers instead of marking each node.

[helios]

Ah, I see what you mean. Why use a separate data structures that requires extra allocations when an intrusive list solves the same problem for cheaper, though?

[jonnin]

I was, I suppose, answering how you can use a list (or 'any') container to do the job, not suggesting a 'better' way. The OP is seeing code online that does these things using various containers, and my main point was the container type can be anything if you want to go there.
Your approach assumes it was already allocated (reasonable assumption). If you are designing it in up front, the alternate container can BE the only allocations done, though (not a new copy, of course, that is, as you said, of no value).
I don't know if there is a One and Only or whatever design pattern or not, where you avoid copying and recreating stuff from the onset.