Revert "Minor clean-up of MemoryPool."

This reverts commit 3c73139.
intel · Oct 18, 2023 · 12768f3 · 12768f3
1 parent 3c73139
commit 12768f3
Show file tree

Hide file tree

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

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

diff --git a/src/gpgmm/common/MemoryPool.cpp b/src/gpgmm/common/MemoryPool.cpp
@@ -14,13 +14,17 @@
 
 #include "gpgmm/common/MemoryPool.h"
 
+#include "gpgmm/common/TraceEvent.h"
+
 namespace gpgmm {
 
     MemoryPoolBase::MemoryPoolBase(uint64_t memorySize) : mMemorySize(memorySize) {
-        ASSERT(mMemorySize != kInvalidSize);
+        GPGMM_TRACE_EVENT_OBJECT_NEW(this);
     }
 
-    MemoryPoolBase::~MemoryPoolBase() = default;
+    MemoryPoolBase::~MemoryPoolBase() {
+        GPGMM_TRACE_EVENT_OBJECT_DESTROY(this);
+    }
 
     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;
 
-        // Gets the number of allocations in the pool.
+        // Get the size of 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 DeallocateAndShrinkUntil(MemoryPoolT* pool, uint64_t bytesToRelease) {
-            uint64_t bytesReleased = 0;
-            uint64_t lastIndexInPool = 0;
+        uint64_t TrimPoolUntil(MemoryPoolT* pool, uint64_t bytesToRelease) {
+            uint64_t totalBytesReleased = 0;
+            uint64_t lastIndex = 0;
             for (auto& allocation : *pool) {
-                bytesReleased += allocation->GetSize();
+                totalBytesReleased += allocation->GetSize();
                 allocation->GetAllocator()->DeallocateMemory(std::move(allocation));
-                lastIndexInPool++;
-                if (bytesReleased >= bytesToRelease) {
+                lastIndex++;
+                if (totalBytesReleased >= bytesToRelease) {
                     break;
                 }
             }
 
             // Last is non-inclusive or [first, last).
-            if (lastIndexInPool > 0) {
-                pool->ResizePool(lastIndexInPool);
+            if (lastIndex > 0) {
+                pool->ShrinkPool(lastIndex);
             }
 
-            return bytesReleased;
+            return totalBytesReleased;
         }
 
       private: