From 9a7be2790cb03671597058b6b2d2804d073857e0 Mon Sep 17 00:00:00 2001 From: "Bernhart, Bryan" Date: Fri, 17 Nov 2023 16:25:28 -0800 Subject: [PATCH] Move Lock/Unlock to ResidencyHeap. --- include/gpgmm_d3d12.h | 39 +++++++++---------- src/gpgmm/d3d12/ResidencyHeapD3D12.cpp | 12 ++++-- src/gpgmm/d3d12/ResidencyHeapD3D12.h | 9 ++--- src/gpgmm/d3d12/ResidencyManagerD3D12.cpp | 4 +- src/gpgmm/d3d12/ResidencyManagerD3D12.h | 5 ++- src/mvi/gpgmm_d3d12.cpp | 16 ++++---- src/mvi/gpgmm_d3d12.h | 4 +- .../end2end/D3D12ResidencyManagerTests.cpp | 26 ++++++------- 8 files changed, 58 insertions(+), 57 deletions(-) diff --git a/include/gpgmm_d3d12.h b/include/gpgmm_d3d12.h index 8077c376..4a0f7e61 100644 --- a/include/gpgmm_d3d12.h +++ b/include/gpgmm_d3d12.h @@ -203,6 +203,22 @@ namespace gpgmm::d3d12 { */ virtual RESIDENCY_HEAP_INFO GetInfo() const = 0; + /** \brief Locks the specified heap. + + Locking a heap means the residency manager will never evict it when over budget. + + \return S_OK if locking was successful. + */ + virtual HRESULT Lock() = 0; + + /** \brief Unlocks the specified heap. + + Unlocking a heap allows the residency manager will evict it when over budget. + + \return S_OK if unlocking was successful or S_FALSE if a lock remains. + */ + virtual HRESULT Unlock() = 0; + /** \brief Get the residency manager that manages this heap. @param[out] ppResidencyManagerOut Pointer to a memory block that receives a pointer to the @@ -285,9 +301,8 @@ namespace gpgmm::d3d12 { IResourceAllocation::GetMemory, into the list. Once IResidencyManager::ExecuteCommandLists is called, the list can be reset or cleared for the next frame or compute job. - Without IResidencyList, the application would need to call IResidencyManager::LockHeap and - IResidencyManager::UnlockHeap for each heap before and after every GPU command or command-list - being executed. + Without IResidencyList, the application would need to lock and unlock each heap before and + after every GPU command or command-list being executed. */ GPGMM_INTERFACE IResidencyList : public IUnknown { /** \brief Adds a heap to the residency list. @@ -569,24 +584,6 @@ namespace gpgmm::d3d12 { **/ GPGMM_INTERFACE IResidencyManager : public IDebugObject { public: - /** \brief Locks the specified heap. - - Locking a heap means the residency manager will never evict it when over budget. - - @param pHeap A pointer to the heap being locked. - */ - virtual HRESULT LockHeap(IResidencyHeap * pHeap) = 0; - - /** \brief Unlocks the specified heap. - - Unlocking a heap allows the residency manager will evict it when over budget. - - @param pHeap A pointer to the heap being unlocked. - - \return S_OK if unlocking was successful or S_FALSE if a lock remains. - */ - virtual HRESULT UnlockHeap(IResidencyHeap * pHeap) = 0; - /** \brief Execute command lists using residency managed heaps or E_OUTOFMEMORY. Submits an array of command lists and residency lists for the specified command queue. diff --git a/src/gpgmm/d3d12/ResidencyHeapD3D12.cpp b/src/gpgmm/d3d12/ResidencyHeapD3D12.cpp index fc65fbc4..ce5349a6 100644 --- a/src/gpgmm/d3d12/ResidencyHeapD3D12.cpp +++ b/src/gpgmm/d3d12/ResidencyHeapD3D12.cpp @@ -154,7 +154,7 @@ namespace gpgmm::d3d12 { } std::unique_ptr heap( - new ResidencyHeap(pResidencyManager, pPageable, newDescriptor, isResidencyDisabled)); + new ResidencyHeap(residencyManager, pPageable, newDescriptor, isResidencyDisabled)); if (!isResidencyDisabled) { // Check if the underlying memory was implicitly made resident. @@ -280,7 +280,7 @@ namespace gpgmm::d3d12 { ppResidencyHeapOut); } - ResidencyHeap::ResidencyHeap(ComPtr residencyManager, + ResidencyHeap::ResidencyHeap(ComPtr residencyManager, ComPtr pageable, const RESIDENCY_HEAP_DESC& descriptor, bool isResidencyDisabled) @@ -382,12 +382,16 @@ namespace gpgmm::d3d12 { } HRESULT ResidencyHeap::Lock() { - ASSERT(mResidencyManager != nullptr); + if (mResidencyManager == nullptr) { + return S_FALSE; + } return mResidencyManager->LockHeap(this); } HRESULT ResidencyHeap::Unlock() { - ASSERT(mResidencyManager != nullptr); + if (mResidencyManager == nullptr) { + return S_FALSE; + } return mResidencyManager->UnlockHeap(this); } diff --git a/src/gpgmm/d3d12/ResidencyHeapD3D12.h b/src/gpgmm/d3d12/ResidencyHeapD3D12.h index aa9ba3ab..308c7b83 100644 --- a/src/gpgmm/d3d12/ResidencyHeapD3D12.h +++ b/src/gpgmm/d3d12/ResidencyHeapD3D12.h @@ -54,6 +54,8 @@ namespace gpgmm::d3d12 { // IResidencyHeap interface RESIDENCY_HEAP_INFO GetInfo() const override; + HRESULT Lock() override; + HRESULT Unlock() override; HRESULT GetResidencyManager(IResidencyManager** ppResidencyManagerOut) const override; // IUnknown interface @@ -65,13 +67,10 @@ namespace gpgmm::d3d12 { LPCWSTR GetDebugName() const override; HRESULT SetDebugName(LPCWSTR Name) override; - HRESULT Lock(); - HRESULT Unlock(); - private: friend ResidencyManager; - ResidencyHeap(ComPtr residencyManager, + ResidencyHeap(ComPtr residencyManager, ComPtr pageable, const RESIDENCY_HEAP_DESC& descriptor, bool isResidencyDisabled); @@ -100,7 +99,7 @@ namespace gpgmm::d3d12 { void AddResidencyLockRef(); void ReleaseResidencyLock(); - ComPtr mResidencyManager; + ComPtr mResidencyManager; ComPtr mPageable; DXGI_MEMORY_SEGMENT_GROUP mHeapSegment; diff --git a/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp b/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp index 87ca3775..7ed2b3dc 100644 --- a/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp +++ b/src/gpgmm/d3d12/ResidencyManagerD3D12.cpp @@ -194,7 +194,7 @@ namespace gpgmm::d3d12 { } // Increments number of locks on a heap to ensure the heap remains resident. - HRESULT ResidencyManager::LockHeap(IResidencyHeap* pHeap) { + HRESULT ResidencyManager::LockHeap(ResidencyHeap* pHeap) { GPGMM_RETURN_IF_NULL(pHeap); std::lock_guard lock(mMutex); @@ -234,7 +234,7 @@ namespace gpgmm::d3d12 { // Decrements number of locks on a heap. When the number of locks becomes zero, the heap is // inserted into the LRU cache and becomes eligible for eviction. - HRESULT ResidencyManager::UnlockHeap(IResidencyHeap* pHeap) { + HRESULT ResidencyManager::UnlockHeap(ResidencyHeap* pHeap) { GPGMM_RETURN_IF_NULL(pHeap); std::lock_guard lock(mMutex); diff --git a/src/gpgmm/d3d12/ResidencyManagerD3D12.h b/src/gpgmm/d3d12/ResidencyManagerD3D12.h index bd30a1e0..d6ce4069 100644 --- a/src/gpgmm/d3d12/ResidencyManagerD3D12.h +++ b/src/gpgmm/d3d12/ResidencyManagerD3D12.h @@ -50,8 +50,6 @@ namespace gpgmm::d3d12 { ~ResidencyManager() override; // IResidencyManager interface - HRESULT LockHeap(IResidencyHeap* pHeap) override; - HRESULT UnlockHeap(IResidencyHeap* pHeap) override; HRESULT ExecuteCommandLists(ID3D12CommandQueue* pQueue, ID3D12CommandList* const* ppCommandLists, IResidencyList* const* ppResidencyLists, @@ -74,6 +72,9 @@ namespace gpgmm::d3d12 { LPCWSTR GetDebugName() const override; HRESULT SetDebugName(LPCWSTR Name) override; + HRESULT LockHeap(ResidencyHeap* pHeap); + HRESULT UnlockHeap(ResidencyHeap* pHeap); + private: friend ResidencyHeap; friend ResourceAllocator; diff --git a/src/mvi/gpgmm_d3d12.cpp b/src/mvi/gpgmm_d3d12.cpp index 936e66bc..38ef48c1 100644 --- a/src/mvi/gpgmm_d3d12.cpp +++ b/src/mvi/gpgmm_d3d12.cpp @@ -97,6 +97,14 @@ namespace gpgmm::d3d12 { return {GetSize(), GetAlignment(), false, RESIDENCY_HEAP_STATUS_UNKNOWN}; } + HRESULT ResidencyHeap::Lock() { + return S_OK; + } + + HRESULT ResidencyHeap::Unlock() { + return S_OK; + } + HRESULT ResidencyHeap::GetResidencyManager(IResidencyManager** ppResidencyManagerOut) const { return E_NOTIMPL; } @@ -176,14 +184,6 @@ namespace gpgmm::d3d12 { ResidencyManager::~ResidencyManager() = default; - HRESULT ResidencyManager::LockHeap(IResidencyHeap* pHeap) { - return S_OK; - } - - HRESULT ResidencyManager::UnlockHeap(IResidencyHeap* pHeap) { - return S_OK; - } - HRESULT ResidencyManager::ExecuteCommandLists(ID3D12CommandQueue* pQueue, ID3D12CommandList* const* ppCommandLists, IResidencyList* const* ppResidencyLists, diff --git a/src/mvi/gpgmm_d3d12.h b/src/mvi/gpgmm_d3d12.h index d15be7b8..51d13228 100644 --- a/src/mvi/gpgmm_d3d12.h +++ b/src/mvi/gpgmm_d3d12.h @@ -72,6 +72,8 @@ namespace gpgmm::d3d12 { // IResidencyHeap interface RESIDENCY_HEAP_INFO GetInfo() const override; + HRESULT Lock() override; + HRESULT Unlock() override; HRESULT GetResidencyManager(IResidencyManager** ppResidencyManagerOut) const override; // IUnknown interface @@ -114,8 +116,6 @@ namespace gpgmm::d3d12 { ~ResidencyManager() override; // IResidencyManager interface - HRESULT LockHeap(IResidencyHeap* pHeap) override; - HRESULT UnlockHeap(IResidencyHeap* pHeap) override; HRESULT ExecuteCommandLists(ID3D12CommandQueue* pQueue, ID3D12CommandList* const* ppCommandLists, IResidencyList* const* ppResidencyLists, diff --git a/src/tests/end2end/D3D12ResidencyManagerTests.cpp b/src/tests/end2end/D3D12ResidencyManagerTests.cpp index 1593713e..2b71c1b6 100644 --- a/src/tests/end2end/D3D12ResidencyManagerTests.cpp +++ b/src/tests/end2end/D3D12ResidencyManagerTests.cpp @@ -203,14 +203,14 @@ TEST_F(D3D12ResidencyManagerTests, CreateResourceHeapLocked) { CreateResourceHeapCallbackContext::CreateHeap, &createHeapContext, nullptr)); - ASSERT_SUCCEEDED(residencyManager->LockHeap(resourceHeap.Get())); + ASSERT_SUCCEEDED(resourceHeap->Lock()); // Unlocking a heap with another lock must return S_FALSE. - EXPECT_EQ(residencyManager->UnlockHeap(resourceHeap.Get()), S_FALSE); + EXPECT_EQ(resourceHeap->Unlock(), S_FALSE); EXPECT_TRUE(resourceHeap->GetInfo().IsLocked); // But unlocking the last lock must return S_OK. - EXPECT_EQ(residencyManager->UnlockHeap(resourceHeap.Get()), S_OK); + EXPECT_EQ(resourceHeap->Unlock(), S_OK); EXPECT_FALSE(resourceHeap->GetInfo().IsLocked); } @@ -333,7 +333,7 @@ TEST_F(D3D12ResidencyManagerTests, CreateResourceHeap) { ASSERT_SUCCEEDED(resourceHeap.As(&heap)); EXPECT_NE(heap, nullptr); - ASSERT_SUCCEEDED(residencyManager->LockHeap(resourceHeap.Get())); + ASSERT_SUCCEEDED(resourceHeap->Lock()); EXPECT_EQ(resourceHeap->GetInfo().Status, gpgmm::d3d12::RESIDENCY_HEAP_STATUS_RESIDENT); EXPECT_EQ(resourceHeap->GetInfo().IsLocked, true); @@ -341,7 +341,7 @@ TEST_F(D3D12ResidencyManagerTests, CreateResourceHeap) { EXPECT_EQ(GetStats(residencyManager).CurrentHeapUsage, heapDesc.SizeInBytes); EXPECT_EQ(GetStats(residencyManager).CurrentHeapCount, 1u); - ASSERT_SUCCEEDED(residencyManager->UnlockHeap(resourceHeap.Get())); + ASSERT_SUCCEEDED(resourceHeap->Unlock()); EXPECT_EQ(resourceHeap->GetInfo().Status, gpgmm::d3d12::RESIDENCY_HEAP_STATUS_RESIDENT); EXPECT_EQ(resourceHeap->GetInfo().IsLocked, false); @@ -350,7 +350,7 @@ TEST_F(D3D12ResidencyManagerTests, CreateResourceHeap) { EXPECT_EQ(GetStats(residencyManager).CurrentHeapUsage, heapDesc.SizeInBytes); EXPECT_EQ(GetStats(residencyManager).CurrentHeapCount, 1u); - ASSERT_SUCCEEDED(residencyManager->UnlockHeap(resourceHeap.Get())); // Not locked + ASSERT_SUCCEEDED(resourceHeap->Unlock()); // Not locked } TEST_F(D3D12ResidencyManagerTests, CreateDescriptorHeap) { @@ -387,7 +387,7 @@ TEST_F(D3D12ResidencyManagerTests, CreateDescriptorHeap) { EXPECT_EQ(GetStats(residencyManager).CurrentHeapUsage, 0u); EXPECT_EQ(GetStats(residencyManager).CurrentHeapCount, 0u); - ASSERT_SUCCEEDED(residencyManager->LockHeap(descriptorHeap.Get())); + ASSERT_SUCCEEDED(descriptorHeap->Lock()); EXPECT_EQ(descriptorHeap->GetInfo().Status, gpgmm::d3d12::RESIDENCY_HEAP_STATUS_RESIDENT); EXPECT_EQ(descriptorHeap->GetInfo().IsLocked, true); @@ -395,7 +395,7 @@ TEST_F(D3D12ResidencyManagerTests, CreateDescriptorHeap) { EXPECT_EQ(GetStats(residencyManager).CurrentHeapUsage, descriptorHeapDesc.SizeInBytes); EXPECT_EQ(GetStats(residencyManager).CurrentHeapCount, 1u); - ASSERT_SUCCEEDED(residencyManager->UnlockHeap(descriptorHeap.Get())); + ASSERT_SUCCEEDED(descriptorHeap->Unlock()); EXPECT_EQ(descriptorHeap->GetInfo().Status, gpgmm::d3d12::RESIDENCY_HEAP_STATUS_RESIDENT); EXPECT_EQ(descriptorHeap->GetInfo().IsLocked, false); @@ -404,7 +404,7 @@ TEST_F(D3D12ResidencyManagerTests, CreateDescriptorHeap) { EXPECT_EQ(GetStats(residencyManager).CurrentHeapUsage, descriptorHeapDesc.SizeInBytes); EXPECT_EQ(GetStats(residencyManager).CurrentHeapCount, 1u); - ASSERT_SUCCEEDED(residencyManager->UnlockHeap(descriptorHeap.Get())); + ASSERT_SUCCEEDED(descriptorHeap->Unlock()); } TEST_F(D3D12ResidencyManagerTests, CreateDescriptorHeapResident) { @@ -571,8 +571,8 @@ TEST_F(D3D12ResidencyManagerTests, GetResidencyManager) { EXPECT_REFCOUNT_EQ(residencyManager.Get(), 4); // Use the resource manager object from the new pointer. - EXPECT_SUCCEEDED(residencyManagerAgain->LockHeap(allocationWithResidency->GetMemory())); - EXPECT_SUCCEEDED(residencyManagerAgain->UnlockHeap(allocationWithResidency->GetMemory())); + EXPECT_SUCCEEDED(allocationWithResidency->GetMemory()->Lock()); + EXPECT_SUCCEEDED(allocationWithResidency->GetMemory()->Unlock()); // Getting a NULL pointer to a residency manager cannot claim ownership. EXPECT_SUCCEEDED(allocationWithResidency->GetMemory()->GetResidencyManager(nullptr)); @@ -811,7 +811,7 @@ TEST_F(D3D12ResidencyManagerTests, OverBudgetWithLockedHeaps) { ASSERT_SUCCEEDED(resourceAllocator->CreateResource( bufferAllocationDesc, bufferDesc, D3D12_RESOURCE_STATE_COMMON, nullptr, &allocation)); - ASSERT_SUCCEEDED(residencyManager->LockHeap(allocation->GetMemory())); + ASSERT_SUCCEEDED(allocation->GetMemory()->Lock()); allocationsBelowBudget.push_back(std::move(allocation)); } @@ -834,7 +834,7 @@ TEST_F(D3D12ResidencyManagerTests, OverBudgetWithLockedHeaps) { // Unlocked allocations should be always eligable for eviction. for (auto& allocation : allocationsBelowBudget) { - ASSERT_SUCCEEDED(residencyManager->UnlockHeap(allocation->GetMemory())); + ASSERT_SUCCEEDED(allocation->GetMemory()->Unlock()); } ASSERT_SUCCEEDED(resourceAllocator->CreateResource(