Core/MemoryPool: Remake memory pool

include/Nazara/Core/MemoryPool.hpp

@@ -8,45 +8,55 @@
 #define NAZARA_CORE_MEMORYPOOL_HPP
 
 #include <Nazara/Prerequisites.hpp>
-#include <atomic>
+#include <Nazara/Core/Bitset.hpp>
 #include <memory>
+#include <vector>
 
 namespace Nz
 {
+	template<typename T, std::size_t Alignment = alignof(T)>
 	class MemoryPool
 	{
 		public:
-			MemoryPool(unsigned int blockSize, unsigned int size = 1024, bool canGrow = true);
+			MemoryPool(std::size_t blockSize);
 			MemoryPool(const MemoryPool&) = delete;
-			MemoryPool(MemoryPool&& pool) noexcept;
+			MemoryPool(MemoryPool&&) noexcept = default;
-			~MemoryPool() = default;
+			~MemoryPool();
 
-			void* Allocate(unsigned int size);
+			template<typename... Args> T* Allocate(std::size_t& index, Args&&... args);
 
-			template<typename T> void Delete(T* ptr);
+			void Clear();
 
-			void Free(void* ptr);
+			void Free(std::size_t index);
 
-			inline unsigned int GetBlockSize() const;
+			std::size_t GetAllocatedEntryCount() const;
-			inline unsigned int GetFreeBlocks() const;
+			std::size_t GetBlockCount() const;
-			inline unsigned int GetSize() const;
+			std::size_t GetBlockSize() const;
+			std::size_t GetFreeEntryCount() const;
 
-			template<typename T, typename... Args> T* New(Args&&... args);
+			void Reset();
+
+			std::size_t RetrieveEntryIndex(const T* data);
 
 			MemoryPool& operator=(const MemoryPool&) = delete;
-			MemoryPool& operator=(MemoryPool&& pool) noexcept;
+			MemoryPool& operator=(MemoryPool&& pool) noexcept = default;
+
+			static constexpr std::size_t InvalidIndex = std::numeric_limits<std::size_t>::max();
 
 		private:
-			MemoryPool(MemoryPool* pool);
+			void AllocateBlock();
 
-			std::unique_ptr<void* []> m_freeList;
+			using AlignedStorage = std::aligned_storage_t<sizeof(T), Alignment>;
-			std::unique_ptr<UInt8[]> m_pool;
+
-			std::unique_ptr<MemoryPool> m_next;
+			struct Block
-			std::atomic_uint m_freeCount;
+			{
-			MemoryPool* m_previous;
+				std::size_t occupiedEntryCount = 0;
-			bool m_canGrow;
+				std::unique_ptr<AlignedStorage[]> memory;
-			unsigned int m_blockSize;
+				Bitset<UInt64> freeEntries;
-			unsigned int m_size;
+			};
+
+			std::size_t m_blockSize;
+			std::vector<Block> m_blocks;
 	};
 }
 

include/Nazara/Core/MemoryPool.inl

@@ -3,6 +3,7 @@
 // For conditions of distribution and use, see copyright notice in Config.hpp
 
 #include <Nazara/Core/MemoryPool.hpp>
+#include <Nazara/Core/Algorithm.hpp>
 #include <Nazara/Core/MemoryHelper.hpp>
 #include <stdexcept>
 #include <utility>
@@ -20,208 +21,231 @@ namespace Nz
 	* \brief Constructs a MemoryPool object
 	*
 	* \param blockSize Size of blocks that will be allocated
-	* \param size Size of the pool
-	* \param canGrow Determine if the pool can allocate more memory
 	*/
-
+	template<typename T, std::size_t Alignment>
-	inline MemoryPool::MemoryPool(unsigned int blockSize, unsigned int size, bool canGrow) :
+	MemoryPool<T, Alignment>::MemoryPool(std::size_t blockSize) :
-	m_freeCount(size),
+	m_blockSize(blockSize)
-	m_previous(nullptr),
-	m_canGrow(canGrow),
-	m_blockSize(blockSize),
-	m_size(size)
 	{
-		m_pool.reset(new UInt8[blockSize * size]);
+		// Allocate one block by default
-		m_freeList.reset(new void* [size]);
+		AllocateBlock();
-
-		// Remplissage de la free list
-		for (unsigned int i = 0; i < size; ++i)
-			m_freeList[i] = &m_pool[m_blockSize * (size-i-1)];
 	}
 
 	/*!
-	* \brief Constructs a MemoryPool object by move semantic
+	* \brief Destroy the memory pool, calling the destructor for every allocated object and desallocating blocks
-	*
-	* \param pool MemoryPool to move into this
 	*/
