Minor clean-up of MemoryPool.

intel · Sep 14, 2023 · 3c73139 · 3c73139
1 parent 12a2394
commit 3c73139
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 58 deletions.
diff --git a/src/gpgmm/common/IndexedMemoryPool.cpp b/src/gpgmm/common/IndexedMemoryPool.cpp
@@ -15,53 +15,56 @@
 #include "gpgmm/common/IndexedMemoryPool.h"
 
 #include "gpgmm/common/MemoryAllocator.h"
-#include "gpgmm/common/TraceEvent.h"
 
 namespace gpgmm {
 
     IndexedMemoryPool::IndexedMemoryPool(uint64_t memorySize) : MemoryPoolBase(memorySize) {
     }
 
+    IndexedMemoryPool::~IndexedMemoryPool() {
+        ReleasePool(kInvalidSize);
+    }
+
     ResultOrError<std::unique_ptr<MemoryAllocationBase>> IndexedMemoryPool::AcquireFromPool(
         uint64_t indexInPool) {
-        if (indexInPool >= mPool.size()) {
-            mPool.resize(indexInPool + 1);
+        if (indexInPool >= mVec.size()) {
+            mVec.resize(indexInPool + 1);
         }
-        return std::unique_ptr<MemoryAllocationBase>(mPool[indexInPool].release());
+        return std::unique_ptr<MemoryAllocationBase>(mVec[indexInPool].release());
     }
 
     MaybeError IndexedMemoryPool::ReturnToPool(std::unique_ptr<MemoryAllocationBase> allocation,
                                                uint64_t indexInPool) {
-        GPGMM_RETURN_ERROR_IF(this, indexInPool >= mPool.size(), "Index exceeded pool size",
+        GPGMM_RETURN_ERROR_IF(this, indexInPool >= mVec.size(), "Index exceeded pool size",
                               ErrorCode::kBadOperation);
-        mPool[indexInPool] = std::move(allocation);
+        mVec[indexInPool] = std::move(allocation);
         return {};
     }
 
     uint64_t IndexedMemoryPool::ReleasePool(uint64_t bytesToRelease) {
-        return TrimPoolUntil(this, bytesToRelease);
+        return DeallocateAndShrinkUntil(this, bytesToRelease);
     }
 
     uint64_t IndexedMemoryPool::GetPoolSize() const {
         uint64_t count = 0;
-        for (auto& allocation : mPool) {
+        for (auto& allocation : mVec) {
             if (allocation != nullptr) {
                 count++;
             }
         }
         return count;
     }
 
-    IndexedMemoryPool::UnderlyingContainerType::iterator IndexedMemoryPool::begin() {
-        return mPool.begin();
+    IndexedMemoryPool::Iterator IndexedMemoryPool::begin() {
+        return mVec.begin();
     }
 
-    IndexedMemoryPool::UnderlyingContainerType::iterator IndexedMemoryPool::end() {
-        return mPool.end();
+    IndexedMemoryPool::Iterator IndexedMemoryPool::end() {
+        return mVec.end();
     }
 
-    void IndexedMemoryPool::ShrinkPool(uint64_t lastIndex) {
-        mPool.erase(begin(), begin() + lastIndex);
+    void IndexedMemoryPool::ResizePool(uint64_t lastIndex) {
+        mVec.erase(begin(), begin() + lastIndex);
     }
 
 }  // namespace gpgmm
diff --git a/src/gpgmm/common/IndexedMemoryPool.h b/src/gpgmm/common/IndexedMemoryPool.h
@@ -21,12 +21,14 @@
 
 namespace gpgmm {
 
+    // Direct mapped storage of memory allocations.
     class IndexedMemoryPool final : public MemoryPoolBase {
         using UnderlyingContainerType = std::vector<std::unique_ptr<MemoryAllocationBase>>;
+        using Iterator = UnderlyingContainerType::iterator;
 
       public:
         explicit IndexedMemoryPool(uint64_t memorySize);
-        ~IndexedMemoryPool() override = default;
+        ~IndexedMemoryPool() override;
 
         // MemoryPoolBase interface
         ResultOrError<std::unique_ptr<MemoryAllocationBase>> AcquireFromPool(
@@ -36,14 +38,14 @@ namespace gpgmm {
         uint64_t ReleasePool(uint64_t bytesToRelease) override;
         uint64_t GetPoolSize() const override;
 
-        UnderlyingContainerType::iterator begin();
-        UnderlyingContainerType::iterator end();
+        Iterator begin();
+        Iterator end();
 
         // Resizes the pool up to but not including |lastIndex|.
-        void ShrinkPool(uint64_t lastIndex);
+        void ResizePool(uint64_t lastIndex);
 
       private:
-        UnderlyingContainerType mPool;
+        UnderlyingContainerType mVec;
     };
 
 }  // namespace gpgmm

diff --git a/src/gpgmm/common/LIFOMemoryPool.cpp b/src/gpgmm/common/LIFOMemoryPool.cpp
@@ -14,25 +14,24 @@
 
 #include "gpgmm/common/LIFOMemoryPool.h"
 
-#include "gpgmm/common/MemoryAllocation.h"
-#include "gpgmm/common/MemoryAllocator.h"
-#include "gpgmm/common/TraceEvent.h"
-#include "gpgmm/utils/Assert.h"
-
 namespace gpgmm {
 
     LIFOMemoryPool::LIFOMemoryPool(uint64_t memorySize) : MemoryPoolBase(memorySize) {
     }
 
+    LIFOMemoryPool::~LIFOMemoryPool() {
+        ReleasePool(kInvalidSize);
+    }
+
     ResultOrError<std::unique_ptr<MemoryAllocationBase>> LIFOMemoryPool::AcquireFromPool(
         uint64_t indexInPool) {
         GPGMM_RETURN_ERROR_IF(this, indexInPool != kInvalidIndex,
                               "Index was specified but not allowed", ErrorCode::kBadOperation);
 
         std::unique_ptr<MemoryAllocationBase> allocation;
-        if (!mPool.empty()) {
-            allocation = std::move(mPool.front());
-            mPool.pop_front();
+        if (!mStack.empty()) {
+            allocation = std::move(mStack.front());
+            mStack.pop_front();
         }
 
         return allocation;
@@ -42,28 +41,28 @@ namespace gpgmm {
                                             uint64_t indexInPool) {
         GPGMM_RETURN_ERROR_IF(this, indexInPool != kInvalidIndex,
                               "Index was specified but not allowed", ErrorCode::kBadOperation);
-        mPool.push_front(std::move(allocation));
+        mStack.push_front(std::move(allocation));
         return {};
     }
 
     uint64_t LIFOMemoryPool::ReleasePool(uint64_t bytesToRelease) {
-        return TrimPoolUntil(this, bytesToRelease);
+        return DeallocateAndShrinkUntil(this, bytesToRelease);
     }
 
     uint64_t LIFOMemoryPool::GetPoolSize() const {
-        return mPool.size();
+        return mStack.size();
     }
 
-    LIFOMemoryPool::UnderlyingContainerType::iterator LIFOMemoryPool::begin() {
-        return mPool.begin();
+    LIFOMemoryPool::Iterator LIFOMemoryPool::begin() {
+        return mStack.begin();
     }
 
-    LIFOMemoryPool::UnderlyingContainerType::iterator LIFOMemoryPool::end() {
-        return mPool.end();
+    LIFOMemoryPool::Iterator LIFOMemoryPool::end() {
+        return mStack.end();
     }
 
-    void LIFOMemoryPool::ShrinkPool(uint64_t lastIndex) {
-        mPool.erase(begin(), begin() + lastIndex);
+    void LIFOMemoryPool::ResizePool(uint64_t lastIndex) {
+        mStack.erase(begin(), begin() + lastIndex);
     }
 
 }  // namespace gpgmm
diff --git a/src/gpgmm/common/LIFOMemoryPool.h b/src/gpgmm/common/LIFOMemoryPool.h
@@ -21,13 +21,14 @@
 
 namespace gpgmm {
 
-    // Pool using LIFO (newest are recycled first).
+    // LIFO storage of memory allocations (newest are recycled first).
     class LIFOMemoryPool : public MemoryPoolBase {
         using UnderlyingContainerType = std::deque<std::unique_ptr<MemoryAllocationBase>>;
+        using Iterator = UnderlyingContainerType::iterator;
 
       public:
         explicit LIFOMemoryPool(uint64_t memorySize);
-        ~LIFOMemoryPool() override = default;
+        ~LIFOMemoryPool() override;
 
         // MemoryPoolBase interface
         ResultOrError<std::unique_ptr<MemoryAllocationBase>> AcquireFromPool(
@@ -37,14 +38,14 @@ namespace gpgmm {
         uint64_t ReleasePool(uint64_t bytesToFree = kInvalidSize) override;
         uint64_t GetPoolSize() const override;
 
-        UnderlyingContainerType::iterator begin();
-        UnderlyingContainerType::iterator end();
+        Iterator begin();
+        Iterator end();
 
         // Resizes the pool up to but not including |lastIndex|.
-        void ShrinkPool(uint64_t lastIndex);
+        void ResizePool(uint64_t lastIndex);
 
       private:
-        UnderlyingContainerType mPool;
+        UnderlyingContainerType mStack;
     };
 
 }  // namespace gpgmm

diff --git a/src/gpgmm/common/MemoryPool.cpp b/src/gpgmm/common/MemoryPool.cpp
@@ -14,17 +14,13 @@
 
 #include "gpgmm/common/MemoryPool.h"
 
-#include "gpgmm/common/TraceEvent.h"
-
 namespace gpgmm {
 
     MemoryPoolBase::MemoryPoolBase(uint64_t memorySize) : mMemorySize(memorySize) {
-        GPGMM_TRACE_EVENT_OBJECT_NEW(this);
+        ASSERT(mMemorySize != kInvalidSize);
     }
 
-    MemoryPoolBase::~MemoryPoolBase() {
-        GPGMM_TRACE_EVENT_OBJECT_DESTROY(this);
-    }
+    MemoryPoolBase::~MemoryPoolBase() = default;
 
     std::unique_ptr<MemoryAllocationBase> MemoryPoolBase::AcquireFromPoolForTesting(
         uint64_t indexInPool) {

diff --git a/src/gpgmm/common/MemoryPool.h b/src/gpgmm/common/MemoryPool.h
@@ -46,7 +46,7 @@ namespace gpgmm {
         // Deallocate or shrink the pool.
         virtual uint64_t ReleasePool(uint64_t bytesToRelease) = 0;
 
-        // Get the size of the pool.
+        // Gets the number of allocations in the pool.
         virtual uint64_t GetPoolSize() const = 0;
 
         // Returns the size of the memory allocations being pooled.
@@ -55,24 +55,24 @@ namespace gpgmm {
       protected:
         // Shrinks the size of the pool in |mMemorySize| sizes until |bytesToRelease| is reached.
         template <typename MemoryPoolT>
-        uint64_t TrimPoolUntil(MemoryPoolT* pool, uint64_t bytesToRelease) {
-            uint64_t totalBytesReleased = 0;
-            uint64_t lastIndex = 0;
+        uint64_t DeallocateAndShrinkUntil(MemoryPoolT* pool, uint64_t bytesToRelease) {
+            uint64_t bytesReleased = 0;
+            uint64_t lastIndexInPool = 0;
             for (auto& allocation : *pool) {
-                totalBytesReleased += allocation->GetSize();
+                bytesReleased += allocation->GetSize();
                 allocation->GetAllocator()->DeallocateMemory(std::move(allocation));
-                lastIndex++;
-                if (totalBytesReleased >= bytesToRelease) {
+                lastIndexInPool++;
+                if (bytesReleased >= bytesToRelease) {
                     break;
                 }
             }
 
             // Last is non-inclusive or [first, last).
-            if (lastIndex > 0) {
-                pool->ShrinkPool(lastIndex);
+            if (lastIndexInPool > 0) {
+                pool->ResizePool(lastIndexInPool);
             }
 
-            return totalBytesReleased;
+            return bytesReleased;
         }
 
       private: