From 3cd4cbbd0cd34ca8e48d1473bb349fff6d7050c5 Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Wed, 3 Oct 2018 22:51:27 -0700 Subject: Reworked command list management in D3D12 backend --- .../GraphicsEngineD3D12/include/CommandContext.h | 6 +- .../include/CommandListD3D12Impl.h | 2 +- .../include/CommandListManager.h | 44 ++++++----- .../GraphicsEngineD3D12/src/CommandContext.cpp | 20 +++-- .../GraphicsEngineD3D12/src/CommandListManager.cpp | 89 +++++++++++++--------- .../src/DeviceContextD3D12Impl.cpp | 4 +- .../src/RenderDeviceD3D12Impl.cpp | 22 +++--- 7 files changed, 105 insertions(+), 82 deletions(-) (limited to 'Graphics/GraphicsEngineD3D12') diff --git a/Graphics/GraphicsEngineD3D12/include/CommandContext.h b/Graphics/GraphicsEngineD3D12/include/CommandContext.h index 13fd65eb..717354f3 100644 --- a/Graphics/GraphicsEngineD3D12/include/CommandContext.h +++ b/Graphics/GraphicsEngineD3D12/include/CommandContext.h @@ -61,11 +61,11 @@ public: CommandContext( class CommandListManager& CmdListManager); - ~CommandContext(void); + ~CommandContext(); // Submit the command buffer and reset it. This is encouraged to keep the GPU busy and reduce latency. // Taking too long to build command lists and submit them can idle the GPU. - ID3D12GraphicsCommandList* Close(ID3D12CommandAllocator **ppAllocator); + ID3D12GraphicsCommandList* Close(CComPtr& pAllocator); void Reset( CommandListManager& CmdListManager ); class GraphicsContext& AsGraphicsContext(); @@ -134,7 +134,7 @@ protected: void InsertAliasBarrier(D3D12ResourceBase& Before, D3D12ResourceBase& After, IDeviceObject &BeforeObj, IDeviceObject &AfterObj, bool FlushImmediate = false); CComPtr m_pCommandList; - CComPtr m_pCurrentAllocator; + CComPtr m_pCurrentAllocator; ID3D12PipelineState* m_pCurPipelineState = nullptr; ID3D12RootSignature* m_pCurGraphicsRootSignature = nullptr; diff --git a/Graphics/GraphicsEngineD3D12/include/CommandListD3D12Impl.h b/Graphics/GraphicsEngineD3D12/include/CommandListD3D12Impl.h index e3221fe4..b9c43692 100644 --- a/Graphics/GraphicsEngineD3D12/include/CommandListD3D12Impl.h +++ b/Graphics/GraphicsEngineD3D12/include/CommandListD3D12Impl.h @@ -53,7 +53,7 @@ public: ~CommandListD3D12Impl() { - VERIFY(m_pCmdContext == nullptr && m_pDeferredCtx == nullptr, "Destroying a command list that has not been executed"); + DEV_CHECK_ERR(m_pCmdContext == nullptr && m_pDeferredCtx == nullptr, "Destroying a command list that has not been executed"); } void Close(CommandContext*& pCmdContext, RefCntAutoPtr& pDeferredCtx) diff --git a/Graphics/GraphicsEngineD3D12/include/CommandListManager.h b/Graphics/GraphicsEngineD3D12/include/CommandListManager.h index b88eb622..7f338d24 100644 --- a/Graphics/GraphicsEngineD3D12/include/CommandListManager.h +++ b/Graphics/GraphicsEngineD3D12/include/CommandListManager.h @@ -24,43 +24,49 @@ #pragma once #include -#include #include #include namespace Diligent { +class RenderDeviceD3D12Impl; + class CommandListManager { public: - CommandListManager(class RenderDeviceD3D12Impl *pDeviceD3D12); + CommandListManager(RenderDeviceD3D12Impl& DeviceD3D12Impl); ~CommandListManager(); - CommandListManager(const CommandListManager&) = delete; - CommandListManager(CommandListManager&&) = delete; - CommandListManager& operator = (const CommandListManager&) = delete; - CommandListManager& operator = (CommandListManager&&) = delete; + CommandListManager (const CommandListManager&) = delete; + CommandListManager ( CommandListManager&&) = delete; + CommandListManager& operator = (const CommandListManager&) = delete; + CommandListManager& operator = ( CommandListManager&&) = delete; void CreateNewCommandList( ID3D12GraphicsCommandList** ppList, ID3D12CommandAllocator** ppAllocator ); - // Discards the allocator. - // FenceValue is the value that was signaled by the command queue after it - // executed the the command list created by the allocator - void DiscardAllocator( Uint64 FenceValue, ID3D12CommandAllocator* pAllocator ); - void RequestAllocator(ID3D12CommandAllocator** ppAllocator); + void ReleaseAllocator(CComPtr&& Allocator, Uint32 CmdQueue, Uint64 FenceValue); -private: - // fist - the fence value associated with the command list that was created by the allocator - // second - the allocator to be discarded - typedef std::pair > DiscardedAllocatorQueueElemType; - std::deque< DiscardedAllocatorQueueElemType, STDAllocatorRawMem > m_DiscardedAllocators; + // Returns allocator to the list of available allocators. The GPU must have finished using the + // allocator + void FreeAllocator( CComPtr&& Allocator ); - std::mutex m_AllocatorMutex; - RenderDeviceD3D12Impl *m_pDeviceD3D12; +#ifdef DEVELOPMENT + Atomics::Long GetAllocatorCounter() const {return m_AllocatorCounter;} +#endif + +private: + std::mutex m_AllocatorMutex; + std::vector< CComPtr, STDAllocatorRawMem> > m_FreeAllocators; + + RenderDeviceD3D12Impl& m_DeviceD3D12Impl; Atomics::AtomicLong m_NumAllocators = 0; // For debug purposes only + +#ifdef DEVELOPMENT + Atomics::AtomicLong m_AllocatorCounter = 0; +#endif }; -} \ No newline at end of file +} diff --git a/Graphics/GraphicsEngineD3D12/src/CommandContext.cpp b/Graphics/GraphicsEngineD3D12/src/CommandContext.cpp index 5010e1c6..c0236a4a 100644 --- a/Graphics/GraphicsEngineD3D12/src/CommandContext.cpp +++ b/Graphics/GraphicsEngineD3D12/src/CommandContext.cpp @@ -45,19 +45,20 @@ CommandContext::CommandContext( CommandListManager& CmdListManager) : CommandContext::~CommandContext( void ) { + DEV_CHECK_ERR(m_pCurrentAllocator == nullptr, "Command allocator must be released prior to destroying the command context"); } - - void CommandContext::Reset( CommandListManager& CmdListManager ) { // We only call Reset() on previously freed contexts. The command list persists, but we need to - // request a new allocator if there is none - // The allocator may not be null if the command context was previously disposed without being executed + // request a new allocator VERIFY_EXPR(m_pCommandList != nullptr); if( !m_pCurrentAllocator ) { CmdListManager.RequestAllocator(&m_pCurrentAllocator); + // Unlike ID3D12CommandAllocator::Reset, ID3D12GraphicsCommandList::Reset can be called while the + // command list is still being executed. A typical pattern is to submit a command list and then + // immediately reset it to reuse the allocated memory for another command list. m_pCommandList->Reset(m_pCurrentAllocator, nullptr); } @@ -76,7 +77,7 @@ void CommandContext::Reset( CommandListManager& CmdListManager ) #endif } -ID3D12GraphicsCommandList* CommandContext::Close(ID3D12CommandAllocator** ppAllocator) +ID3D12GraphicsCommandList* CommandContext::Close(CComPtr& pAllocator) { FlushResourceBarriers(); @@ -85,15 +86,12 @@ ID3D12GraphicsCommandList* CommandContext::Close(ID3D12CommandAllocator** ppAllo VERIFY_EXPR(m_pCurrentAllocator != nullptr); auto hr = m_pCommandList->Close(); - VERIFY(SUCCEEDED(hr), "Failed to close the command list"); - - if( ppAllocator != nullptr ) - *ppAllocator = m_pCurrentAllocator.Detach(); + DEV_CHECK_ERR(SUCCEEDED(hr), "Failed to close the command list"); + + pAllocator = std::move(m_pCurrentAllocator); return m_pCommandList; } - - void GraphicsContext::SetRenderTargets( UINT NumRTVs, ITextureViewD3D12** ppRTVs, ITextureViewD3D12* pDSV ) { D3D12_CPU_DESCRIPTOR_HANDLE RTVHandles[8]; // Do not waste time initializing array to zero diff --git a/Graphics/GraphicsEngineD3D12/src/CommandListManager.cpp b/Graphics/GraphicsEngineD3D12/src/CommandListManager.cpp index 98095c1d..52ed168b 100644 --- a/Graphics/GraphicsEngineD3D12/src/CommandListManager.cpp +++ b/Graphics/GraphicsEngineD3D12/src/CommandListManager.cpp @@ -28,21 +28,22 @@ namespace Diligent { -CommandListManager::CommandListManager(RenderDeviceD3D12Impl *pDeviceD3D12) : - m_pDeviceD3D12(pDeviceD3D12), - m_DiscardedAllocators(STD_ALLOCATOR_RAW_MEM(DiscardedAllocatorQueueElemType, GetRawAllocator(), "Allocator for deque")) +CommandListManager::CommandListManager(RenderDeviceD3D12Impl& DeviceD3D12Impl) : + m_DeviceD3D12Impl(DeviceD3D12Impl), + m_FreeAllocators(STD_ALLOCATOR_RAW_MEM(CComPtr, GetRawAllocator(), "Allocator for vector>")) { } CommandListManager::~CommandListManager() { + DEV_CHECK_ERR(m_AllocatorCounter == 0, m_AllocatorCounter, " allocator(s) have not been returned to the manager. This will cause a crash if these allocators are referenced by release queues and later returned via FreeAllocator()"); + LOG_INFO_MESSAGE("Command list manager: created ", m_FreeAllocators.size(), " allocators"); } - void CommandListManager::CreateNewCommandList( ID3D12GraphicsCommandList** List, ID3D12CommandAllocator** Allocator ) { RequestAllocator(Allocator); - auto *pd3d12Device = m_pDeviceD3D12->GetD3D12Device(); + auto* pd3d12Device = m_DeviceD3D12Impl.GetD3D12Device(); auto hr = pd3d12Device->CreateCommandList(1, D3D12_COMMAND_LIST_TYPE_DIRECT, *Allocator, nullptr, __uuidof(*List), reinterpret_cast(List) ); VERIFY(SUCCEEDED(hr), "Failed to create command list"); (*List)->SetName(L"CommandList"); @@ -56,53 +57,69 @@ void CommandListManager::RequestAllocator(ID3D12CommandAllocator** ppAllocator) VERIFY( (*ppAllocator) == nullptr, "Allocator pointer is not null" ); (*ppAllocator) = nullptr; - if (!m_DiscardedAllocators.empty()) + if (!m_FreeAllocators.empty()) { - // Pick the oldest allocator at the front of the deque - // If this allocator is not yet available, there is no point in - // looking at other allocators since they were released even - // later - auto& AllocatorPair = m_DiscardedAllocators.front(); - - // TODO: Rework - // Get the last completed fence value - auto CompletedFenceValue = m_pDeviceD3D12->GetCompletedFenceValue(0); - // Note that CompletedFenceValue only grows. So if after we queried - // the value, the actual value is increased in other thread, this will not - // be an issue as the only consequence is that potentially available - // allocator may not be used. - - // AllocatorPair.first is the fence value that was signaled AFTER the - // command list has been submitted. If CompletedFenceValue is at least - // this value, the allocator can be safely reused - if ( CompletedFenceValue >= AllocatorPair.first ) - { - *ppAllocator = AllocatorPair.second.Detach(); - auto hr = (*ppAllocator)->Reset(); - VERIFY_EXPR(SUCCEEDED(hr)); - m_DiscardedAllocators.pop_front(); - } + *ppAllocator = m_FreeAllocators.back().Detach(); + auto hr = (*ppAllocator)->Reset(); + DEV_CHECK_ERR(SUCCEEDED(hr), "Failed to reset command allocator"); + m_FreeAllocators.pop_back(); } // If no allocators were ready to be reused, create a new one if ((*ppAllocator) == nullptr) { - auto *pd3d12Device = m_pDeviceD3D12->GetD3D12Device(); + auto* pd3d12Device = m_DeviceD3D12Impl.GetD3D12Device(); auto hr = pd3d12Device->CreateCommandAllocator(D3D12_COMMAND_LIST_TYPE_DIRECT, __uuidof(*ppAllocator), reinterpret_cast(ppAllocator)); VERIFY(SUCCEEDED(hr), "Failed to create command allocator"); wchar_t AllocatorName[32]; swprintf(AllocatorName, _countof(AllocatorName), L"Cmd list allocator %ld", Atomics::AtomicIncrement(m_NumAllocators)-1); (*ppAllocator)->SetName(AllocatorName); } +#ifdef DEVELOPMENT + Atomics::AtomicIncrement(m_AllocatorCounter); +#endif } -void CommandListManager::DiscardAllocator( Uint64 FenceValue, ID3D12CommandAllocator* pAllocator ) +void CommandListManager::ReleaseAllocator( CComPtr&& Allocator, Uint32 CmdQueue, Uint64 FenceValue ) { - std::lock_guard LockGuard(m_AllocatorMutex); + struct StaleAllocator + { + CComPtr Allocator; + CommandListManager* Mgr; + + StaleAllocator(CComPtr&& _Allocator, CommandListManager& _Mgr)noexcept : + Allocator (std::move(_Allocator)), + Mgr (&_Mgr) + { + } + + StaleAllocator (const StaleAllocator&) = delete; + StaleAllocator& operator= (const StaleAllocator&) = delete; + StaleAllocator& operator= ( StaleAllocator&&) = delete; + + StaleAllocator(StaleAllocator&& rhs)noexcept : + Allocator (std::move(rhs.Allocator)), + Mgr (rhs.Mgr) + { + rhs.Mgr = nullptr; + } - // FenceValue is the value that was signaled by the command queue after it - // executed the command list created by the allocator - m_DiscardedAllocators.emplace_back( FenceValue, CComPtr(pAllocator) ); + ~StaleAllocator() + { + if (Mgr != nullptr) + Mgr->FreeAllocator( std::move(Allocator) ); + } + }; + m_DeviceD3D12Impl.GetReleaseQueue(CmdQueue).DiscardResource(StaleAllocator{std::move(Allocator), *this}, FenceValue); +} + +void CommandListManager::FreeAllocator( CComPtr&& Allocator ) +{ + std::lock_guard LockGuard(m_AllocatorMutex); + m_FreeAllocators.emplace_back( std::move(Allocator) ); +#ifdef DEVELOPMENT + Atomics::AtomicDecrement(m_AllocatorCounter); +#endif } } diff --git a/Graphics/GraphicsEngineD3D12/src/DeviceContextD3D12Impl.cpp b/Graphics/GraphicsEngineD3D12/src/DeviceContextD3D12Impl.cpp index b98c526f..f2732a65 100644 --- a/Graphics/GraphicsEngineD3D12/src/DeviceContextD3D12Impl.cpp +++ b/Graphics/GraphicsEngineD3D12/src/DeviceContextD3D12Impl.cpp @@ -547,7 +547,7 @@ namespace Diligent if (m_NumCommandsInCurCtx != 0) { m_pCurrCmdCtx->FlushResourceBarriers(); - auto FenceValue = pDeviceD3D12Impl->CloseAndExecuteCommandContext(m_pCurrCmdCtx, true, &m_PendingFences); + pDeviceD3D12Impl->CloseAndExecuteCommandContext(m_pCurrCmdCtx, true, &m_PendingFences); m_PendingFences.clear(); } else @@ -1072,7 +1072,7 @@ namespace Diligent CommandContext* pCmdContext = nullptr; RefCntAutoPtr pDeferredCtx; pCmdListD3D12->Close(pCmdContext, pDeferredCtx); - auto FenceValue = m_pDevice.RawPtr()->CloseAndExecuteCommandContext(pCmdContext, true, nullptr); + m_pDevice.RawPtr()->CloseAndExecuteCommandContext(pCmdContext, true, nullptr); // Set the bit in the deferred context cmd queue mask corresponding to cmd queue of this context pDeferredCtx->m_SubmittedBuffersCmdQueueMask |= Uint64{1} << m_CommandQueueId; } diff --git a/Graphics/GraphicsEngineD3D12/src/RenderDeviceD3D12Impl.cpp b/Graphics/GraphicsEngineD3D12/src/RenderDeviceD3D12Impl.cpp index e4b07ab8..2812402d 100644 --- a/Graphics/GraphicsEngineD3D12/src/RenderDeviceD3D12Impl.cpp +++ b/Graphics/GraphicsEngineD3D12/src/RenderDeviceD3D12Impl.cpp @@ -63,7 +63,7 @@ RenderDeviceD3D12Impl :: RenderDeviceD3D12Impl(IReferenceCounters* pRef }, m_pd3d12Device (pd3d12Device), m_EngineAttribs (CreationAttribs), - m_CmdListManager(this), + m_CmdListManager(*this), m_CPUDescriptorHeaps { {RawMemAllocator, *this, CreationAttribs.CPUDescriptorHeapAllocationSize[0], D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV, D3D12_DESCRIPTOR_HEAP_FLAG_NONE}, @@ -100,6 +100,7 @@ RenderDeviceD3D12Impl::~RenderDeviceD3D12Impl() DEV_CHECK_ERR(m_DynamicMemoryManager.GetAllocatedPageCounter() == 0, "All allocated dynamic pages must have been returned to the manager at this point."); m_DynamicMemoryManager.Destroy(); + DEV_CHECK_ERR(m_CmdListManager.GetAllocatorCounter() == 0, "All allocators must have been returned to the manager at this point."); m_ContextPool.clear(); DestroyCommandQueues(); @@ -108,13 +109,17 @@ RenderDeviceD3D12Impl::~RenderDeviceD3D12Impl() void RenderDeviceD3D12Impl::DisposeCommandContext(CommandContext* pCtx) { std::lock_guard LockGuard(m_ContextAllocationMutex); + CComPtr pAllocator; + pCtx->Close(pAllocator); + // Since allocator has not been used, we can add it directly to the free allocator list + m_CmdListManager.FreeAllocator(std::move(pAllocator)); m_AvailableContexts.push_back(pCtx); } void RenderDeviceD3D12Impl::CloseAndExecuteTransientCommandContext(Uint32 CommandQueueIndex, CommandContext *pCtx) { CComPtr pAllocator; - auto *pCmdList = pCtx->Close(&pAllocator); + ID3D12GraphicsCommandList* pCmdList = pCtx->Close(pAllocator); Uint64 FenceValue = 0; // Execute command list directly through the queue to avoid interference with command list numbers in the queue LockCommandQueue(CommandQueueIndex, @@ -123,9 +128,8 @@ void RenderDeviceD3D12Impl::CloseAndExecuteTransientCommandContext(Uint32 Comman FenceValue = pCmdQueue->Submit(pCmdList); } ); - // DiscardAllocator() is thread-safe - // TODO: Rework - m_CmdListManager.DiscardAllocator(FenceValue, pAllocator); + + m_CmdListManager.ReleaseAllocator(std::move(pAllocator), CommandQueueIndex, FenceValue); { std::lock_guard LockGuard(m_ContextAllocationMutex); @@ -136,7 +140,7 @@ void RenderDeviceD3D12Impl::CloseAndExecuteTransientCommandContext(Uint32 Comman Uint64 RenderDeviceD3D12Impl::CloseAndExecuteCommandContext(CommandContext* pCtx, bool DiscardStaleObjects, std::vector > >* pSignalFences) { CComPtr pAllocator; - auto *pCmdList = pCtx->Close(&pAllocator); + ID3D12GraphicsCommandList* pCmdList = pCtx->Close(pAllocator); Uint32 QueueIndex = 0; Uint64 FenceValue = 0; @@ -172,9 +176,7 @@ Uint64 RenderDeviceD3D12Impl::CloseAndExecuteCommandContext(CommandContext* pCtx } } - // DiscardAllocator() is thread-safe - // TODO: Rework - m_CmdListManager.DiscardAllocator(FenceValue, pAllocator); + m_CmdListManager.ReleaseAllocator(std::move(pAllocator), QueueIndex, FenceValue); { std::lock_guard LockGuard(m_ContextAllocationMutex); @@ -198,7 +200,7 @@ void RenderDeviceD3D12Impl::FlushStaleResources(Uint32 CmdQueueIndex) // Submit empty command list to the queue. This will effectively signal the fence and // discard all resources ID3D12GraphicsCommandList* pNullCmdList = nullptr; - TRenderDeviceBase::SubmitCommandBuffer(0, pNullCmdList, true); + TRenderDeviceBase::SubmitCommandBuffer(CmdQueueIndex, pNullCmdList, true); } void RenderDeviceD3D12Impl::ReleaseStaleResources(bool ForceRelease) -- cgit v1.2.3