From 71b239a97e97ee3589f90d817f557122c9430681 Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Mon, 24 Sep 2018 09:36:35 -0700 Subject: Reworked command buffer recycling in Vk backend using release queues --- .../include/DeviceContextVkImpl.h | 18 ++- .../VulkanUtilities/VulkanCommandBufferPool.h | 14 +-- .../src/DeviceContextVkImpl.cpp | 121 ++++++++++++--------- .../src/RenderDeviceVkImpl.cpp | 4 +- .../VulkanUtilities/VulkanCommandBufferPool.cpp | 57 +++------- 5 files changed, 99 insertions(+), 115 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h index da7ebe7e..086e0bbd 100644 --- a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h +++ b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h @@ -164,7 +164,7 @@ public: } virtual void FinishFrame(bool ForceRelease)override final; - void FinishFrame(Uint64 CompletedFenceValue); + void FinishFrame(); VkDescriptorSet AllocateDynamicDescriptorSet(VkDescriptorSetLayout SetLayout) { @@ -186,9 +186,8 @@ private: void CommitScissorRects(); inline void EnsureVkCmdBuffer(); - inline void DisposeVkCmdBuffer(VkCommandBuffer vkCmdBuff, Uint64 FenceValue); - inline void DisposeCurrentCmdBuffer(Uint64 FenceValue); - void ReleaseStaleContextResources(Uint64 SubmittedFenceValue, Uint64 CompletedFenceValue); + inline void DisposeVkCmdBuffer(Uint32 CmdQueue, VkCommandBuffer vkCmdBuff, Uint64 FenceValue); + inline void DisposeCurrentCmdBuffer(Uint32 CmdQueue, Uint64 FenceValue); struct BufferToTextureCopyInfo { @@ -250,8 +249,6 @@ private: // allocated by the context during the frame when FinishFrame() is called. Uint64 m_SubmittedBuffersCmdQueueMask = 0; - VulkanUtilities::VulkanCommandBufferPool m_CmdPool; - // Semaphores are not owned by the command context std::vector m_WaitSemaphores; std::vector m_WaitDstStageMasks; @@ -290,15 +287,14 @@ private: std::unordered_map m_MappedTextures; - - VulkanUploadHeap m_UploadHeap; - DynamicDescriptorSetAllocator m_DynamicDescrSetAllocator; + VulkanUtilities::VulkanCommandBufferPool m_CmdPool; + VulkanUploadHeap m_UploadHeap; + VulkanDynamicHeap m_DynamicHeap; + DynamicDescriptorSetAllocator m_DynamicDescrSetAllocator; Atomics::AtomicInt64 m_ContextFrameNumber; - Uint64 m_LastSubmittedFenceValue = 0; PipelineLayout::DescriptorSetBindInfo m_DescrSetBindInfo; - VulkanDynamicHeap m_DynamicHeap; std::shared_ptr m_GenerateMipsHelper; RefCntAutoPtr m_GenerateMipsSRB; diff --git a/Graphics/GraphicsEngineVulkan/include/VulkanUtilities/VulkanCommandBufferPool.h b/Graphics/GraphicsEngineVulkan/include/VulkanUtilities/VulkanCommandBufferPool.h index 98edc3d8..6023a42b 100644 --- a/Graphics/GraphicsEngineVulkan/include/VulkanUtilities/VulkanCommandBufferPool.h +++ b/Graphics/GraphicsEngineVulkan/include/VulkanUtilities/VulkanCommandBufferPool.h @@ -37,8 +37,7 @@ namespace VulkanUtilities public: VulkanCommandBufferPool(std::shared_ptr LogicalDevice, uint32_t queueFamilyIndex, - VkCommandPoolCreateFlags flags, - bool IsThreadSafe); + VkCommandPoolCreateFlags flags); VulkanCommandBufferPool (const VulkanCommandBufferPool&) = delete; VulkanCommandBufferPool (VulkanCommandBufferPool&&) = delete; @@ -47,8 +46,8 @@ namespace VulkanUtilities ~VulkanCommandBufferPool(); - VkCommandBuffer GetCommandBuffer(uint64_t LastCompletedFence, const char* DebugName = ""); - void DisposeCommandBuffer(VkCommandBuffer CmdBuffer, uint64_t FenceValue); + VkCommandBuffer GetCommandBuffer(const char* DebugName = ""); + void FreeCommandBuffer(VkCommandBuffer&& CmdBuffer); CommandPoolWrapper&& Release(); @@ -57,12 +56,7 @@ namespace VulkanUtilities std::shared_ptr m_LogicalDevice; CommandPoolWrapper m_CmdPool; - const bool m_IsThreadSafe; - std::mutex m_Mutex; - // fist - the fence value associated with the command buffer when it was executed - // second - the command buffer - typedef std::pair QueueElemType; - std::deque< QueueElemType > m_DiscardedCmdBuffers; + std::deque< VkCommandBuffer > m_CmdBuffers; }; } diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp index f7d1a054..5841e37c 100644 --- a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp @@ -52,19 +52,19 @@ namespace Diligent Uint32 ContextId, Uint32 CommandQueueId, std::shared_ptr GenerateMipsHelper) : - TDeviceContextBase{pRefCounters, pDeviceVkImpl, bIsDeferred}, - m_NumCommandsToFlush{bIsDeferred ? std::numeric_limits::max() : Attribs.NumCommandsToFlushCmdBuffer}, - m_CmdListAllocator{ GetRawAllocator(), sizeof(CommandListVkImpl), 64 }, - m_ContextId{ContextId}, - m_CommandQueueId{CommandQueueId}, + TDeviceContextBase {pRefCounters, pDeviceVkImpl, bIsDeferred}, + m_NumCommandsToFlush {bIsDeferred ? std::numeric_limits::max() : Attribs.NumCommandsToFlushCmdBuffer}, + m_CmdListAllocator { GetRawAllocator(), sizeof(CommandListVkImpl), 64 }, + m_ContextId {ContextId}, + m_CommandQueueId {CommandQueueId}, m_SubmittedBuffersCmdQueueMask{bIsDeferred ? 0 : Uint64{1} << CommandQueueId}, - // Command pools for deferred contexts must be thread safe because finished command buffers are executed and released from another thread + // Command pools must be thread safe because command buffers are returned into pools by release queues + // potentially running in another thread m_CmdPool { pDeviceVkImpl->GetLogicalDevice().GetSharedPtr(), pDeviceVkImpl->GetCommandQueue(CommandQueueId).GetQueueFamilyIndex(), - VK_COMMAND_POOL_CREATE_TRANSIENT_BIT | VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT, - bIsDeferred + VK_COMMAND_POOL_CREATE_TRANSIENT_BIT | VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT }, // Upload heap must always be thread-safe as Finish() may be called from another thread m_UploadHeap @@ -73,7 +73,6 @@ namespace Diligent GetContextObjectName("Upload heap", bIsDeferred, ContextId), Attribs.UploadHeapPageSize }, - m_ContextFrameNumber(0), m_DynamicHeap { pDeviceVkImpl->GetDynamicMemoryManager(), @@ -85,6 +84,7 @@ namespace Diligent pDeviceVkImpl->GetDynamicDescriptorPool(), GetContextObjectName("Dynamic descriptor set allocator", bIsDeferred, ContextId), }, + m_ContextFrameNumber(0), m_GenerateMipsHelper(std::move(GenerateMipsHelper)) { m_GenerateMipsHelper->CreateSRB(&m_GenerateMipsSRB); @@ -113,13 +113,8 @@ namespace Diligent Flush(); } - // We must now wait for GPU to finish so that we can safely destroy all context resources. - // We need to idle when destroying deferred contexts as well since some resources may still be in use. - // Also, upload allocation for instance reference the upload heap, so it must be alive when these - // allocations are destroyed by the release queue. - pDeviceVkImpl->IdleGPU(true); - - DisposeCurrentCmdBuffer(m_LastSubmittedFenceValue); + // TODO: remove + //DisposeCurrentCmdBuffer(m_CommandQueueId, m_LastSubmittedFenceValue); // There must be no resources in the stale resource list. For immediate context, all stale resources must have been // moved to the release queue by Flush(). For deferred contexts, this should have happened in the last FinishCommandList() @@ -127,14 +122,15 @@ namespace Diligent VERIFY(m_UploadHeap.GetStalePagesCount() == 0, "All stale pages must have been discarded at this point"); VERIFY(m_DynamicDescrSetAllocator.GetAllocatedPoolCount() == 0, "All allocated dynamic descriptor set pools must have been released at this point"); - // TODO: rework - ReleaseStaleContextResources(m_LastSubmittedFenceValue, pDeviceVkImpl->GetCompletedFenceValue(0)); - // Since we idled the GPU, all stale resources must have been destroyed now - // TODO: remove - //VERIFY(m_DynamicDescriptorPool.GetPendingReleaseAllocationCount() == 0, "All stale descriptor set allocations must have been destroyed at this point"); - auto VkCmdPool = m_CmdPool.Release(); pDeviceVkImpl->SafeReleaseDeviceObject(std::move(VkCmdPool), ~Uint64{0}); + + // TODO: + // We must now wait for GPU to finish so that we can safely destroy all context resources. + // We need to idle when destroying deferred contexts as well since some resources may still be in use. + // Also, upload allocation for instance reference the upload heap, so it must be alive when these + // allocations are destroyed by the release queue. + pDeviceVkImpl->IdleGPU(true); } IMPLEMENT_QUERY_INTERFACE( DeviceContextVkImpl, IID_DeviceContextVk, TDeviceContextBase ) @@ -146,26 +142,61 @@ namespace Diligent m_State.NumCommands = m_State.NumCommands != 0 ? m_State.NumCommands : 1; if (m_CommandBuffer.GetVkCmdBuffer() == VK_NULL_HANDLE) { - auto pDeviceVkImpl = m_pDevice.RawPtr(); - // TODO: rework - auto vkCmdBuff = m_CmdPool.GetCommandBuffer(pDeviceVkImpl->GetCompletedFenceValue(0)); + auto vkCmdBuff = m_CmdPool.GetCommandBuffer(); m_CommandBuffer.SetVkCmdBuffer(vkCmdBuff); } } - void DeviceContextVkImpl::DisposeVkCmdBuffer(VkCommandBuffer vkCmdBuff, Uint64 FenceValue) + void DeviceContextVkImpl::DisposeVkCmdBuffer(Uint32 CmdQueue, VkCommandBuffer vkCmdBuff, Uint64 FenceValue) { VERIFY_EXPR(vkCmdBuff != VK_NULL_HANDLE); - m_CmdPool.DisposeCommandBuffer(vkCmdBuff, FenceValue); + auto& DeviceVkImpl = *m_pDevice.RawPtr(); + auto& ReleaseQueue = DeviceVkImpl.GetReleaseQueue(CmdQueue); + class CmdBufferDeleter + { + public: + CmdBufferDeleter(VkCommandBuffer _vkCmdBuff, + VulkanUtilities::VulkanCommandBufferPool& _Pool) : + vkCmdBuff(_vkCmdBuff), + Pool (&_Pool) + { + VERIFY_EXPR(vkCmdBuff != VK_NULL_HANDLE); + } + CmdBufferDeleter (const CmdBufferDeleter&) = delete; + CmdBufferDeleter& operator = (const CmdBufferDeleter&) = delete; + CmdBufferDeleter& operator = ( CmdBufferDeleter&&) = delete; + + CmdBufferDeleter(CmdBufferDeleter&& rhs) : + vkCmdBuff(rhs.vkCmdBuff), + Pool (rhs.Pool) + { + rhs.vkCmdBuff = VK_NULL_HANDLE; + rhs.Pool = nullptr; + } + + ~CmdBufferDeleter() + { + if(Pool != nullptr) + { + Pool->FreeCommandBuffer(std::move(vkCmdBuff)); + } + } + + private: + VkCommandBuffer vkCmdBuff; + VulkanUtilities::VulkanCommandBufferPool* Pool; + }; + + ReleaseQueue.DiscardResource(CmdBufferDeleter{vkCmdBuff, m_CmdPool}, FenceValue); } - inline void DeviceContextVkImpl::DisposeCurrentCmdBuffer(Uint64 FenceValue) + inline void DeviceContextVkImpl::DisposeCurrentCmdBuffer(Uint32 CmdQueue, Uint64 FenceValue) { VERIFY(m_CommandBuffer.GetState().RenderPass == VK_NULL_HANDLE, "Disposing command buffer with unifinished render pass"); auto vkCmdBuff = m_CommandBuffer.GetVkCmdBuffer(); if (vkCmdBuff != VK_NULL_HANDLE) { - DisposeVkCmdBuffer(vkCmdBuff, FenceValue); + DisposeVkCmdBuffer(CmdQueue, vkCmdBuff, FenceValue); m_CommandBuffer.Reset(); } } @@ -719,10 +750,10 @@ namespace Diligent void DeviceContextVkImpl::FinishFrame(bool ForceRelease) { // TODO: rework - FinishFrame(ForceRelease ? std::numeric_limits::max() : m_pDevice.RawPtr()->GetCompletedFenceValue(0)); + FinishFrame(); } - void DeviceContextVkImpl::FinishFrame(Uint64 CompletedFenceValue) + void DeviceContextVkImpl::FinishFrame() { if(GetNumCommandsInCtx() != 0) LOG_ERROR_MESSAGE(m_bIsDeferred ? @@ -752,12 +783,6 @@ namespace Diligent Atomics::AtomicIncrement(m_ContextFrameNumber); } - void DeviceContextVkImpl::ReleaseStaleContextResources(Uint64 SubmittedFenceValue, Uint64 CompletedFenceValue) - { - //m_DynamicDescriptorPool.DisposeAllocations(SubmittedFenceValue); - //m_DynamicDescriptorPool.ReleaseStaleAllocations(CompletedFenceValue); - } - void DeviceContextVkImpl::Flush() { #ifdef DEVELOPMENT @@ -794,14 +819,14 @@ namespace Diligent SubmitInfo.waitSemaphoreCount = static_cast(m_WaitSemaphores.size()); VERIFY_EXPR(m_WaitSemaphores.size() == m_WaitDstStageMasks.size()); - SubmitInfo.pWaitSemaphores = SubmitInfo.waitSemaphoreCount != 0 ? m_WaitSemaphores.data() : nullptr; - SubmitInfo.pWaitDstStageMask = SubmitInfo.waitSemaphoreCount != 0 ? m_WaitDstStageMasks.data() : nullptr; + SubmitInfo.pWaitSemaphores = SubmitInfo.waitSemaphoreCount != 0 ? m_WaitSemaphores.data() : nullptr; + SubmitInfo.pWaitDstStageMask = SubmitInfo.waitSemaphoreCount != 0 ? m_WaitDstStageMasks.data() : nullptr; SubmitInfo.signalSemaphoreCount = static_cast(m_SignalSemaphores.size()); - SubmitInfo.pSignalSemaphores = SubmitInfo.signalSemaphoreCount != 0 ? m_SignalSemaphores.data() : nullptr; + SubmitInfo.pSignalSemaphores = SubmitInfo.signalSemaphoreCount != 0 ? m_SignalSemaphores.data() : nullptr; // Submit command buffer even if there are no commands to release stale resources. //if (SubmitInfo.commandBufferCount != 0 || SubmitInfo.waitSemaphoreCount !=0 || SubmitInfo.signalSemaphoreCount != 0) - m_LastSubmittedFenceValue = pDeviceVkImpl->ExecuteCommandBuffer(m_CommandQueueId, SubmitInfo, this, &m_PendingFences); + auto SubmittedFenceValue = pDeviceVkImpl->ExecuteCommandBuffer(m_CommandQueueId, SubmitInfo, this, &m_PendingFences); m_WaitSemaphores.clear(); m_WaitDstStageMasks.clear(); @@ -810,14 +835,9 @@ namespace Diligent if (vkCmdBuff != VK_NULL_HANDLE) { - DisposeCurrentCmdBuffer(m_LastSubmittedFenceValue); + DisposeCurrentCmdBuffer(m_CommandQueueId, SubmittedFenceValue); } - // Release temporary resources that were used by this context while recording the last command buffer - // TODO: rework - auto CompletedFenceValue = pDeviceVkImpl->GetCompletedFenceValue(0); - ReleaseStaleContextResources(m_LastSubmittedFenceValue, CompletedFenceValue); - m_State = ContextState{}; m_DescrSetBindInfo.Reset(); m_CommandBuffer.Reset(); @@ -1427,16 +1447,11 @@ namespace Diligent VERIFY_EXPR(m_PendingFences.empty()); auto pDeferredCtxVkImpl = pDeferredCtx.RawPtr(); auto SubmittedFenceValue = pDeviceVkImpl->ExecuteCommandBuffer(m_CommandQueueId, SubmitInfo, this, nullptr); - pDeferredCtxVkImpl->m_LastSubmittedFenceValue = SubmittedFenceValue; // Set the bit in the deferred context cmd queue mask corresponding to cmd queue of this context pDeferredCtxVkImpl->m_SubmittedBuffersCmdQueueMask |= Uint64{1} << m_CommandQueueId; // It is OK to dispose command buffer from another thread. We are not going to // record any commands and only need to add the buffer to the queue - pDeferredCtxVkImpl->DisposeVkCmdBuffer(vkCmdBuff, SubmittedFenceValue); - // We can now release all temporary resources in the deferred context associated with the submitted command list - // TODO: rework - auto CompletedFenceValue = pDeviceVkImpl->GetCompletedFenceValue(0); - pDeferredCtxVkImpl->ReleaseStaleContextResources(SubmittedFenceValue, CompletedFenceValue); + pDeferredCtxVkImpl->DisposeVkCmdBuffer(m_CommandQueueId, vkCmdBuff, SubmittedFenceValue); } void DeviceContextVkImpl::SignalFence(IFence* pFence, Uint64 Value) diff --git a/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp index f5947039..6831edf8 100644 --- a/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp @@ -230,7 +230,9 @@ void RenderDeviceVkImpl::ExecuteAndDisposeTransientCmdBuff(Uint32 QueueIndex, Vk CommandPoolDeleter(CommandPoolManager& _CmdPoolMgr, VulkanUtilities::CommandPoolWrapper&& _Pool) : CmdPoolMgr(&_CmdPoolMgr), Pool (std::move(_Pool)) - {} + { + VERIFY_EXPR(Pool != VK_NULL_HANDLE); + } CommandPoolDeleter (const CommandPoolDeleter&) = delete; CommandPoolDeleter& operator = (const CommandPoolDeleter&) = delete; diff --git a/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanCommandBufferPool.cpp b/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanCommandBufferPool.cpp index 6929fc14..91a1b0a6 100644 --- a/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanCommandBufferPool.cpp +++ b/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanCommandBufferPool.cpp @@ -32,10 +32,8 @@ namespace VulkanUtilities { VulkanCommandBufferPool::VulkanCommandBufferPool(std::shared_ptr LogicalDevice, uint32_t queueFamilyIndex, - VkCommandPoolCreateFlags flags, - bool IsThreadSafe) : - m_LogicalDevice(LogicalDevice), - m_IsThreadSafe(IsThreadSafe) + VkCommandPoolCreateFlags flags) : + m_LogicalDevice(LogicalDevice) { VkCommandPoolCreateInfo CmdPoolCI = {}; CmdPoolCI.sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO; @@ -51,41 +49,22 @@ namespace VulkanUtilities m_CmdPool.Release(); } - VkCommandBuffer VulkanCommandBufferPool::GetCommandBuffer(uint64_t LastCompletedFence, const char* DebugName) + VkCommandBuffer VulkanCommandBufferPool::GetCommandBuffer(const char* DebugName) { VkCommandBuffer CmdBuffer = VK_NULL_HANDLE; { - std::unique_lock Lock(m_Mutex, std::defer_lock); - if (m_IsThreadSafe) - Lock.lock(); + std::lock_guard Lock(m_Mutex); - if (!m_DiscardedCmdBuffers.empty()) + if (!m_CmdBuffers.empty()) { - // Pick the oldest cmd buffer at the front of the deque - // If this buffer is not yet available, there is no point in - // looking at other buffers since they were released even - // later - auto& OldestBuff = m_DiscardedCmdBuffers.front(); - - // Note that LastCompletedFence 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 - // cmd buffer may not be used. - - // OldestBuff.first is the fence value that was signaled AFTER the - // command buffer has been submitted. If LastCompletedFence is at least - // this value, the buffer can be safely reused - if (LastCompletedFence >= OldestBuff.first) - { - CmdBuffer = OldestBuff.second; - auto err = vkResetCommandBuffer(CmdBuffer, - 0 // VK_COMMAND_BUFFER_RESET_RELEASE_RESOURCES_BIT - specifies that most or all memory resources currently - // owned by the command buffer should be returned to the parent command pool. - ); - VERIFY(err == VK_SUCCESS, "Failed to reset command buffer"); - m_DiscardedCmdBuffers.pop_front(); - } + CmdBuffer = m_CmdBuffers.front(); + auto err = vkResetCommandBuffer(CmdBuffer, + 0 // VK_COMMAND_BUFFER_RESET_RELEASE_RESOURCES_BIT - specifies that most or all memory resources currently + // owned by the command buffer should be returned to the parent command pool. + ); + DEV_CHECK_ERR(err == VK_SUCCESS, "Failed to reset command buffer"); + m_CmdBuffers.pop_front(); } } @@ -115,21 +94,19 @@ namespace VulkanUtilities return CmdBuffer; } - void VulkanCommandBufferPool::DisposeCommandBuffer(VkCommandBuffer CmdBuffer, uint64_t FenceValue) + void VulkanCommandBufferPool::FreeCommandBuffer(VkCommandBuffer&& CmdBuffer) { - std::unique_lock Lock(m_Mutex, std::defer_lock); - if (m_IsThreadSafe) - Lock.lock(); - + std::lock_guard Lock(m_Mutex); // FenceValue is the value that was signaled by the command queue after it // executed the command buffer - m_DiscardedCmdBuffers.emplace_back(FenceValue, CmdBuffer); + m_CmdBuffers.emplace_back(CmdBuffer); + CmdBuffer = VK_NULL_HANDLE; } CommandPoolWrapper&& VulkanCommandBufferPool::Release() { m_LogicalDevice.reset(); - m_DiscardedCmdBuffers.clear(); + m_CmdBuffers.clear(); return std::move(m_CmdPool); } } -- cgit v1.2.3