##Comment on testing If you are not going to bother with delete then you may as well write a much simpler version.
template <typename T>
struct pool
{
private:
node* m_head = nullptr;
node* m_tail = nullptr;
std::size_t m_max = 0;
bool m_resize= false;
public:
pool(pool const&) = delete;
pool& operator=(pool const&) = delete;
pool(uint64 nElems, bool resiz = false)
: m_max(nElems)
, m_resize(resiz) {
allocBlock();
}
pool(pool&& o) nothrow {
swap(o);
}
pool& operator=(pool&& o) nothrow {
swap(o);
return *this;
}
void swap(pool& other) nothrow {
using std::swap;
swap(m_head, other.m_head);
swap(m_tail, other.m_tail);
swap(m_max, other.m_max);
swap(m_resize, other.m_resize);
}
operator bool() const {
return m_head != m_tail;
}
T* alloc() {
if ((m_head == m_tail) && m_resizable) {
allocBlock();
}
return m_head == m_tail ? nullptr : m_head++;
}
void free(T* ptr) {}
private:
void allocBlock() {
uint64 alloc_sz = sizeof(T) * m_max;
m_head = m_tail = reinterpret_cast<T*>(std::malloc(alloc_sz));
if (m_head == nullptr) {
throw std::bad_alloc;
}
m_tail += m_max;
}
};
##Code Review
You should mark you move operator as no-throw.
pool(pool&& o) // Why is this not nothrow here.
They should be no-throw anyway good to let the language put that extra testing in for you. Also it can help if you were to use your pool with standard libraries as it enables some optimizations.
Sure your object is invalid after a move. And will not cause any problems if used correctly.
pool(pool&& o)
: m_Chunks{ std::move(o.m_Chunks) }, m_Head{ o.m_Head },
m_MaxElements{ o.m_MaxElements }, m_Resizable{ o.m_Resizable } {
}
But I think it will cause problems if used incorrectly. The m_Head pointer still points at the chain of elements. So you can still accidentally use this and cause real problems for the other allocator that just took ownership of the memory.
The standard way to implement this is using swap(). See my implementation above. This handles situations like this in a safe way.
Same comments about the move assignment operator.
But this looks weird.
pool& operator=(pool&& o) {
for (auto n : m_Chunks) {
std::free(n);
}
You free all the memory. BUT your head still points into this chain and can be-reused. I think that is definitely a bug. A move should not be freeing the memory.
This is strange.
operator bool() const {
return m_Chunks.size();
}
I would have made that return true if there is available memory. Not if we ever successfully allocated memory. But I have not read the standard allocator documentation. So that may be what you want (though I doubt it).
Why are you using uint8?
uint8* mem_raw = reinterpret_cast<uint8*>(ptr);
Should this not be char*. The call offsetof() returns the offset in bytes. The char by definition fits into 1 byte, thus a char* is a 1 byte addressable range. Also the standard has special properties for char* that other integer types don't have.
Also the uint8 type is not standard, though std::uint8_t is standard. But it is not required to be defined. If it is defined then it has exactly 8 bits but if that is not available the type is not available.
##Comments on testing code.
Don't allow the vector to expand.
std::vector<message*> control;
std::vector<message*> test;
Otherwise your are testing the memory manager code. Which is very complex and will give you different results (because memory has all ready been messed with in a previous test).
So pre-allocate the size of the vectors (so they don't reallocate during the test).
control.reserve(2000000);
test.reserve(2000000);
That is way to small a test.
for (uint64 i = 0; i < 200000; ++i) {
Both of these allocators will finish in the blink of an eye. You want a test that last a couple of seconds to get real information.
You are not running the constructor.
// Allocation but no construction
auto t = pool.alloc();
// Allocation and construction.
new message{ r_id, r_time };
You could use placement new to make them more similar. But personally I would override the new/delete operator so that it uses your pool.
The test time will be dominated by this call.
uint64 idx = std::rand() % control.size();
test.erase(test.begin() + idx);
You are forcing a move of all the iterators in the vector. Rather than erasing it just free the memory and set the pointer to nullptr. That would be a much better test.
You should use exactly the same code for both tests. The only difference should be the allocator used. You should have one test that uses your custom allocator. While the second test uses an allocator that simply calls new/delete underneath the hood.
template<typename T>
class normal
{
public:
T* alloc() {return reinterpret_cast<T*>(new char[sizeof(T)]);}
void free(T* ptr) {delete [] reinterpret_cast<char*>(ptr);}
};
int main(void)
{
std::size_t testSize = 200'000'000;
std::vector<message*> control;
std::vector<message*> test;
control.reserve(testSize);
test.reserve(testSize);
measure_exec("Pool", [&]{mem::pool<message> pool{ 32, true }; runtTest(testSize, test, pool);});
measure_exec("New", [&]{mem::normal<message> pool;runtTest(testSize, control, pool);});
std::cin.get();
}