From b47c3c1be311567417d93d8f0207fd04a61cc533 Mon Sep 17 00:00:00 2001 From: Bryan Bernhart Date: Fri, 20 May 2022 17:45:11 -0700 Subject: [PATCH] Fix leak from ResourceAllocator::Trim(). --- src/gpgmm/common/PooledMemoryAllocator.cpp | 3 + src/gpgmm/common/SegmentedMemoryAllocator.cpp | 1 + src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp | 11 +- src/tests/BUILD.gn | 1 + .../end2end/D3D12ResourceAllocatorTests.cpp | 6 + .../unittests/PooledMemoryAllocatorTests.cpp | 133 ++++++++++++++++++ 6 files changed, 151 insertions(+), 4 deletions(-) create mode 100644 src/tests/unittests/PooledMemoryAllocatorTests.cpp diff --git a/src/gpgmm/common/PooledMemoryAllocator.cpp b/src/gpgmm/common/PooledMemoryAllocator.cpp index d2cc9f8e9..05f0fa8d0 100644 --- a/src/gpgmm/common/PooledMemoryAllocator.cpp +++ b/src/gpgmm/common/PooledMemoryAllocator.cpp @@ -36,6 +36,8 @@ namespace gpgmm { std::lock_guard lock(mMutex); + GPGMM_CHECK_NONZERO(request.SizeInBytes); + std::unique_ptr allocation = mPool->AcquireFromPool(); if (allocation == nullptr) { GPGMM_TRY_ASSIGN(GetNextInChain()->TryAllocateMemory(request), allocation); @@ -69,6 +71,7 @@ namespace gpgmm { } void PooledMemoryAllocator::ReleaseMemory() { + mInfo.FreeMemoryUsage -= mPool->GetInfo().PoolSizeInBytes; mPool->ReleasePool(); } diff --git a/src/gpgmm/common/SegmentedMemoryAllocator.cpp b/src/gpgmm/common/SegmentedMemoryAllocator.cpp index 6161c9944..8a4e28ad6 100644 --- a/src/gpgmm/common/SegmentedMemoryAllocator.cpp +++ b/src/gpgmm/common/SegmentedMemoryAllocator.cpp @@ -187,6 +187,7 @@ namespace gpgmm { for (auto node = mFreeSegments.head(); node != mFreeSegments.end(); node = node->next()) { MemorySegment* segment = node->value(); ASSERT(segment != nullptr); + mInfo.FreeMemoryUsage -= segment->GetInfo().PoolSizeInBytes; segment->ReleasePool(); } } diff --git a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp index d2df424f8..620a525a3 100644 --- a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp +++ b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp @@ -675,10 +675,13 @@ namespace gpgmm { namespace d3d12 { void ResourceAllocator::Trim() { std::lock_guard lock(mMutex); - - for (auto& allocator : mResourceHeapAllocatorOfType) { - ASSERT(allocator != nullptr); - allocator->ReleaseMemory(); + for (uint32_t resourceHeapTypeIndex = 0; resourceHeapTypeIndex < kNumOfResourceHeapTypes; + resourceHeapTypeIndex++) { + mSmallBufferAllocatorOfType[resourceHeapTypeIndex]->ReleaseMemory(); + mMSAAResourceHeapAllocatorOfType[resourceHeapTypeIndex]->ReleaseMemory(); + mMSAAResourceAllocatorOfType[resourceHeapTypeIndex]->ReleaseMemory(); + mResourceHeapAllocatorOfType[resourceHeapTypeIndex]->ReleaseMemory(); + mResourceAllocatorOfType[resourceHeapTypeIndex]->ReleaseMemory(); } } diff --git a/src/tests/BUILD.gn b/src/tests/BUILD.gn index 902d725a6..a07cb7851 100644 --- a/src/tests/BUILD.gn +++ b/src/tests/BUILD.gn @@ -113,6 +113,7 @@ test("gpgmm_unittests") { "unittests/MathTests.cpp", "unittests/MemoryAllocatorTests.cpp", "unittests/MemoryCacheTests.cpp", + "unittests/PooledMemoryAllocatorTests.cpp", "unittests/RefCountTests.cpp", "unittests/SegmentedMemoryAllocatorTests.cpp", "unittests/SlabBlockAllocatorTests.cpp", diff --git a/src/tests/end2end/D3D12ResourceAllocatorTests.cpp b/src/tests/end2end/D3D12ResourceAllocatorTests.cpp index 733a80de6..62acc81e6 100644 --- a/src/tests/end2end/D3D12ResourceAllocatorTests.cpp +++ b/src/tests/end2end/D3D12ResourceAllocatorTests.cpp @@ -611,9 +611,13 @@ TEST_F(D3D12ResourceAllocatorTests, CreateBufferPooled) { EXPECT_EQ(allocation->GetMethod(), gpgmm::AllocationMethod::kStandalone); } + EXPECT_EQ(poolAllocator->GetInfo().FreeMemoryUsage, bufferSize + bufferSize / 2); + // Release the pooled resource heaps. poolAllocator->Trim(); + EXPECT_EQ(poolAllocator->GetInfo().FreeMemoryUsage, 0u); + // Create buffer of size A again with it's own resource heap from the empty pool. { ALLOCATION_DESC reusePoolOnlyDesc = standaloneAllocationDesc; @@ -651,6 +655,8 @@ TEST_F(D3D12ResourceAllocatorTests, CreateBufferPooled) { ASSERT_NE(allocation, nullptr); EXPECT_EQ(allocation->GetMethod(), gpgmm::AllocationMethod::kStandalone); } + + EXPECT_EQ(poolAllocator->GetInfo().FreeMemoryUsage, 0u); } TEST_F(D3D12ResourceAllocatorTests, CreateBufferGetInfo) { diff --git a/src/tests/unittests/PooledMemoryAllocatorTests.cpp b/src/tests/unittests/PooledMemoryAllocatorTests.cpp new file mode 100644 index 000000000..78faf0bfb --- /dev/null +++ b/src/tests/unittests/PooledMemoryAllocatorTests.cpp @@ -0,0 +1,133 @@ +// Copyright 2021 The GPGMM Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "gpgmm/common/PooledMemoryAllocator.h" +#include "tests/DummyMemoryAllocator.h" + +using namespace gpgmm; + +static constexpr uint64_t kDefaultMemorySize = 128u; +static constexpr uint64_t kDefaultMemoryAlignment = 1u; + +class PooledMemoryAllocatorTests : public testing::Test { + public: + MEMORY_ALLOCATION_REQUEST CreateBasicRequest(uint64_t size, + uint64_t alignment, + bool neverAllocate = false) { + MEMORY_ALLOCATION_REQUEST request = {}; + request.SizeInBytes = size; + request.Alignment = alignment; + request.NeverAllocate = neverAllocate; + request.CacheSize = false; + request.AlwaysPrefetch = false; + request.AvailableForAllocation = kInvalidSize; + return request; + } +}; + +TEST_F(PooledMemoryAllocatorTests, SingleHeap) { + PooledMemoryAllocator allocator(kDefaultMemorySize, std::make_unique()); + + std::unique_ptr invalidAllocation = + allocator.TryAllocateMemory(CreateBasicRequest(0, kDefaultMemoryAlignment)); + ASSERT_EQ(invalidAllocation, nullptr); + + std::unique_ptr allocation = allocator.TryAllocateMemory( + CreateBasicRequest(kDefaultMemorySize, kDefaultMemoryAlignment)); + ASSERT_NE(allocation, nullptr); + EXPECT_EQ(allocation->GetSize(), kDefaultMemorySize); + EXPECT_EQ(allocation->GetMethod(), AllocationMethod::kStandalone); + EXPECT_EQ(allocator.GetInfo().UsedMemoryCount, 1u); + + allocator.DeallocateMemory(std::move(allocation)); + EXPECT_EQ(allocator.GetInfo().UsedMemoryCount, 0u); + + allocator.ReleaseMemory(); + EXPECT_EQ(allocator.GetInfo().FreeMemoryUsage, 0u); +} + +TEST_F(PooledMemoryAllocatorTests, MultipleHeaps) { + PooledMemoryAllocator allocator(kDefaultMemorySize, std::make_unique()); + + std::unique_ptr firstAllocation = allocator.TryAllocateMemory( + CreateBasicRequest(kDefaultMemorySize, kDefaultMemoryAlignment)); + ASSERT_NE(firstAllocation, nullptr); + EXPECT_EQ(firstAllocation->GetSize(), kDefaultMemorySize); + + std::unique_ptr secondAllocation = allocator.TryAllocateMemory( + CreateBasicRequest(kDefaultMemorySize, kDefaultMemoryAlignment)); + ASSERT_NE(secondAllocation, nullptr); + EXPECT_EQ(secondAllocation->GetSize(), kDefaultMemorySize); + + EXPECT_EQ(allocator.GetInfo().UsedMemoryCount, 2u); + + allocator.DeallocateMemory(std::move(firstAllocation)); + allocator.DeallocateMemory(std::move(secondAllocation)); + + EXPECT_EQ(allocator.GetInfo().UsedMemoryCount, 0u); + + allocator.ReleaseMemory(); + EXPECT_EQ(allocator.GetInfo().FreeMemoryUsage, 0u); +} + +TEST_F(PooledMemoryAllocatorTests, ReuseFreedHeaps) { + PooledMemoryAllocator allocator(kDefaultMemorySize, std::make_unique()); + { + std::unique_ptr allocation = allocator.TryAllocateMemory( + CreateBasicRequest(kDefaultMemorySize, kDefaultMemoryAlignment)); + ASSERT_NE(allocation, nullptr); + EXPECT_EQ(allocation->GetSize(), kDefaultMemorySize); + EXPECT_EQ(allocation->GetMethod(), AllocationMethod::kStandalone); + allocator.DeallocateMemory(std::move(allocation)); + } + + EXPECT_EQ(allocator.GetInfo().FreeMemoryUsage, kDefaultMemorySize); + + { + std::unique_ptr allocation = allocator.TryAllocateMemory( + CreateBasicRequest(kDefaultMemorySize, kDefaultMemoryAlignment)); + ASSERT_NE(allocation, nullptr); + EXPECT_EQ(allocation->GetSize(), kDefaultMemorySize); + EXPECT_EQ(allocation->GetMethod(), AllocationMethod::kStandalone); + allocator.DeallocateMemory(std::move(allocation)); + } + + EXPECT_EQ(allocator.GetInfo().FreeMemoryUsage, kDefaultMemorySize); +} + +TEST_F(PooledMemoryAllocatorTests, GetInfo) { + PooledMemoryAllocator allocator(kDefaultMemorySize, std::make_unique()); + + std::unique_ptr allocation = allocator.TryAllocateMemory( + CreateBasicRequest(kDefaultMemorySize, kDefaultMemoryAlignment)); + EXPECT_NE(allocation, nullptr); + + // Single memory block should be allocated. + EXPECT_EQ(allocator.GetInfo().UsedBlockCount, 0u); + EXPECT_EQ(allocator.GetInfo().UsedBlockUsage, 0u); + EXPECT_EQ(allocator.GetInfo().UsedMemoryCount, 1u); + EXPECT_EQ(allocator.GetInfo().UsedMemoryUsage, kDefaultMemorySize); + EXPECT_EQ(allocator.GetInfo().FreeMemoryUsage, 0u); + + allocator.DeallocateMemory(std::move(allocation)); + + // Single memory is made available as free after being released. + EXPECT_EQ(allocator.GetInfo().UsedBlockCount, 0u); + EXPECT_EQ(allocator.GetInfo().UsedBlockUsage, 0u); + EXPECT_EQ(allocator.GetInfo().UsedMemoryCount, 0u); + EXPECT_EQ(allocator.GetInfo().UsedMemoryUsage, 0u); + EXPECT_EQ(allocator.GetInfo().FreeMemoryUsage, kDefaultMemorySize); +}