-
+	template<typename T, std::size_t Alignment>
-	inline MemoryPool::MemoryPool(MemoryPool&& pool) noexcept
+	MemoryPool<T, Alignment>::~MemoryPool()
 	{
-		operator=(std::move(pool));
+		Reset();
-	}
-
-	/*!
-	* \brief Constructs a MemoryPool object by chaining memory pool
-	*
-	* \param pool Previous MemoryPool
-	*/
-
-	inline MemoryPool::MemoryPool(MemoryPool* pool) :
-	MemoryPool(pool->m_blockSize, pool->m_size, pool->m_canGrow)
-	{
-		m_previous = pool;
 	}
 
 	/*!
 	* \brief Allocates enough memory for the size and returns a pointer to it
 	* \return A pointer to memory allocated
 	*
-	* \param size Size to allocate
+	* \param index Output entry index (which can be used for deallocation)
 	*
-	* \remark If the size is greather than the blockSize of pool, new operator is called
+	* \remark If the size is greater than the blockSize of pool, new operator is called
 	*/
-
+	template<typename T, std::size_t Alignment>
-	inline void* MemoryPool::Allocate(unsigned int size)
+	template<typename... Args>
+	T* MemoryPool<T, Alignment>::Allocate(std::size_t& index, Args&&... args)
 	{
-		if (size <= m_blockSize)
+		std::size_t blockIndex = 0;
+		std::size_t localIndex = InvalidIndex;
+		for (; blockIndex < m_blocks.size(); ++blockIndex)
 		{
-			if (m_freeCount > 0)
+			auto& block = m_blocks[blockIndex];
-				return m_freeList[--m_freeCount];
+			if (block.occupiedEntryCount == m_blockSize)
-			else if (m_canGrow)
+				continue;
-			{
-				if (!m_next)
-					m_next.reset(new MemoryPool(this));
 
-				return m_next->Allocate(size);
+			localIndex = block.freeEntries.FindFirst();
-			}
+			assert(localIndex != block.freeEntries.npos);
+			break;
 		}
 
-		return OperatorNew(size);
+		if (blockIndex == m_blocks.size())
+		{
+			// No more room, allocate a new block
+			blockIndex = m_blocks.size();
+			localIndex = 0;
+
+			AllocateBlock();
+		}
+
+		assert(localIndex != InvalidIndex);
+
+		auto& block = m_blocks[blockIndex];
+		block.freeEntries.Reset(localIndex);
+		block.occupiedEntryCount++;
+
+		T* entry = reinterpret_cast<T*>(&block.memory[localIndex]);
+		PlacementNew(entry, std::forward<Args>(args)...);
+
+		index = blockIndex * m_blockSize + localIndex;
+
+		return entry;
 	}
 
 	/*!
-	* \brief Deletes the memory represented by the poiner
+	* \brief Clears the memory pool
 	*
-	* Calls the destructor of the object before releasing it
+	* This is call the destructor of every active entry and invalidate every entry index, and will free every allocated block
 	*
-	* \remark If ptr is null, nothing is done
+	* \see Reset
 	*/
-	template<typename T>
+	template<typename T, std::size_t Alignment>
-	void MemoryPool::Delete(T* ptr)
+	void MemoryPool<T, Alignment>::Clear()
 	{
-		if (ptr)
+		Reset();
-		{
+
-			ptr->~T();
+		m_blocks.clear();
-			Free(ptr);
-		}
 	}
 
 	/*!
-	* \brief Frees the memory represented by the poiner
+	* \brief Returns an object memory to the memory pool
 	*
-	* If the pool gets empty after the call and we are the child of another pool, we commit suicide. If the pointer does not own to a block of the pool, operator delete is called
+	* Calls the destructor of the target object and returns its memory to the pool
 	*
-	* \remark Throws a std::runtime_error if pointer does not point to an element of the pool with NAZARA_CORE_SAFE defined
+	* \param index Index of the allocated object
-	* \remark If ptr is null, nothing is done
+	*
+	* \see Reset
 	*/
