3
\$\begingroup\$

Description

This is an improved implementation for a previous post

My goal was to implement a decent object pool using templates and policy based design. The object pool behaviour can be customized trough 2 policies:

  • Allocation Policy: responsible for providing valid memory adresses
  • Initialization Policy: initializes and resets objects when they are acquired/released

The default implementation allocates memory using std::aligned_storage on construction and uses placement new to initilize each object when it's acquired. When an object is returned to the pool, its destructor is called.

Thanks to the policy based design users could easily adapt the pool to fit their needs (e.g.: create a dynamic resizeable object pool).

I decided to not offer a smart pointer interface because I think that ensuring to call ObjectPool::release is the user's responsiblity.

Questions

  • What do you think of the class design?
  • Should it return smart pointers?
  • Do you think the policies should be different?

Code

ObjectPool.h

#include <iostream>
#include <memory>
#include <functional>

namespace {
/**
 * Allocator policy for fixed object pools
 *
 * @tparam T Type of the contained objects
 * @tparam Size Max capacity
 */
template <class T, unsigned int Size> struct FixedPoolAllocatorPolicy {
protected:
  FixedPoolAllocatorPolicy() = default;
  FixedPoolAllocatorPolicy(const FixedPoolAllocatorPolicy&) = delete;
  ~FixedPoolAllocatorPolicy() = default;

  /**
   * Get a pointer to uninitialized raw memory able to hold
   * an object of type T
   * @return pointer or nullptr if there is no space available
   */
  void* acquire() {
    unsigned int index = 0;
    while(m_occupied_registry[index]) ++index;
    if(index >= Size) return nullptr;

    void* element = (void*) &m_objects[index];
    m_occupied_registry[index] = true; 
    return element;
  }

  /**
   * Return an element to the pool
   *
   * @param element The element
   */
  void release(T* element) {
    unsigned int index = element - (T*)&m_objects;
    if(index >= Size) return;

    m_occupied_registry[index] = false;
  }


  /**
   * Return a pointer to the whole memory block
   */
  void* data() { return &m_objects; }


  /**
   * Get the max number of elements that the allcator can hold
   */
  unsigned int size() const { return Size; }

  friend std::ostream& operator<<(std::ostream& ostream, const FixedPoolAllocatorPolicy<T, Size>& pool) {
    for(unsigned int index = 0; index < Size; ++index) {
      if(pool.m_occupied_registry[index]) {
        std::cout<<"["<<&pool.m_objects[index]<<"]"<<(T&)pool.m_objects[index]<<"\n";
      }
      else {
        std::cout<<"[free]\n";
      }
    }
    return ostream;
  }

private:
  typename std::aligned_storage<sizeof(T),alignof(T)>::type m_objects[Size];
  bool m_occupied_registry[Size] {0};
};


/**
 * Policy responsible for initilizing/resetting an element 
 * when it's acquired/returned to the pool
 * @tparam T element type
 */
template <class T> struct DefaultInitializePolicy {
  template <class ... Args>
  T* onAcquire(void* memory, Args&& ... args) {
    return new (memory) T(std::forward<Args>(args)...);
  }

  void onRelease(T* element) {
    element->~T();
  }
};
}

/**
 * Class for object pool.
 *
 * @tparam T Type of the ojects that will be contained in the pool
 * 
 * @tparam AllocatorPolicy Policy responsible for managing memory
 *   - Default constructible
 *   - Non copyable/movable
 *   - Will be inherited (public)
 *   - Required members:
 *     - void* acquire(void): 
 *       Return pointer to memory block big enough to hold an object of type T
 *       Whether there is an object already allocated or not is implementation dependent
 *       
 *     - void release(T*):
 *       Return an object to the pool
 *     
 * @tparam InitilizePolicy Policy responsible for initializing/resetting objects when they are acquired or released
 *   - Required members:
 *     - template <class ... Args> T* onAcquire(void* memory, Args&& ... args): 
 *       Initialize object given the pointer that AllocatorPolicy::acquire() returns
 *       This member receives the arguments sent to ObjectPool::acquire (perfect forwarding)
 *       
 *     - void onRelease(T* element):
 *       Reset object given the pointer that ObjectPool::relase() receives
 */
template <class T, class AllocatorPolicy, class InitilizePolicy> class ObjectPool final : public AllocatorPolicy {
public:

  ObjectPool(InitilizePolicy&& initialize_policy = InitilizePolicy{}) :
  m_initialize_policy(initialize_policy)
  {
    std::cout<< "Pool of elements with size " << sizeof(T) << " and alignment "<< alignof(T) << "\n";
  }

  ObjectPool(const ObjectPool<T, AllocatorPolicy, InitilizePolicy>& orig) = delete;
  ~ObjectPool() = default;

  /**
   * Acquires an object not being currently used
   * @return pointer to the acquired object
   */
  template <class ... Args>
  T* acquire(Args&& ... args) {        
    void *memory = AllocatorPolicy::acquire();
    if(!memory) return nullptr;

    return m_initialize_policy.onAcquire(memory, std::forward<Args>(args)...);
  }

  void release(T* element) {
    m_initialize_policy.onRelease(element);
    AllocatorPolicy::release(element);
  }

  friend std::ostream& operator<<(std::ostream& ostream, const ObjectPool<T, AllocatorPolicy, InitilizePolicy>& pool) {
    ostream << (const AllocatorPolicy&) pool;
    return ostream;
  }

private:
  InitilizePolicy m_initialize_policy;
};

