1
\$\begingroup\$

I'm following Robert Nystroms example to try and implement a decent object pool for my next couple of C++ based games. My version is templatized to allow arbitrary classes to be pooled.

I'm asking for input on two things:

  1. The method names and interface design (both on the Pool and the iPoolable interface).

  2. A slightly more concrete smell is that I'm adding two methods to the public interface of all "poolable" classes. Is there a way to make sure the iPoolable interface is only accessible to the Pool?

Any other tips and input would of course be greatly appreciated as well!

(Here's all the code on pastebin, for proper syntax highlighting.)

#pragma once
#include <cassert>
#include <array>
#include "iPoolable.h"

//This class borrows liberally from Robert Nystroms excellent ObjectPool
    //http://gameprogrammingpatterns.com/object-pool.html

template<class T, size_t POOL_SIZE = 32>
class Pool{ 
    static_assert(std::is_base_of<iPoolable, T>::value, "Type must implement iPoolable!");
    mutable iPoolable* _head = nullptr; //linked list of unused objects
    std::array<T, POOL_SIZE> _objects;
public:
    Pool(){             
        _head = &_objects[0];               
        for(size_t i = 0; i < POOL_SIZE - 1; ++i){ // Each object points to the next.   
            _objects[i].setNext(&_objects[i + 1]);          
        }       
        _objects[POOL_SIZE - 1].setNext(nullptr); // The last one terminates the list.      
    }
    void recycle(T* object) const noexcept {
        assert(object != nullptr);
        object->onReturnToPool(); //notify the object
        object->setNext(_head); //add the object to front of the list
        _head = object; 
    }
    T* getNext() const noexcept{
        assert(_head != nullptr); // Make sure the pool isn't exhausted.                
        T* nextObject = static_cast<T*>(_head); //get the head of the list
        _head = nextObject->getNext();  // Update the head of the list  
        nextObject->onTakenFromPool(); //notify the object
        return nextObject;
    }       
    template<typename Function>
    void apply(Function f) {        
        std::for_each(_objects.begin(), _objects.end(), f);     
    }   
    ~Pool(){
        //give objects a chance to clean up before being destroyed. (onReturnToPool is noexcept!)
        apply(std::mem_fn(&T::onReturnToPool));
    }
    size_t static constexpr size() noexcept { return POOL_SIZE; }   
};

(Not happy with recycle() or getNext())

The Pool expects type T to implement the iPoolable interface.

#pragma once
class iPoolable{
public: 
    virtual iPoolable* getNext() const noexcept = 0;
    virtual void setNext(iPoolable* next) noexcept = 0; 

    //Optional: release resource handles, stop timers, clean up.
    virtual void onReturnToPool() noexcept {}; 

    //Optional: re-aquire resources, start timers, re-initialize members. 
    virtual void onTakenFromPool() noexcept {}; 

    virtual ~iPoolable(){};
};

Again I'm looking for input on the interface. setNext and getNext might be too generic and collide with members in the derived classes. onReturnToPool / onTakenFromPool are overly verbose. What should I call them instead?

Implementing the interface should be trivial - either inherit from the concrete class Poolable (at the cost of an additional pointer member):

#pragma once
#include "iPoolable.h"
class Poolable : public iPoolable{
    iPoolable* _next = nullptr;
public: 
    void setNext(iPoolable* next) noexcept override{
        _next = next;
    }
    iPoolable* getNext() const noexcept override{
        return _next;
    }       
    virtual ~Poolable(){
        _next = nullptr;
    }   
};

Or avoid the cost of an additional pointer by implementing iPoolable manually in your class, and stuffing the pointer in a union with members we are already using:

template<class T = float>
class Vector2D : public iPoolable{
public:     
    union{ //anymous union
        struct{
            T x;
            T y;
        }; //struct of 2xfloat = 8 bytes        
        iPoolable* _next; //pointer = 8 bytes
    }; //union total = 8 bytes
    iPoolable* getNext() const noexcept override { return _next; }
    void setNext(iPoolable* next) noexcept override { _next = next; }

/* ... rest of Vector2D interface ...*/
};
\$\endgroup\$
1
\$\begingroup\$

I think there are two major improvement you could do here.

You don't need the object to be set up as a linked list.

Consider that you could have the following:

// gets the array index at which an object resides.
std::size_t get_index(T* object) const noexcept {
  return object - &_objects[0];
}

And you could rebuild the class in a much simpler way around it, and it gets rid of iPoolable entirely! want a Pool<std::unique_ptr<int>>? no problem!

You are creating too many objects too soon.

std::array<T, POOL_SIZE> _objects;

This creates a bunch of problems:

  1. T must have a default constructor
  2. T's constructor get called for all objects as soon as the pool iscreated
  3. T's destructor doesn't ever get called.
  4. T basically can't use RAII semantics

the stl provides you with the super handy std::aligned_storage to deal with these situations, where you can in-place create and in-place delete object from safely.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Thanks for your input! I'm not sure that I understand your first point - the linked list allows me to find the first unused object in the pool, with O(1) lookup time. I see how your get_index work, but not how it provides an object that's ready to be (re)used? Point 3: the std::array will in fact call the dtor of all objects when the pool is destroyed. ... right? \$\endgroup\$ – ulfben Dec 1 '17 at 9:35
  • \$\begingroup\$ Point 4: The loss of RAII does bother me alot - my first attempt had a variadic forwarding method and allowed exceptions to escape, but like you say; the construction has already happened. So the user had to rely on setup/teardown methods for object (re-)initialization and it seemed to be more demanding than handing back objects in whatever-state to let the user reinit them. Definitely going to look up std::aligned_storage to maybe delay construction! \$\endgroup\$ – ulfben Dec 1 '17 at 9:35
  • \$\begingroup\$ @ulfben, regarding destructors: yes, but not when you are done with the object... \$\endgroup\$ – Frank Dec 1 '17 at 13:13
  • \$\begingroup\$ @ulfben: regarding O(1), It's 100% possible to do this in O(1) and maintain a non-intrusive approach. A simple way of doing this is maintaining a std::stack of freed indces. If you want to be a bit fancier... I normally try to avoid tooting my own horn, but check out my solution to the same problem for an example if you need inspiration \$\endgroup\$ – Frank Dec 1 '17 at 13:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.