-
+	template<typename T, std::size_t Alignment>
-	inline void MemoryPool::Free(void* ptr)
+	void MemoryPool<T, Alignment>::Free(std::size_t index)
 	{
-		if (ptr)
+		std::size_t blockIndex = index / m_blockSize;
-		{
+		std::size_t localIndex = index % m_blockSize;
-			// Does the pointer belong to us ?
-			UInt8* freePtr = static_cast<UInt8*>(ptr);
-			UInt8* poolPtr = m_pool.get();
-			if (freePtr >= poolPtr && freePtr < poolPtr + m_blockSize*m_size)
-			{
-				#if NAZARA_CORE_SAFE
-				if ((freePtr - poolPtr) % m_blockSize != 0)
-					throw std::runtime_error("Invalid pointer (does not point to an element of the pool)");
-				#endif
 
-				m_freeList[m_freeCount++] = ptr;
+		assert(blockIndex < m_blocks.size());
+		auto& block = m_blocks[blockIndex];
+		assert(!block.freeEntries.Test(localIndex));
 
-				// If we are empty and the extension of another pool, we commit suicide
+		assert(block.occupiedEntryCount > 0);
-				if (m_freeCount == m_size && m_previous && !m_next)
+		block.occupiedEntryCount--;
-				{
+
-					m_previous->m_next.release();
+		T* entry = reinterpret_cast<T*>(&block.memory[localIndex]);
-					delete this; // Suicide
+		PlacementDestroy(entry);
-				}
+
-			}
+		block.freeEntries.Set(localIndex);
-			else
+	}
-			{
+
-				if (m_next)
+	/*!
-					m_next->Free(ptr);
+	* \brief Returns the number of allocated entries
-				else
+	* \return How many entries are currently allocated
-					OperatorDelete(ptr);
+	*/
-			}
+	template<typename T, std::size_t Alignment>
-		}
+	std::size_t MemoryPool<T, Alignment>::GetAllocatedEntryCount() const
+	{
+		std::size_t count = 0;
+		for (auto& block : m_blocks)
+			count += block.occupiedEntryCount;
+
+		return count;
+	}
+
+	/*!
+	* \brief Gets the block count
+	* \return How many block are currently allocated for this memory pool
+	*/
+	template<typename T, std::size_t Alignment>
+	std::size_t MemoryPool<T, Alignment>::GetBlockCount() const
+	{
+		return m_blocks.size();
 	}
 
 	/*!
 	* \brief Gets the block size
-	* \return Size of the blocks
+	* \return Size of each block (i.e. how many items can fit in a block)
 	*/
-
+	template<typename T, std::size_t Alignment>
-	inline unsigned int MemoryPool::GetBlockSize() const
+	std::size_t MemoryPool<T, Alignment>::GetBlockSize() const
 	{
 		return m_blockSize;
 	}
 
 	/*!
-	* \brief Gets the number of free blocks
+	* \brief Returns the number of free entries
-	* \return Number of free blocks in the pool
+	* \return How many entries are currently freed
 	*/
-
+	template<typename T, std::size_t Alignment>
-	inline unsigned int MemoryPool::GetFreeBlocks() const
+	std::size_t MemoryPool<T, Alignment>::GetFreeEntryCount() const
 	{
-		return m_freeCount;
+		std::size_t count = m_blocks.size() * m_blockSize;
+		return count - GetAllocatedEntryCount();
 	}
 
 	/*!
-	* \brief Gets the pool size
+	* \brief Resets the memory pool
-	* \return Size of the pool
-	*/
-
-	inline unsigned int MemoryPool::GetSize() const
-	{
-		return m_size;
-	}
-
-	/*!
-	* \brief Creates a new value of type T with arguments
-	* \return Pointer to the allocated object
 	*
-	* \param args Arguments for the new object
+	* This is call the destructor of every active entry and invalidate every entry index, returning the pool to full capacity
+	* Note that memory is not freed
 	*
-	* \remark Constructs inplace in the pool
+	* \see Clear
 	*/
-
+	template<typename T, std::size_t Alignment>
-	template<typename T, typename... Args>
+	void MemoryPool<T, Alignment>::Reset()
-	inline T* MemoryPool::New(Args&&... args)
 	{
-		T* object = static_cast<T*>(Allocate(sizeof(T)));
+		for (std::size_t blockIndex = 0; blockIndex < m_blocks.size(); ++blockIndex)
-		PlacementNew(object, std::forward<Args>(args)...);
-
-		return object;
-	}
-
-	/*!
-	* \brief Assigns the content of another pool by move semantic
-	* \return A reference to this
-	*
-	* \param pool Other pool to move into this
-	*/
-
-	inline MemoryPool& MemoryPool::operator=(MemoryPool&& pool) noexcept
-	{
-		m_blockSize = pool.m_blockSize;
-		m_canGrow = pool.m_canGrow;
-		m_freeCount = pool.m_freeCount.load(std::memory_order_relaxed);
-		m_freeList = std::move(pool.m_freeList);
-		m_pool = std::move(pool.m_pool);
-		m_previous = pool.m_previous;
-		m_next = std::move(pool.m_next);
-		m_size = pool.m_size;
-
-		// If we have been created by another pool, we must make it point to us again
-		if (m_previous)
 		{
-			m_previous->m_next.release();
+			auto& block = m_blocks[blockIndex];
-			m_previous->m_next.reset(this);
+			if (block.occupiedEntryCount == 0)
+				continue;
+
+			for (std::size_t localIndex = 0; localIndex < m_blockSize; ++localIndex)
+			{
+				if (!block.freeEntries.Test(localIndex))
+				{
+					T* entry = reinterpret_cast<T*>(&m_blocks[blockIndex].memory[localIndex]);
+					PlacementDestroy(entry);
+				}
+			}
+
+			block.freeEntries.Reset();
+			block.occupiedEntryCount = 0;
+		}
+	}
+
+	/*!
+	* \brief Retrieve an entry index based on an allocated pointer
+	*
+	* \param data Allocated entry pointed
+	* 
+	* \return Corresponding index, or InvalidIndex if it's not part of this pool
+	*/
+	template<typename T, std::size_t Alignment>
+	std::size_t MemoryPool<T, Alignment>::RetrieveEntryIndex(const T* data)
+	{
+		std::size_t blockIndex = 0;
+		std::size_t localIndex = InvalidIndex;
+		for (; blockIndex < m_blocks.size(); ++blockIndex)
+		{
+			auto& block = m_blocks[blockIndex];
+			const T* startPtr = reinterpret_cast<const T*>(&block.memory[0]);
+			if (data >= startPtr && data < startPtr + m_blockSize)
+			{
+				// Part of block
+				localIndex = SafeCast<std::size_t>(data - startPtr);
+				assert(data == reinterpret_cast<const T*>(&block.memory[localIndex]));
+
+				break;
+			}
 		}
 
-		return *this;
+		if (blockIndex == m_blocks.size())
+			return InvalidIndex;
+
+		assert(localIndex != InvalidIndex);
+
+		return blockIndex * m_blockSize + localIndex;
+	}
+
+	template<typename T, std::size_t Alignment>
+	void MemoryPool<T, Alignment>::AllocateBlock()
+	{
+		auto& block = m_blocks.emplace_back();
+		block.freeEntries.Resize(m_blockSize, true);
+		block.memory = std::make_unique<AlignedStorage[]>(m_blockSize);
 	}
 }
 

include/Nazara/Network/ENetHost.hpp

@@ -145,7 +145,7 @@ namespace Nz
 			std::vector<PendingOutgoingPacket> m_pendingOutgoingPackets;
 			MovablePtr<UInt8> m_receivedData;
 			Bitset<UInt64> m_dispatchQueue;
-			MemoryPool m_packetPool;
+			MemoryPool<ENetPacket> m_packetPool;
 			IpAddress m_address;
 			IpAddress m_receivedAddress;
 			SocketPoller m_poller;

