Reworked transient command buffer pool manager to use release queue

3 files changed, 74 insertions, 42 deletions

diff --git a/Graphics/GraphicsEngineVulkan/include/CommandPoolManager.h b/Graphics/GraphicsEngineVulkan/include/CommandPoolManager.h
index 2f597702..f1a62e14 100644
--- a/Graphics/GraphicsEngineVulkan/include/CommandPoolManager.h
+++ b/Graphics/GraphicsEngineVulkan/include/CommandPoolManager.h
@@ -31,12 +31,15 @@
 namespace Diligent
 {
 
+class RenderDeviceVkImpl;
+
 class CommandPoolManager
 {
 public:
-    CommandPoolManager(const VulkanUtilities::VulkanLogicalDevice& LogicalDevice, 
-                       uint32_t                                    queueFamilyIndex, 
-                       VkCommandPoolCreateFlags                    flags)noexcept;
+    CommandPoolManager(RenderDeviceVkImpl&      DeviceVkImpl, 
+                       std::string              Name,
+                       uint32_t                 queueFamilyIndex, 
+                       VkCommandPoolCreateFlags flags)noexcept;
     
     CommandPoolManager             (const CommandPoolManager&) = delete;
     CommandPoolManager             (CommandPoolManager&&)      = delete;
@@ -46,25 +49,21 @@ public:
     ~CommandPoolManager();
 
     // Allocates Vulkan command pool.
-    // The method first tries to find previously disposed command pool that can be safely reused
-    // (i.e., whose FenceValue <= CompletedFenceValue). If no buffer can be reused, the method creates
-    // a new one
-    VulkanUtilities::CommandPoolWrapper AllocateCommandPool(uint64_t CompletedFenceValue, const char *DebugName = nullptr);
+    VulkanUtilities::CommandPoolWrapper AllocateCommandPool(const char *DebugName = nullptr);
     
-    // Disposes command pool. The buffer allocated from this pool MUST have already been submitted to the queue,
-    // and FenceValue must be the value associated with this command buffer
-    void DisposeCommandPool(VulkanUtilities::CommandPoolWrapper&& CmdPool, uint64_t FenceValue);
+    // Returns command pool to the list of available pools. The GPU must have finished using the pool
+    void FreeCommandPool(VulkanUtilities::CommandPoolWrapper&& CmdPool);
 
-    void DestroyPools(uint64_t CompletedFenceValue);
+    void DestroyPools();
 
 private:
-    const VulkanUtilities::VulkanLogicalDevice& m_LogicalDevice;
+    RenderDeviceVkImpl&                         m_DeviceVkImpl;
+    const std::string                           m_Name;
     const uint32_t                              m_QueueFamilyIndex;
     const VkCommandPoolCreateFlags              m_CmdPoolFlags;
 
-    std::mutex m_Mutex;
-    using CmdPoolQueueElemType = std::pair<uint64_t, VulkanUtilities::CommandPoolWrapper>;
-    std::deque< CmdPoolQueueElemType, STDAllocatorRawMem<CmdPoolQueueElemType> > m_CmdPools;
+    std::mutex                                      m_Mutex;
+    std::deque< VulkanUtilities::CommandPoolWrapper, STDAllocatorRawMem<VulkanUtilities::CommandPoolWrapper> > m_CmdPools;
 };
 
 }
diff --git a/Graphics/GraphicsEngineVulkan/src/CommandPoolManager.cpp b/Graphics/GraphicsEngineVulkan/src/CommandPoolManager.cpp
index d49560fc..2d37e145 100644
--- a/Graphics/GraphicsEngineVulkan/src/CommandPoolManager.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/CommandPoolManager.cpp
@@ -23,31 +23,35 @@
 
 #include "pch.h"
 #include "CommandPoolManager.h"
