From 4c5220a31e08386102a99c6460693bae1ec27d77 Mon Sep 17 00:00:00 2001 From: Bryan Bernhart Date: Fri, 9 Sep 2022 16:10:04 -0700 Subject: [PATCH] Prevent backend leak detection from not reporting leaks. * Moves backend leak detection enabling to ALLOCATOR_FLAG_NEVER_LEAK_MEMORY. * Moves front-end leak detection checks to build flag. * Disables front-end leaks checks via GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS. This change prevents front-end checks from conflicting with the backend checks since the same build flag was used to enable both and the backend checks could not report leaks first. --- CMakeLists.txt | 4 ---- build_overrides/gpgmm_features.gni | 2 +- src/gpgmm/common/MemoryAllocator.cpp | 2 ++ src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp | 15 ++++++++------- src/gpgmm/d3d12/ResourceAllocatorD3D12.h | 7 +++++++ src/tests/D3D12Test.cpp | 3 +++ src/tests/end2end/D3D12ResourceAllocatorTests.cpp | 14 ++++++++++++++ 7 files changed, 35 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4c5e9c1b4..0b99ba1e9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -173,10 +173,6 @@ endif() if(GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS) target_compile_definitions(gpgmm_common_config INTERFACE "GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS") -else() - target_compile_definitions(gpgmm_common_config INTERFACE - $<$:GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS> - ) endif() if(GPGMM_DISABLE_SIZE_CACHE) diff --git a/build_overrides/gpgmm_features.gni b/build_overrides/gpgmm_features.gni index 56a915b61..5f1005af8 100644 --- a/build_overrides/gpgmm_features.gni +++ b/build_overrides/gpgmm_features.gni @@ -50,7 +50,7 @@ declare_args() { # Enables checking of allocator leaks. # Sets -dGPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS - gpgmm_enable_allocator_leak_checks = is_debug + gpgmm_enable_allocator_leak_checks = false # Enables ASSERT on severity functionality. # Sets -dGPGMM_ENABLE_ASSERT_ON_WARNING diff --git a/src/gpgmm/common/MemoryAllocator.cpp b/src/gpgmm/common/MemoryAllocator.cpp index 990e037a8..4e529b36f 100644 --- a/src/gpgmm/common/MemoryAllocator.cpp +++ b/src/gpgmm/common/MemoryAllocator.cpp @@ -74,6 +74,7 @@ namespace gpgmm { } MemoryAllocator::~MemoryAllocator() { +#if defined(GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS) // If memory cannot be reused by a (parent) allocator, ensure no used memory leaked. if (GetParent() == nullptr) { ASSERT(mInfo.UsedBlockUsage == 0u); @@ -81,6 +82,7 @@ namespace gpgmm { ASSERT(mInfo.UsedMemoryCount == 0u); ASSERT(mInfo.UsedMemoryUsage == 0u); } +#endif } std::unique_ptr MemoryAllocator::TryAllocateMemory( diff --git a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp index b83992eed..9a9d84e8e 100644 --- a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp +++ b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp @@ -482,9 +482,9 @@ namespace gpgmm::d3d12 { mIsCustomHeapsDisabled(descriptor.Flags & ALLOCATOR_FLAG_DISABLE_CUSTOM_HEAPS) { GPGMM_TRACE_EVENT_OBJECT_NEW(this); -#if defined(GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS) - mDebugAllocator = std::make_unique(); -#endif + if (descriptor.Flags & ALLOCATOR_FLAG_NEVER_LEAK_MEMORY) { + mDebugAllocator = std::make_unique(); + } const bool isUMA = (IsResidencyEnabled()) ? mResidencyManager->IsUMA() : mCaps->IsAdapterUMA(); @@ -686,6 +686,11 @@ namespace gpgmm::d3d12 { ResourceAllocator::~ResourceAllocator() { GPGMM_TRACE_EVENT_OBJECT_DESTROY(this); + // Give the debug allocator the first chance to report leaks. + if (mDebugAllocator) { + mDebugAllocator->ReportLiveAllocations(); + } + // Destroy allocators in the reverse order they were created so we can record delete events // before event tracer shutdown. mSmallBufferAllocatorOfType = {}; @@ -696,10 +701,6 @@ namespace gpgmm::d3d12 { mResourceAllocatorOfType = {}; mResourceHeapAllocatorOfType = {}; -#if defined(GPGMM_ENABLE_ALLOCATOR_LEAK_CHECKS) - mDebugAllocator->ReportLiveAllocations(); -#endif - #if defined(GPGMM_ENABLE_DEVICE_LEAK_CHECKS) ReportLiveDeviceObjects(mDevice); #endif diff --git a/src/gpgmm/d3d12/ResourceAllocatorD3D12.h b/src/gpgmm/d3d12/ResourceAllocatorD3D12.h index 36486484f..048e75516 100644 --- a/src/gpgmm/d3d12/ResourceAllocatorD3D12.h +++ b/src/gpgmm/d3d12/ResourceAllocatorD3D12.h @@ -83,6 +83,13 @@ namespace gpgmm::d3d12 { the corresponding heap type. */ ALLOCATOR_FLAG_DISABLE_CUSTOM_HEAPS = 0x10, + + /** \brief Report leaks of resource allocations. + + Used to track outstanding allocations made with this allocator. When the allocator is about + to be released, it will report details on any leaked allocations as log messages. + */ + ALLOCATOR_FLAG_NEVER_LEAK_MEMORY = 0x20, }; DEFINE_ENUM_FLAG_OPERATORS(ALLOCATOR_FLAGS) diff --git a/src/tests/D3D12Test.cpp b/src/tests/D3D12Test.cpp index f1c2937ec..c8c19ff18 100644 --- a/src/tests/D3D12Test.cpp +++ b/src/tests/D3D12Test.cpp @@ -86,6 +86,9 @@ namespace gpgmm::d3d12 { desc.Flags |= ALLOCATOR_FLAG_DISABLE_MEMORY_PREFETCH; } + // Make sure leak detection is always enabled. + desc.Flags |= gpgmm::d3d12::ALLOCATOR_FLAG_NEVER_LEAK_MEMORY; + desc.MinLogLevel = GetMessageSeverity(GetLogLevel()); if (IsDumpAllEventsEnabled()) { diff --git a/src/tests/end2end/D3D12ResourceAllocatorTests.cpp b/src/tests/end2end/D3D12ResourceAllocatorTests.cpp index 9d6b04a23..0404b7f7f 100644 --- a/src/tests/end2end/D3D12ResourceAllocatorTests.cpp +++ b/src/tests/end2end/D3D12ResourceAllocatorTests.cpp @@ -424,6 +424,20 @@ TEST_F(D3D12ResourceAllocatorTests, CreateBuffer) { } } +TEST_F(D3D12ResourceAllocatorTests, CreateBufferLeaked) { + ComPtr resourceAllocator; + ASSERT_SUCCEEDED( + ResourceAllocator::CreateAllocator(CreateBasicAllocatorDesc(), &resourceAllocator)); + ASSERT_NE(resourceAllocator, nullptr); + + ComPtr allocation; + ASSERT_SUCCEEDED( + resourceAllocator->CreateResource({}, CreateBasicBufferDesc(kDefaultBufferSize), + D3D12_RESOURCE_STATE_COMMON, nullptr, &allocation)); + + allocation.Detach(); // leaked! +} + // Verifies there are no attribution of heaps when UMA + no read-back. TEST_F(D3D12ResourceAllocatorTests, CreateBufferUMA) { GPGMM_SKIP_TEST_IF(!mIsUMA);