include/Nazara/Network/ENetPacket.hpp

@@ -8,6 +8,7 @@
 #define NAZARA_NETWORK_ENETPACKET_HPP
 
 #include <Nazara/Prerequisites.hpp>
+#include <Nazara/Core/MemoryPool.hpp>
 #include <Nazara/Network/NetPacket.hpp>
 
 namespace Nz
@@ -29,13 +30,11 @@ namespace Nz
 
 	constexpr ENetPacketFlags ENetPacketFlag_Unreliable = 0;
 
-	class MemoryPool;
-
 	struct ENetPacket
 	{
-		MemoryPool* owner;
 		ENetPacketFlags flags;
 		NetPacket data;
+		std::size_t poolIndex;
 		std::size_t referenceCount = 0;
 	};
 
@@ -54,7 +53,7 @@ namespace Nz
 			Reset(packet);
 		}
 
-		ENetPacketRef(ENetPacketRef&& packet) :
+		ENetPacketRef(ENetPacketRef&& packet) noexcept :
 		m_packet(packet.m_packet)
 		{
 			packet.m_packet = nullptr;
@@ -91,7 +90,7 @@ namespace Nz
 			return *this;
 		}
 
-		ENetPacketRef& operator=(ENetPacketRef&& packet)
+		ENetPacketRef& operator=(ENetPacketRef&& packet) noexcept
 		{
 			m_packet = packet.m_packet;
 			packet.m_packet = nullptr;
@@ -99,6 +98,7 @@ namespace Nz
 			return *this;
 		}
 
+		MemoryPool<ENetPacket>* m_pool = nullptr;
 		ENetPacket* m_packet = nullptr;
 	};
 }

src/Nazara/Network/ENetHost.cpp

@@ -311,9 +311,11 @@ namespace Nz
 
 	ENetPacketRef ENetHost::AllocatePacket(ENetPacketFlags flags)
 	{
-		ENetPacketRef enetPacket = m_packetPool.New<ENetPacket>();
+		std::size_t poolIndex;
+		ENetPacketRef enetPacket = m_packetPool.Allocate(poolIndex);
 		enetPacket->flags = flags;
-		enetPacket->owner = &m_packetPool;
+		enetPacket->poolIndex = poolIndex;
+		enetPacket.m_pool = &m_packetPool;
 
 		return enetPacket;
 	}

src/Nazara/Network/ENetPacket.cpp

@@ -14,7 +14,10 @@ namespace Nz
 		if (m_packet)
 		{
 			if (--m_packet->referenceCount == 0)
-				m_packet->owner->Delete(m_packet);
+			{
+				assert(m_pool);
+				m_pool->Free(m_packet->poolIndex);
+			}
 		}
 
 		m_packet = packet;

tests/Engine/Core/MemoryPoolTest.cpp

@@ -3,15 +3,54 @@
 
 #include <Nazara/Math/Vector2.hpp>
 