+#include "RenderDeviceVkImpl.h"
 
 namespace Diligent
 {
 
-CommandPoolManager::CommandPoolManager(const VulkanUtilities::VulkanLogicalDevice& LogicalDevice, 
-                                       uint32_t                                    queueFamilyIndex, 
-                                       VkCommandPoolCreateFlags                    flags)noexcept:
-    m_LogicalDevice   (LogicalDevice),
+CommandPoolManager::CommandPoolManager(RenderDeviceVkImpl&      DeviceVkImpl, 
+                                       std::string              Name,
+                                       uint32_t                 queueFamilyIndex, 
+                                       VkCommandPoolCreateFlags flags)noexcept:
+    m_DeviceVkImpl    (DeviceVkImpl),
+    m_Name            (std::move(Name)),
     m_QueueFamilyIndex(queueFamilyIndex),
     m_CmdPoolFlags    (flags),
-    m_CmdPools(STD_ALLOCATOR_RAW_MEM(CmdPoolQueueElemType, GetRawAllocator(), "Allocator for deque< std::pair<uint64_t, CommandPoolWrapper > >"))
+    m_CmdPools        (STD_ALLOCATOR_RAW_MEM(VulkanUtilities::CommandPoolWrapper, GetRawAllocator(), "Allocator for deque<VulkanUtilities::CommandPoolWrapper>"))
 {
 }
 
-VulkanUtilities::CommandPoolWrapper CommandPoolManager::AllocateCommandPool(uint64_t CompletedFenceValue, const char* DebugName)
+VulkanUtilities::CommandPoolWrapper CommandPoolManager::AllocateCommandPool(const char* DebugName)
 {
     std::lock_guard<std::mutex> LockGuard(m_Mutex);
 
     VulkanUtilities::CommandPoolWrapper CmdPool;
-    if(!m_CmdPools.empty() && m_CmdPools.front().first <= CompletedFenceValue)
+    if(!m_CmdPools.empty())
     {
-        CmdPool = std::move(m_CmdPools.front().second);
+        CmdPool = std::move(m_CmdPools.front());
         m_CmdPools.pop_front();
     }
 
+    auto& LogicalDevice = m_DeviceVkImpl.GetLogicalDevice();
     if(CmdPool == VK_NULL_HANDLE)
     {
         VkCommandPoolCreateInfo CmdPoolCI = {};
@@ -55,30 +59,26 @@ VulkanUtilities::CommandPoolWrapper CommandPoolManager::AllocateCommandPool(uint
         CmdPoolCI.pNext = nullptr;
         CmdPoolCI.queueFamilyIndex = m_QueueFamilyIndex;
         CmdPoolCI.flags = m_CmdPoolFlags;
-        CmdPool = m_LogicalDevice.CreateCommandPool(CmdPoolCI);
+        CmdPool = LogicalDevice.CreateCommandPool(CmdPoolCI);
         VERIFY_EXPR(CmdPool != VK_NULL_HANDLE);
     }
 
-    m_LogicalDevice.ResetCommandPool(CmdPool);
+    LogicalDevice.ResetCommandPool(CmdPool);
 
     return std::move(CmdPool);
 }
 
-void CommandPoolManager::DisposeCommandPool(VulkanUtilities::CommandPoolWrapper&& CmdPool, uint64_t FenceValue)
+void CommandPoolManager::FreeCommandPool(VulkanUtilities::CommandPoolWrapper&& CmdPool)
 {
     std::lock_guard<std::mutex> LockGuard(m_Mutex);
-    // Command pools must be disposed after the corresponding command list has been submitted to the queue.
-    // At this point the fence value has been incremented, so the pool can be added to the queue.
-    // There is no need to go through stale objects queue as FenceValue is guaranteed to be signaled
-    // afer the command buffer submission
-    m_CmdPools.emplace_back(FenceValue, std::move(CmdPool));
+    m_CmdPools.emplace_back(std::move(CmdPool));
 }
 
-void CommandPoolManager::DestroyPools(uint64_t CompletedFenceValue)
+void CommandPoolManager::DestroyPools()
 {
     std::lock_guard<std::mutex> LockGuard(m_Mutex);
-    while(!m_CmdPools.empty() && m_CmdPools.front().first <= CompletedFenceValue)
-        m_CmdPools.pop_front();
+    LOG_INFO_MESSAGE(m_Name, " allocated descriptor pool count: ", m_CmdPools.size() );
+    m_CmdPools.clear();
 }
 
 CommandPoolManager::~CommandPoolManager()
diff --git a/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp
index 7a10b8d7..6f6770ee 100644
--- a/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp
@@ -110,7 +110,7 @@ RenderDeviceVkImpl :: RenderDeviceVkImpl(IReferenceCounters*
         CreationAttribs.DynamicDescriptorPoolSize.MaxDescriptorSets,
         false // Pools can only be reset
     },
-    m_TransientCmdPoolMgr(*m_LogicalVkDevice, CmdQueues[0]->GetQueueFamilyIndex(), VK_COMMAND_POOL_CREATE_TRANSIENT_BIT),
+    m_TransientCmdPoolMgr(*this, "Transient command buffer pool manager", CmdQueues[0]->GetQueueFamilyIndex(), VK_COMMAND_POOL_CREATE_TRANSIENT_BIT),
     m_MemoryMgr("Global resource memory manager", *m_LogicalVkDevice, *m_PhysicalDevice, GetRawAllocator(), CreationAttribs.DeviceLocalMemoryPageSize, CreationAttribs.HostVisibleMemoryPageSize, CreationAttribs.DeviceLocalMemoryReserveSize, CreationAttribs.HostVisibleMemoryReserveSize),
     m_DynamicMemoryManager
     {
@@ -143,7 +143,7 @@ RenderDeviceVkImpl::~RenderDeviceVkImpl()
     // release queues
     FinishFrame(true);
 
-    m_TransientCmdPoolMgr.DestroyPools(m_CommandQueues[0].CmdQueue->GetCompletedFenceValue());
+    m_TransientCmdPoolMgr.DestroyPools();
 
     // We must destroy command queues explicitly prior to releasing Vulkan device
     DestroyCommandQueues();
@@ -159,9 +159,7 @@ RenderDeviceVkImpl::~RenderDeviceVkImpl()
 
 void RenderDeviceVkImpl::AllocateTransientCmdPool(VulkanUtilities::CommandPoolWrapper& CmdPool, VkCommandBuffer& vkCmdBuff, const Char *DebugPoolName)
 {
-    // TODO: rework this
-    auto CompletedFenceValue = m_CommandQueues[0].CmdQueue->GetCompletedFenceValue();
-    CmdPool = m_TransientCmdPoolMgr.AllocateCommandPool(CompletedFenceValue, DebugPoolName);
+    CmdPool = m_TransientCmdPoolMgr.AllocateCommandPool(DebugPoolName);
 
     // Allocate command buffer from the cmd pool
     VkCommandBufferAllocateInfo BuffAllocInfo = {};
@@ -222,13 +220,48 @@ void RenderDeviceVkImpl::ExecuteAndDisposeTransientCmdBuff(Uint32 QueueIndex, Vk
     // Since transient command buffers do not count as real command buffers, submit them directly to the queue
     // to avoid interference with the command buffer numbers
     Uint64 FenceValue = 0;
-    LockCommandQueue(0, [&](ICommandQueueVk* pCmdQueueVk)
+    LockCommandQueue(QueueIndex, [&](ICommandQueueVk* pCmdQueueVk)
         {
             FenceValue = pCmdQueueVk->Submit(SubmitInfo);
+
+            class CommandPoolDeleter
+            {
+            public:
+                CommandPoolDeleter(CommandPoolManager& _CmdPoolMgr, VulkanUtilities::CommandPoolWrapper&& _Pool) :
+                    CmdPoolMgr(&_CmdPoolMgr),
+                    Pool      (std::move(_Pool))
+                {}
+
+                CommandPoolDeleter             (const CommandPoolDeleter&)  = delete;
+                CommandPoolDeleter& operator = (const CommandPoolDeleter&)  = delete;
+                CommandPoolDeleter& operator = (      CommandPoolDeleter&&) = delete;
+
+                CommandPoolDeleter(CommandPoolDeleter&& rhs) : 
+                    CmdPoolMgr(rhs.CmdPoolMgr),
+                    Pool      (std::move(rhs.Pool))
+                {
+                    rhs.CmdPoolMgr = nullptr;
+                }
+                     
+
+                ~CommandPoolDeleter()
+                {
+                    if (CmdPoolMgr!=nullptr)
+                    {
+                        CmdPoolMgr->FreeCommandPool(std::move(Pool));
+                    }
+                }
+            private:
+                CommandPoolManager*                 CmdPoolMgr;
+                VulkanUtilities::CommandPoolWrapper Pool;
+            };
+
+            // Discard command pool directly to the release queue since we know exactly which queue it was submitted to 
+            // as well as the associated FenceValue
+            m_CommandQueues[QueueIndex].ReleaseQueue.DiscardResource(CommandPoolDeleter{m_TransientCmdPoolMgr, std::move(CmdPool)}, FenceValue);
         }
     );
-    // Dispose command pool
-    m_TransientCmdPoolMgr.DisposeCommandPool(std::move(CmdPool), FenceValue);
+   
 }
 
 void RenderDeviceVkImpl::SubmitCommandBuffer(const VkSubmitInfo& SubmitInfo,