From a32605f37d73ccd9a8b73b77328c6c757da54a6a Mon Sep 17 00:00:00 2001 From: Bryan Bernhart Date: Tue, 20 Sep 2022 13:49:36 -0700 Subject: [PATCH] Refactor "is heap resident" related testing functions. * Adds "ForTesting" suffix to denote these are not apart of the public Heap interface. * Removes Heap::IsResident in favor of having tests be explicit about what to check. It is now required to check the status by Heap::GetInfo() instead of assuming any tracked heap "is resident", even without calling MakeResident on it. --- .github/workflows/.patches/dawn.diff | 21 ++++++++++--------- src/gpgmm/d3d12/HeapD3D12.cpp | 18 ++++++++-------- src/gpgmm/d3d12/HeapD3D12.h | 7 ++++--- src/gpgmm/d3d12/ResidencyManagerD3D12.cpp | 8 +++---- .../end2end/D3D12ResidencyManagerTests.cpp | 20 +++++++++--------- 5 files changed, 38 insertions(+), 36 deletions(-) diff --git a/.github/workflows/.patches/dawn.diff b/.github/workflows/.patches/dawn.diff index 5077dfdb0..c1c0c42d7 100644 --- a/.github/workflows/.patches/dawn.diff +++ b/.github/workflows/.patches/dawn.diff @@ -1,4 +1,4 @@ -From 5ea9503feb3e76166d484069991aa7e52948e264 Mon Sep 17 00:00:00 2001 +From 268687007244e4659e56bd24c48dbf98382b1893 Mon Sep 17 00:00:00 2001 From: Bryan Bernhart Date: Tue, 15 Feb 2022 17:25:29 -0800 Subject: [PATCH] Use GPGMM for D3D12 backend. @@ -15,7 +15,7 @@ Change-Id: I47708462a1d9dd0166120c3a6af93451aae54a07 src/dawn/native/Toggles.cpp | 8 ++ src/dawn/native/Toggles.h | 2 + src/dawn/native/d3d12/AdapterD3D12.cpp | 4 + - src/dawn/native/d3d12/BufferD3D12.cpp | 51 ++++---- + src/dawn/native/d3d12/BufferD3D12.cpp | 52 ++++---- src/dawn/native/d3d12/BufferD3D12.h | 7 +- src/dawn/native/d3d12/CommandBufferD3D12.cpp | 9 +- .../native/d3d12/CommandRecordingContext.cpp | 19 ++- @@ -32,7 +32,7 @@ Change-Id: I47708462a1d9dd0166120c3a6af93451aae54a07 src/dawn/native/d3d12/UtilsD3D12.cpp | 11 ++ src/dawn/native/d3d12/UtilsD3D12.h | 2 + .../tests/white_box/D3D12ResidencyTests.cpp | 17 +-- - 27 files changed, 321 insertions(+), 166 deletions(-) + 27 files changed, 322 insertions(+), 166 deletions(-) create mode 100644 build_overrides/gpgmm.gni diff --git a/.gitignore b/.gitignore @@ -212,7 +212,7 @@ index e23830e89..144ea3e22 100644 // Create a retrieval filter with a deny list to suppress messages. diff --git a/src/dawn/native/d3d12/BufferD3D12.cpp b/src/dawn/native/d3d12/BufferD3D12.cpp -index 0488fce6a..6b0dd1627 100644 +index 0488fce6a..db8f39de5 100644 --- a/src/dawn/native/d3d12/BufferD3D12.cpp +++ b/src/dawn/native/d3d12/BufferD3D12.cpp @@ -153,9 +153,15 @@ MaybeError Buffer::Initialize(bool mappedAtCreation) { @@ -305,7 +305,7 @@ index 0488fce6a..6b0dd1627 100644 } void* Buffer::GetMappedPointerImpl() { -@@ -392,16 +397,15 @@ void Buffer::DestroyImpl() { +@@ -392,16 +397,16 @@ void Buffer::DestroyImpl() { } BufferBase::DestroyImpl(); @@ -316,7 +316,8 @@ index 0488fce6a..6b0dd1627 100644 bool Buffer::CheckIsResidentForTesting() const { - Heap* heap = ToBackend(mResourceAllocation.GetResourceHeap()); - return heap->IsInList() || heap->IsResidencyLocked(); -+ return static_cast(mResourceAllocation->GetMemory())->IsResident(); ++ return mResourceAllocation->GetMemory()->IsInResidencyLRUCacheForTesting() || ++ mResourceAllocation->GetMemory()->IsResidencyLockedForTesting(); } -bool Buffer::CheckAllocationMethodForTesting(AllocationMethod allocationMethod) const { @@ -326,7 +327,7 @@ index 0488fce6a..6b0dd1627 100644 } MaybeError Buffer::EnsureDataInitialized(CommandRecordingContext* commandContext) { -@@ -446,8 +450,7 @@ MaybeError Buffer::EnsureDataInitializedAsDestination(CommandRecordingContext* c +@@ -446,8 +451,7 @@ MaybeError Buffer::EnsureDataInitializedAsDestination(CommandRecordingContext* c } void Buffer::SetLabelImpl() { @@ -742,7 +743,7 @@ index 0373cf9d8..8b0bffdff 100644 static constexpr uint32_t kMaxSamplerDescriptorsPerBindGroup = 3 * kMaxSamplersPerShaderStage; static constexpr uint32_t kMaxViewDescriptorsPerBindGroup = diff --git a/src/dawn/native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp b/src/dawn/native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp -index fe99a63ac..1ecd21da3 100644 +index fe99a63ac..e2b5bd76f 100644 --- a/src/dawn/native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp +++ b/src/dawn/native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp @@ -93,7 +93,8 @@ ShaderVisibleDescriptorAllocator::ShaderVisibleDescriptorAllocator( @@ -843,13 +844,13 @@ index fe99a63ac..1ecd21da3 100644 bool ShaderVisibleDescriptorAllocator::IsShaderVisibleHeapLockedResidentForTesting() const { - return mHeap->IsResidencyLocked(); -+ return mHeap->GetHeap()->IsResidencyLocked(); ++ return mHeap->GetHeap()->IsResidencyLockedForTesting(); } bool ShaderVisibleDescriptorAllocator::IsLastShaderVisibleHeapInLRUForTesting() const { ASSERT(!mPool.empty()); - return mPool.back().heap->IsInResidencyLRUCache(); -+ return mPool.back().heap->GetHeap()->IsInResidencyLRUCache(); ++ return mPool.back().heap->GetHeap()->IsInResidencyLRUCacheForTesting(); } bool ShaderVisibleDescriptorAllocator::IsAllocationStillValid( diff --git a/src/gpgmm/d3d12/HeapD3D12.cpp b/src/gpgmm/d3d12/HeapD3D12.cpp index 409459b5a..51fe5f479 100644 --- a/src/gpgmm/d3d12/HeapD3D12.cpp +++ b/src/gpgmm/d3d12/HeapD3D12.cpp @@ -103,7 +103,7 @@ namespace gpgmm::d3d12 { // When a heap is destroyed, it no longer resides in resident memory, so we must evict // it from the residency cache. If this heap is not manually removed from the residency // cache, the ResidencyManager will attempt to use it after it has been deallocated. - if (IsInResidencyLRUCache()) { + if (IsInList()) { RemoveFromList(); } @@ -126,10 +126,6 @@ namespace gpgmm::d3d12 { return mMemorySegmentGroup; } - bool Heap::IsInResidencyLRUCache() const { - return IsInList(); - } - void Heap::AddResidencyLockRef() { mResidencyLock.Ref(); } @@ -142,10 +138,6 @@ namespace gpgmm::d3d12 { return mResidencyLock.GetRefCount() > 0; } - bool Heap::IsResident() const { - return IsInList() || IsResidencyLocked(); - } - HEAP_INFO Heap::GetInfo() const { return {IsResidencyLocked(), mState}; } @@ -162,4 +154,12 @@ namespace gpgmm::d3d12 { mState = newStatus; } + bool Heap::IsInResidencyLRUCacheForTesting() const { + return IsInList(); + } + + bool Heap::IsResidencyLockedForTesting() const { + return IsResidencyLocked(); + } + } // namespace gpgmm::d3d12 diff --git a/src/gpgmm/d3d12/HeapD3D12.h b/src/gpgmm/d3d12/HeapD3D12.h index 8c22be558..acef78239 100644 --- a/src/gpgmm/d3d12/HeapD3D12.h +++ b/src/gpgmm/d3d12/HeapD3D12.h @@ -186,9 +186,8 @@ namespace gpgmm::d3d12 { HEAP_INFO GetInfo() const; // Testing only. - bool IsInResidencyLRUCache() const; - bool IsResidencyLocked() const; - bool IsResident() const; + bool IsInResidencyLRUCacheForTesting() const; + bool IsResidencyLockedForTesting() const; private: friend ResidencyManager; @@ -210,6 +209,8 @@ namespace gpgmm::d3d12 { void SetResidencyState(RESIDENCY_STATUS newStatus); + bool IsResidencyLocked() const; + // Locks residency to ensure the heap cannot be evicted (ex. shader-visible descriptor // heaps or mapping resources). void AddResidencyLockRef(); diff --git a/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp b/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp index 153798bf7..b04251450 100644 --- a/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp +++ b/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp @@ -283,7 +283,7 @@ namespace gpgmm::d3d12 { return E_INVALIDARG; } - if (!pHeap->IsResident()) { + if (!pHeap->IsInList() && !pHeap->IsResidencyLocked()) { ComPtr pageable; ReturnIfFailed(pHeap->QueryInterface(IID_PPV_ARGS(&pageable))); ReturnIfFailed(MakeResident(pHeap->GetMemorySegmentGroup(), pHeap->GetSize(), 1, @@ -292,7 +292,7 @@ namespace gpgmm::d3d12 { } // Since we can't evict the heap, it's unnecessary to track the heap in the LRU Cache. - if (pHeap->IsInResidencyLRUCache()) { + if (pHeap->IsInList()) { pHeap->RemoveFromList(); // Untracked heaps are not attributed toward residency usage. @@ -318,7 +318,7 @@ namespace gpgmm::d3d12 { return E_FAIL; } - if (pHeap->IsInResidencyLRUCache()) { + if (pHeap->IsInList()) { return E_FAIL; } @@ -667,7 +667,7 @@ namespace gpgmm::d3d12 { continue; } - if (heap->IsInResidencyLRUCache()) { + if (heap->IsInList()) { // If the heap is already in the LRU, we must remove it and append again below to // update its position in the LRU. heap->RemoveFromList(); diff --git a/src/tests/end2end/D3D12ResidencyManagerTests.cpp b/src/tests/end2end/D3D12ResidencyManagerTests.cpp index 460b689d7..a5d8c8f20 100644 --- a/src/tests/end2end/D3D12ResidencyManagerTests.cpp +++ b/src/tests/end2end/D3D12ResidencyManagerTests.cpp @@ -300,9 +300,9 @@ TEST_F(D3D12ResidencyManagerTests, OverBudget) { allocationsBelowBudget.push_back(std::move(allocation)); } - // Created allocations below the budget should be resident. + // Created allocations below the budget should become resident. for (auto& allocation : allocationsBelowBudget) { - EXPECT_TRUE(allocation->GetMemory()->IsResident()); + EXPECT_TRUE(allocation->GetMemory()->IsInResidencyLRUCacheForTesting()); } // Keep allocating |kMemoryOverBudget| over the budget. @@ -319,14 +319,14 @@ TEST_F(D3D12ResidencyManagerTests, OverBudget) { allocationsAboveBudget.push_back(std::move(allocation)); } - // Created allocations above the budget should be resident. + // Created allocations above the budget should become resident. for (auto& allocation : allocationsAboveBudget) { - EXPECT_TRUE(allocation->GetMemory()->IsResident()); + EXPECT_TRUE(allocation->GetMemory()->IsInResidencyLRUCacheForTesting()); } - // Created allocations below the budget should NOT be resident. + // Created allocations below the budget should NOT become resident. for (auto& allocation : allocationsBelowBudget) { - EXPECT_FALSE(allocation->GetMemory()->IsResident()); + EXPECT_FALSE(allocation->GetMemory()->IsInResidencyLRUCacheForTesting()); } } @@ -369,9 +369,9 @@ TEST_F(D3D12ResidencyManagerTests, OverBudgetAsync) { allocations.push_back(std::move(allocation)); } - // All allocations should be created resident. + // All allocations should become resident. for (auto& allocation : allocations) { - EXPECT_TRUE(allocation->GetMemory()->IsResident()); + EXPECT_TRUE(allocation->GetMemory()->IsInResidencyLRUCacheForTesting()); } } @@ -449,9 +449,9 @@ TEST_F(D3D12ResidencyManagerTests, OverBudgetWithLockedHeaps) { allocationsBelowBudget.push_back(std::move(allocation)); } - // Locked allocations should stay resident. + // Locked heaps should stay locked. for (auto& allocation : allocationsBelowBudget) { - EXPECT_TRUE(allocation->GetMemory()->IsResident()); + EXPECT_EQ(allocation->GetMemory()->GetInfo().IsLocked, true); } // Since locked heaps are ineligable for eviction and HEAP_FLAG_ALWAYS_IN_BUDGET is true,