+namespace
+{
+	std::size_t allocationCount = 0;
+
+	template<typename T>
+	struct AllocatorTest : T
+	{
+		template<typename... Args>
+		AllocatorTest(Args&&... args) :
+		T(std::forward<Args>(args)...)
+		{
+			allocationCount++;
+		}
+
+		AllocatorTest(const AllocatorTest&) = delete;
+		AllocatorTest(AllocatorTest&&) = delete;
+
+		~AllocatorTest()
+		{
+			assert(allocationCount > 0);
+			allocationCount--;
+		}
+	};
+}
+
 SCENARIO("MemoryPool", "[CORE][MEMORYPOOL]")
 {
 	GIVEN("A MemoryPool to contain one Nz::Vector2<int>")
 	{
-		Nz::MemoryPool memoryPool(sizeof(Nz::Vector2<int>), 1, false);
+		using T = AllocatorTest<Nz::Vector2<int>>;
+
+		allocationCount = 0;
+
+		Nz::MemoryPool<T> memoryPool(2);
+		CHECK(memoryPool.GetAllocatedEntryCount() == 0);
+		CHECK(memoryPool.GetBlockCount() == 1);
+		CHECK(memoryPool.GetBlockSize() == 2);
+		CHECK(memoryPool.GetFreeEntryCount() == 2);
+		CHECK(allocationCount == 0);
 
 		WHEN("We construct a Nz::Vector2<int>")
 		{
-			Nz::Vector2<int>* vector2 = memoryPool.New<Nz::Vector2<int>>(1, 2);
+			std::size_t index;
+			T* vector2 = memoryPool.Allocate(index, 1, 2);
+			CHECK(allocationCount == 1);
+			CHECK(memoryPool.GetAllocatedEntryCount() == 1);
+			CHECK(memoryPool.GetFreeEntryCount() == 1);
+			CHECK(memoryPool.RetrieveEntryIndex(vector2) == index);
 
 			THEN("Memory is available")
 			{
@@ -19,14 +58,37 @@ SCENARIO("MemoryPool", "[CORE][MEMORYPOOL]")
 				REQUIRE(*vector2 == Nz::Vector2<int>(3, 2));
 			}
 
-			memoryPool.Delete(vector2);
+			memoryPool.Free(index);
+			CHECK(allocationCount == 0);
+			CHECK(memoryPool.GetAllocatedEntryCount() == 0);
+			CHECK(memoryPool.GetFreeEntryCount() == 2);
 		}
 
 		WHEN("We construct three vectors")
 		{
-			Nz::Vector2<int>* vector1 = memoryPool.New<Nz::Vector2<int>>(1, 2);
+			CHECK(memoryPool.GetAllocatedEntryCount() == 0);
-			Nz::Vector2<int>* vector2 = memoryPool.New<Nz::Vector2<int>>(3, 4);
+			CHECK(memoryPool.GetFreeEntryCount() == 2);
-			Nz::Vector2<int>* vector3 = memoryPool.New<Nz::Vector2<int>>(5, 6);
+
+			std::size_t index1, index2, index3;
+			T* vector1 = memoryPool.Allocate(index1, 1, 2);
+			CHECK(allocationCount == 1);
+			CHECK(memoryPool.GetAllocatedEntryCount() == 1);
+			CHECK(memoryPool.GetBlockCount() == 1);
+			CHECK(memoryPool.GetFreeEntryCount() == 1);
+			T* vector2 = memoryPool.Allocate(index2, 3, 4);
+			CHECK(allocationCount == 2);
+			CHECK(memoryPool.GetAllocatedEntryCount() == 2);
+			CHECK(memoryPool.GetBlockCount() == 1);
+			CHECK(memoryPool.GetFreeEntryCount() == 0);
+			T* vector3 = memoryPool.Allocate(index3, 5, 6);
+			CHECK(allocationCount == 3);
+			CHECK(memoryPool.GetAllocatedEntryCount() == 3);
+			CHECK(memoryPool.GetBlockCount() == 2);
+			CHECK(memoryPool.GetFreeEntryCount() == 1); //< a new block has been allocated
+
+			CHECK(memoryPool.RetrieveEntryIndex(vector1) == index1);
+			CHECK(memoryPool.RetrieveEntryIndex(vector2) == index2);
+			CHECK(memoryPool.RetrieveEntryIndex(vector3) == index3);
 
 			THEN("Memory is available")
 			{
@@ -37,9 +99,17 @@ SCENARIO("MemoryPool", "[CORE][MEMORYPOOL]")
 				CHECK(vector3->GetSquaredLength() == Approx(61.f));
 			}
 
-			memoryPool.Delete(vector1);
+			memoryPool.Reset();
-			memoryPool.Delete(vector2);
+			CHECK(allocationCount == 0);
-			memoryPool.Delete(vector3);
+			CHECK(memoryPool.GetAllocatedEntryCount() == 0);
+			CHECK(memoryPool.GetBlockCount() == 2);
+			CHECK(memoryPool.GetFreeEntryCount() == 4);
+
+			memoryPool.Clear();
+			CHECK(allocationCount == 0);
+			CHECK(memoryPool.GetAllocatedEntryCount() == 0);
+			CHECK(memoryPool.GetBlockCount() == 0);
+			CHECK(memoryPool.GetFreeEntryCount() == 0);
 		}
 	}
 }