From 98653fddf5c713fb1b3915bb62972275acb0dbe7 Mon Sep 17 00:00:00 2001 From: "Bernhart, Bryan" Date: Tue, 29 Aug 2023 14:34:55 -0700 Subject: [PATCH] Use std::unique_ptr for pool-allocated memory. --- src/gpgmm/common/BuddyMemoryAllocator.cpp | 21 ++++++++++--------- src/gpgmm/common/IndexedMemoryPool.cpp | 12 +++++------ src/gpgmm/common/IndexedMemoryPool.h | 7 ++++--- src/gpgmm/common/LIFOMemoryPool.cpp | 9 ++++---- src/gpgmm/common/LIFOMemoryPool.h | 7 ++++--- src/gpgmm/common/MemoryAllocation.h | 4 ---- src/gpgmm/common/MemoryPool.h | 10 ++++----- src/gpgmm/common/PooledMemoryAllocator.cpp | 20 +++++++++--------- src/gpgmm/common/SegmentedMemoryAllocator.cpp | 16 +++++++------- src/tests/unittests/MemoryPoolTests.cpp | 4 ++-- 10 files changed, 53 insertions(+), 57 deletions(-) diff --git a/src/gpgmm/common/BuddyMemoryAllocator.cpp b/src/gpgmm/common/BuddyMemoryAllocator.cpp index 58b82117b..f203f0c16 100644 --- a/src/gpgmm/common/BuddyMemoryAllocator.cpp +++ b/src/gpgmm/common/BuddyMemoryAllocator.cpp @@ -64,10 +64,11 @@ namespace gpgmm { &mBuddyBlockAllocator, allocationSize, request.Alignment, request.NeverAllocate, [&](const auto& block) -> ResultOrError { const uint64_t memoryIndex = GetMemoryIndex(block->Offset); - MemoryAllocationBase memoryAllocation = mUsedPool.AcquireFromPool(memoryIndex); + std::unique_ptr memoryAllocation = + mUsedPool.AcquireFromPool(memoryIndex); // No existing, allocate new memory for the block. - if (memoryAllocation == GPGMM_INVALID_ALLOCATION) { + if (memoryAllocation == nullptr) { MemoryAllocationRequest newRequest = request; newRequest.SizeInBytes = mMemorySize; newRequest.Alignment = mMemoryAlignment; @@ -79,11 +80,11 @@ namespace gpgmm { return memoryAllocationResult.GetErrorCode(); } - memoryAllocation = *memoryAllocationResult.AcquireResult(); + memoryAllocation = memoryAllocationResult.AcquireResult(); } - MemoryBase* memory = memoryAllocation.GetMemory(); - mUsedPool.ReturnToPool(memoryAllocation, memoryIndex); + MemoryBase* memory = memoryAllocation->GetMemory(); + mUsedPool.ReturnToPool(std::move(memoryAllocation), memoryIndex); return memory; }), @@ -117,16 +118,16 @@ namespace gpgmm { mBuddyBlockAllocator.DeallocateBlock(subAllocation->GetBlock()); - MemoryAllocationBase memoryAllocation = mUsedPool.AcquireFromPool(memoryIndex); + std::unique_ptr memoryAllocation = + mUsedPool.AcquireFromPool(memoryIndex); - MemoryBase* memory = memoryAllocation.GetMemory(); + MemoryBase* memory = memoryAllocation->GetMemory(); ASSERT(memory != nullptr); if (memory->Unref()) { - GetNextInChain()->DeallocateMemory( - std::make_unique(memoryAllocation)); + GetNextInChain()->DeallocateMemory(std::move(memoryAllocation)); } else { - mUsedPool.ReturnToPool(memoryAllocation, memoryIndex); + mUsedPool.ReturnToPool(std::move(memoryAllocation), memoryIndex); } } diff --git a/src/gpgmm/common/IndexedMemoryPool.cpp b/src/gpgmm/common/IndexedMemoryPool.cpp index 2918d15f8..6ef662183 100644 --- a/src/gpgmm/common/IndexedMemoryPool.cpp +++ b/src/gpgmm/common/IndexedMemoryPool.cpp @@ -22,18 +22,16 @@ namespace gpgmm { IndexedMemoryPool::IndexedMemoryPool(uint64_t memorySize) : MemoryPoolBase(memorySize) { } - MemoryAllocationBase IndexedMemoryPool::AcquireFromPool(uint64_t indexInPool) { + std::unique_ptr IndexedMemoryPool::AcquireFromPool(uint64_t indexInPool) { if (indexInPool >= mPool.size()) { mPool.resize(indexInPool + 1); } - MemoryAllocationBase tmp = mPool[indexInPool]; - mPool[indexInPool] = {}; // invalidate it - return tmp; + return std::unique_ptr(mPool[indexInPool].release()); } - void IndexedMemoryPool::ReturnToPool(MemoryAllocationBase allocation, uint64_t indexInPool) { + void IndexedMemoryPool::ReturnToPool(std::unique_ptr allocation, + uint64_t indexInPool) { ASSERT(indexInPool < mPool.size()); - ASSERT(allocation.GetSize() == GetMemorySize()); mPool[indexInPool] = std::move(allocation); } @@ -44,7 +42,7 @@ namespace gpgmm { uint64_t IndexedMemoryPool::GetPoolSize() const { uint64_t count = 0; for (auto& allocation : mPool) { - if (allocation != GPGMM_INVALID_ALLOCATION) { + if (allocation != nullptr) { count++; } } diff --git a/src/gpgmm/common/IndexedMemoryPool.h b/src/gpgmm/common/IndexedMemoryPool.h index 754f80853..4badfb581 100644 --- a/src/gpgmm/common/IndexedMemoryPool.h +++ b/src/gpgmm/common/IndexedMemoryPool.h @@ -27,14 +27,15 @@ namespace gpgmm { ~IndexedMemoryPool() override = default; // MemoryPoolBase interface - MemoryAllocationBase AcquireFromPool(uint64_t indexInPool) override; - void ReturnToPool(MemoryAllocationBase allocation, uint64_t indexInPool) override; + std::unique_ptr AcquireFromPool(uint64_t indexInPool) override; + void ReturnToPool(std::unique_ptr allocation, + uint64_t indexInPool) override; uint64_t ReleasePool(uint64_t bytesToRelease) override; uint64_t GetPoolSize() const override; private: - std::vector mPool; + std::vector> mPool; }; } // namespace gpgmm diff --git a/src/gpgmm/common/LIFOMemoryPool.cpp b/src/gpgmm/common/LIFOMemoryPool.cpp index f6adb4452..7032cf56a 100644 --- a/src/gpgmm/common/LIFOMemoryPool.cpp +++ b/src/gpgmm/common/LIFOMemoryPool.cpp @@ -24,10 +24,10 @@ namespace gpgmm { LIFOMemoryPool::LIFOMemoryPool(uint64_t memorySize) : MemoryPoolBase(memorySize) { } - MemoryAllocationBase LIFOMemoryPool::AcquireFromPool(uint64_t indexInPool) { + std::unique_ptr LIFOMemoryPool::AcquireFromPool(uint64_t indexInPool) { ASSERT(indexInPool == kInvalidIndex); - MemoryAllocationBase allocation = {}; + std::unique_ptr allocation; if (!mPool.empty()) { allocation = std::move(mPool.front()); mPool.pop_front(); @@ -35,10 +35,9 @@ namespace gpgmm { return allocation; } - void LIFOMemoryPool::ReturnToPool(MemoryAllocationBase allocation, uint64_t indexInPool) { + void LIFOMemoryPool::ReturnToPool(std::unique_ptr allocation, + uint64_t indexInPool) { ASSERT(indexInPool == kInvalidIndex); - ASSERT(allocation.GetSize() == GetMemorySize()); - mPool.push_front(std::move(allocation)); } diff --git a/src/gpgmm/common/LIFOMemoryPool.h b/src/gpgmm/common/LIFOMemoryPool.h index d8770d698..682271627 100644 --- a/src/gpgmm/common/LIFOMemoryPool.h +++ b/src/gpgmm/common/LIFOMemoryPool.h @@ -28,15 +28,16 @@ namespace gpgmm { ~LIFOMemoryPool() override = default; // MemoryPoolBase interface - MemoryAllocationBase AcquireFromPool(uint64_t indexInPool = kInvalidIndex) override; - void ReturnToPool(MemoryAllocationBase allocation, + std::unique_ptr AcquireFromPool( + uint64_t indexInPool = kInvalidIndex) override; + void ReturnToPool(std::unique_ptr allocation, uint64_t indexInPool = kInvalidIndex) override; uint64_t ReleasePool(uint64_t bytesToFree = kInvalidSize) override; uint64_t GetPoolSize() const override; private: - std::deque mPool; + std::deque> mPool; }; } // namespace gpgmm diff --git a/src/gpgmm/common/MemoryAllocation.h b/src/gpgmm/common/MemoryAllocation.h index 8d1eb1952..690389b2e 100644 --- a/src/gpgmm/common/MemoryAllocation.h +++ b/src/gpgmm/common/MemoryAllocation.h @@ -19,10 +19,6 @@ #include "gpgmm/common/Object.h" #include "gpgmm/utils/Limits.h" -#define GPGMM_INVALID_ALLOCATION \ - MemoryAllocationBase { \ - } - namespace gpgmm { // Represents how memory was allocated. diff --git a/src/gpgmm/common/MemoryPool.h b/src/gpgmm/common/MemoryPool.h index 821a7d8ab..a959d53c6 100644 --- a/src/gpgmm/common/MemoryPool.h +++ b/src/gpgmm/common/MemoryPool.h @@ -31,11 +31,12 @@ namespace gpgmm { // Retrieves a memory allocation from the pool using an optional index. // Use kInvalidIndex to specify |this| pool is not indexed. - virtual MemoryAllocationBase AcquireFromPool(uint64_t indexInPool) = 0; + virtual std::unique_ptr AcquireFromPool(uint64_t indexInPool) = 0; // Returns a memory allocation back to the pool using an optional index. // Use kInvalidIndex to specify |this| pool is not indexed. - virtual void ReturnToPool(MemoryAllocationBase allocation, uint64_t indexInPool) = 0; + virtual void ReturnToPool(std::unique_ptr allocation, + uint64_t indexInPool) = 0; // Deallocate or shrink the pool. virtual uint64_t ReleasePool(uint64_t bytesToRelease) = 0; @@ -53,9 +54,8 @@ namespace gpgmm { uint64_t totalBytesReleased = 0; uint64_t lastIndex = 0; for (auto& allocation : pool) { - totalBytesReleased += allocation.GetSize(); - allocation.GetAllocator()->DeallocateMemory( - std::make_unique(allocation)); + totalBytesReleased += allocation->GetSize(); + allocation->GetAllocator()->DeallocateMemory(std::move(allocation)); lastIndex++; if (totalBytesReleased >= bytesToRelease) { break; diff --git a/src/gpgmm/common/PooledMemoryAllocator.cpp b/src/gpgmm/common/PooledMemoryAllocator.cpp index 9c0d31991..5777634a9 100644 --- a/src/gpgmm/common/PooledMemoryAllocator.cpp +++ b/src/gpgmm/common/PooledMemoryAllocator.cpp @@ -44,26 +44,26 @@ namespace gpgmm { GPGMM_RETURN_IF_ERROR(ValidateRequest(request)); - MemoryAllocationBase allocation = mPool->AcquireFromPool(kInvalidIndex); - if (allocation == GPGMM_INVALID_ALLOCATION) { + std::unique_ptr allocation = mPool->AcquireFromPool(kInvalidIndex); + if (allocation == nullptr) { MemoryAllocationRequest memoryRequest = request; memoryRequest.Alignment = mMemoryAlignment; memoryRequest.SizeInBytes = AlignTo(request.SizeInBytes, mMemoryAlignment); std::unique_ptr allocationPtr; GPGMM_TRY_ASSIGN(GetNextInChain()->TryAllocateMemory(memoryRequest), allocationPtr); - allocation = *allocationPtr; + allocation.reset(allocationPtr.release()); } else { - mStats.FreeMemoryUsage -= allocation.GetSize(); + mStats.FreeMemoryUsage -= allocation->GetSize(); } mStats.UsedMemoryCount++; - mStats.UsedMemoryUsage += allocation.GetSize(); + mStats.UsedMemoryUsage += allocation->GetSize(); - MemoryBase* memory = allocation.GetMemory(); + MemoryBase* memory = allocation->GetMemory(); ASSERT(memory != nullptr); - return std::make_unique(this, memory, allocation.GetRequestSize()); + return std::make_unique(this, memory, allocation->GetRequestSize()); } void PooledMemoryAllocator::DeallocateMemory(std::unique_ptr allocation) { @@ -80,9 +80,9 @@ namespace gpgmm { MemoryBase* memory = allocation->GetMemory(); ASSERT(memory != nullptr); - mPool->ReturnToPool( - MemoryAllocationBase(GetNextInChain(), memory, allocation->GetRequestSize()), - kInvalidIndex); + mPool->ReturnToPool(std::make_unique(GetNextInChain(), memory, + allocation->GetRequestSize()), + kInvalidIndex); } uint64_t PooledMemoryAllocator::ReleaseMemory(uint64_t bytesToRelease) { diff --git a/src/gpgmm/common/SegmentedMemoryAllocator.cpp b/src/gpgmm/common/SegmentedMemoryAllocator.cpp index ba9d8530f..a26be18fd 100644 --- a/src/gpgmm/common/SegmentedMemoryAllocator.cpp +++ b/src/gpgmm/common/SegmentedMemoryAllocator.cpp @@ -132,23 +132,23 @@ namespace gpgmm { MemorySegment* segment = GetOrCreateFreeSegment(memorySize); ASSERT(segment != nullptr); - MemoryAllocationBase allocation = segment->AcquireFromPool(); - if (allocation == GPGMM_INVALID_ALLOCATION) { + std::unique_ptr allocation = segment->AcquireFromPool(); + if (allocation == nullptr) { MemoryAllocationRequest memoryRequest = request; memoryRequest.Alignment = mMemoryAlignment; memoryRequest.SizeInBytes = AlignTo(request.SizeInBytes, mMemoryAlignment); std::unique_ptr allocationPtr; GPGMM_TRY_ASSIGN(GetNextInChain()->TryAllocateMemory(memoryRequest), allocationPtr); - allocation = *allocationPtr; + allocation.reset(allocationPtr.release()); } else { - mStats.FreeMemoryUsage -= allocation.GetSize(); + mStats.FreeMemoryUsage -= allocation->GetSize(); } mStats.UsedMemoryCount++; - mStats.UsedMemoryUsage += allocation.GetSize(); + mStats.UsedMemoryUsage += allocation->GetSize(); - MemoryBase* memory = allocation.GetMemory(); + MemoryBase* memory = allocation->GetMemory(); ASSERT(memory != nullptr); memory->SetPool(segment); @@ -175,8 +175,8 @@ namespace gpgmm { MemoryPoolBase* pool = memory->GetPool(); ASSERT(pool != nullptr); - static_cast(pool)->ReturnToPool( - MemoryAllocationBase(GetNextInChain(), memory, allocation->GetRequestSize())); + static_cast(pool)->ReturnToPool(std::make_unique( + GetNextInChain(), memory, allocation->GetRequestSize())); } uint64_t SegmentedMemoryAllocator::ReleaseMemory(uint64_t bytesToRelease) { diff --git a/src/tests/unittests/MemoryPoolTests.cpp b/src/tests/unittests/MemoryPoolTests.cpp index 32ed4d6b3..263550f41 100644 --- a/src/tests/unittests/MemoryPoolTests.cpp +++ b/src/tests/unittests/MemoryPoolTests.cpp @@ -42,7 +42,7 @@ TEST_F(LIFOMemoryPoolTests, SingleAllocation) { EXPECT_EQ(pool.GetPoolSize() * pool.GetMemorySize(), 0u); pool.ReturnToPool( - *allocator.TryAllocateMemoryForTesting(CreateBasicRequest(kDefaultMemorySize))); + allocator.TryAllocateMemoryForTesting(CreateBasicRequest(kDefaultMemorySize))); EXPECT_EQ(pool.GetPoolSize() * pool.GetMemorySize(), kDefaultMemorySize); EXPECT_EQ(pool.GetPoolSize(), 1u); @@ -68,7 +68,7 @@ TEST_F(LIFOMemoryPoolTests, MultipleAllocations) { constexpr uint64_t kPoolSize = 64; while (pool.GetPoolSize() < kPoolSize) { pool.ReturnToPool( - *allocator.TryAllocateMemoryForTesting(CreateBasicRequest(kDefaultMemorySize))); + allocator.TryAllocateMemoryForTesting(CreateBasicRequest(kDefaultMemorySize))); } EXPECT_EQ(pool.GetPoolSize() * pool.GetMemorySize(), kDefaultMemorySize * kPoolSize);