main.cpp

#include "ObjectPool.h"
#include <vector>

template <class T> struct MyInitializePolicy {
public:
  MyInitializePolicy(const char* message) : m_message(message) {}

  //template <class ... Args>
  T* onAcquire(void* memory) {
    std::cout<<m_message;
    return new (memory) T(/*std::forward<Args>(args)...*/);
  }

  void onRelease(T* element) {
    element->~T();
  }

private:
  const char* m_message;
};

class Test {
public:
  Test():m_a(0),m_b('0'){
    std::cout<<"Test()["<<this<<"]"<<*this<<"\n";
  }
  Test(int a, char b): m_a(a), m_b(b) { 
    std::cout<<"Test(int,char)["<<this<<"]"<<*this<<"\n";
  }
  Test(const Test& orig): m_a(orig.m_a),m_b(orig.m_b) {
    std::cout<<"Test(Test&)["<<this<<"]"<<*this<<"\n";
  }
  ~Test() {
    std::cout<<"~Test()["<<this<<"]"<<*this<<"\n";
  }
  friend std::ostream& operator<<(std::ostream& ostream, const Test& test) {
    ostream<<"["<<test.m_a<<",'"<<test.m_b<<"']";
    return ostream;
  }
private:
  int m_a;
  char m_b;
};

int main(int argc, char** argv) {
  using TestPool = ObjectPool<Test,FixedPoolAllocatorPolicy<Test,5>, MyInitializePolicy<Test>>;
  TestPool pool(MyInitializePolicy<Test>{"My own message"});
  Test a(3,'c');
  std::vector<Test*> pool_objects {pool.acquire(), pool.acquire(), pool.acquire(), pool.acquire(), pool.acquire(), pool.acquire(), pool.acquire()};

  std::cout<<"Acquired elements: ";
  for(const Test* test : pool_objects) {
    std::cout << test << " ";
  }
  std::cout<<"\n";

  std::cout<<pool;

  std::cout<<"Returning element 0 to the pool:\n";
  pool.release(pool_objects[0]);
  std::cout<<pool;

  std::cout<<"Returning element 3 to the pool:\n";
  pool.release(pool_objects[3]);
  std::cout<<pool;
  return 0;
}

You can play with the code a little here

\$\endgroup\$
2
\$\begingroup\$

What do you think of the class design?

It's generally fine, but there's room for improvement.

flexibility

Your pool should accept stateful allocators.

readability

~ObjectPool() = default; is redundant.

ObjectPool(const ObjectPool<T, AllocatorPolicy, InitilizePolicy>& orig) = delete;
//can simply be:
ObjectPool(const ObjectPool&) = delete;

design

The AllocatorPolicy should be a member of ObjectPool, not a base class. The relationship is clearly "has-a" instead of "is-a".

Your acquire() function of FixedPoolAllocatorPolicy runs in O(N), which is pretty poor for a fixed pool allocator. You can, and should, make it O(1).

code details

void* should be used when you have no idea what the type is. That's not the case here, you know you are dealing with arrays of bytes, so char* is preferable.

Do not use C-style casts. They are dangerous. Primarily because you can accidentally cast away constness.

wording

"Policy" is a bit of an abuse of language here. In my mind, policies tend to be fully stateless tag types. I would have used Allocator, and Initializer instead.

Should it return smart pointers?

It should return auto, this way, you can delegate that problem to the initializer policy.

Do you think the policies should be different?

I think the fixed pool policy should allocate its data in a vector since it's likely to be allocated on the stack (like in your example!). You are running too high of a risk of someone accidentally making a huge allocation on the stack, and the performance gain from removing that one indirection is negligible. This would also free you up to choose the pool size at runtime.

\$\endgroup\$
  • \$\begingroup\$ Great points! I chose to use inheritance for the Allocator because it would allow users to implement an Allocator with extended funcionality (e.g.: a resize method) I didnt think of using auto, it's a great idea. \$\endgroup\$ – amc176 Sep 20 '17 at 14:55
  • 1
    \$\begingroup\$ You can acheive the "extended functionality" by simply adding a allocator() member. \$\endgroup\$ – Frank Sep 20 '17 at 14:58
  • \$\begingroup\$ That would be great, thanks for helping. I'll try to work on it this evening :) \$\endgroup\$ – amc176 Sep 20 '17 at 15:23

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.