Implemented command pool manager to handle multithreaded resource initialization in Vulkan

8 files changed, 223 insertions, 20 deletions

diff --git a/Graphics/GraphicsEngineVulkan/CMakeLists.txt b/Graphics/GraphicsEngineVulkan/CMakeLists.txt
index ef9273ab..fb574032 100644
--- a/Graphics/GraphicsEngineVulkan/CMakeLists.txt
+++ b/Graphics/GraphicsEngineVulkan/CMakeLists.txt
@@ -7,6 +7,7 @@ set(INCLUDE
     include/BufferViewVkImpl.h
     include/CommandContext.h
     include/CommandListVkImpl.h
+    include/CommandPoolManager.h
     include/CommandQueueVkImpl.h
     include/VulkanTypeConversions.h
     include/DescriptorPoolManager.h
@@ -64,6 +65,7 @@ set(SRC
     src/BufferVkImpl.cpp
     src/BufferViewVkImpl.cpp
     src/CommandContext.cpp
+    src/CommandPoolManager.cpp
     src/CommandQueueVkImpl.cpp
     src/VulkanTypeConversions.cpp
     src/DescriptorPoolManager.cpp
diff --git a/Graphics/GraphicsEngineVulkan/include/CommandPoolManager.h b/Graphics/GraphicsEngineVulkan/include/CommandPoolManager.h
new file mode 100644
index 00000000..19c66148
--- /dev/null
+++ b/Graphics/GraphicsEngineVulkan/include/CommandPoolManager.h
@@ -0,0 +1,70 @@
+/*     Copyright 2015-2018 Egor Yusov
+ *  
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ * 
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF ANY PROPRIETARY RIGHTS.
+ *
+ *  In no event and under no legal theory, whether in tort (including negligence), 
+ *  contract, or otherwise, unless required by applicable law (such as deliberate 
+ *  and grossly negligent acts) or agreed to in writing, shall any Contributor be
+ *  liable for any damages, including any direct, indirect, special, incidental, 
+ *  or consequential damages of any character arising as a result of this License or 
+ *  out of the use or inability to use the software (including but not limited to damages 
+ *  for loss of goodwill, work stoppage, computer failure or malfunction, or any and 
+ *  all other commercial damages or losses), even if such Contributor has been advised 
+ *  of the possibility of such damages.
+ */
+
+#pragma once
+
+#include <deque>
+#include <mutex>
+#include "STDAllocator.h"
+#include "VulkanUtilities/VulkanDescriptorPool.h"
+
+namespace Diligent
+{
+
+class CommandPoolManager
+{
+public:
+    CommandPoolManager(const VulkanUtilities::VulkanLogicalDevice& LogicalDevice, 
+                       uint32_t                                    queueFamilyIndex, 
+                       VkCommandPoolCreateFlags                    flags)noexcept;
+    
+    CommandPoolManager             (CommandPoolManager&& )     = delete;
+    CommandPoolManager             (const CommandPoolManager&) = delete;
+    CommandPoolManager& operator = (const CommandPoolManager&) = delete;
+    CommandPoolManager& operator = (CommandPoolManager&&)      = delete;
+
+    ~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);
+    
+    // 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);
+
+    void DestroyPools(uint64_t CompletedFenceValue);
+
+private:
+    const VulkanUtilities::VulkanLogicalDevice& m_LogicalDevice;
+    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;
+};
+
+}
diff --git a/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h b/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h
index c44b97f2..8946e694 100644
--- a/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h
+++ b/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h
@@ -40,6 +40,7 @@
 #include "VulkanUtilities/VulkanLogicalDevice.h"
 #include "VulkanUtilities/VulkanObjectWrappers.h"
 #include "FramebufferCache.h"
+#include "CommandPoolManager.h"
 
 /// Namespace for the Direct3D11 implementation of the graphics engine
 namespace Diligent
@@ -93,10 +94,11 @@ public:
     ICommandQueueVk *GetCmdQueue(){return m_pCommandQueue;}
     
 	void IdleGPU(bool ReleaseStaleObjects);
-    VkCommandBuffer AllocateCommandBuffer(const Char *DebugName = nullptr);
     void ExecuteCommandBuffer(const VkSubmitInfo &SubmitInfo, bool DiscardStaleObjects);
     void ExecuteCommandBuffer(VkCommandBuffer CmdBuff, bool DiscardStaleObjects);
-    void DisposeCommandBuffer(VkCommandBuffer CmdBuff);
+    
+    void AllocateTransientCmdPool(VulkanUtilities::CommandPoolWrapper& CmdPool, VkCommandBuffer& vkCmdBuff, const Char* DebugPoolName = nullptr);
+    void DisposeTransientCmdPool(VulkanUtilities::CommandPoolWrapper&& CmdPool);
 
 
     template<typename VulkanObjectType>
@@ -179,9 +181,6 @@ private:
     std::deque<CommandContext*, STDAllocatorRawMem<CommandContext*> > m_AvailableContexts;
 #endif
 	
-    VulkanUtilities::VulkanCommandBufferPool m_CmdBufferPool;
-	std::mutex m_CmdPoolMutex;
-
     std::mutex m_ReleaseQueueMutex;
 
     class StaleVulkanObjectBase
@@ -204,9 +203,13 @@ private:
     // [2+] - Deferred context dynamic descriptor pool
     std::vector<DescriptorPoolManager, STDAllocatorRawMem<DescriptorPoolManager> > m_DescriptorPools;
 
-
     typedef std::unique_ptr<VulkanDynamicHeap, STDDeleterRawMem<VulkanDynamicHeap> > UploadHeapPoolElemType;
     std::vector< UploadHeapPoolElemType, STDAllocatorRawMem<UploadHeapPoolElemType> > m_UploadHeaps;
+
+    // These one-time command pools are used by buffer and texture constructors to
+    // issue copy commands. Vulkan requires that every command pool is used by one thread 
+    // at a time, so every constructor must allocate command buffer from its own pool.
+    CommandPoolManager m_TransientCmdPoolMgr;
 };
 
 }
diff --git a/Graphics/GraphicsEngineVulkan/include/VulkanUtilities/VulkanLogicalDevice.h b/Graphics/GraphicsEngineVulkan/include/VulkanUtilities/VulkanLogicalDevice.h
index dbc62d2f..2ed955c0 100644
--- a/Graphics/GraphicsEngineVulkan/include/VulkanUtilities/VulkanLogicalDevice.h
+++ b/Graphics/GraphicsEngineVulkan/include/VulkanUtilities/VulkanLogicalDevice.h
@@ -118,6 +118,9 @@ namespace VulkanUtilities
                                   const VkWriteDescriptorSet*   pDescriptorWrites,
                                   uint32_t                      descriptorCopyCount,
                                   const VkCopyDescriptorSet*    pDescriptorCopies)const;
+        
+        VkResult ResetCommandPool(VkCommandPool             vkCmdPool,
+                                  VkCommandPoolResetFlags   flags = 0)const;
 
     private:
         VulkanLogicalDevice(VkPhysicalDevice vkPhysicalDevice, 
diff --git a/Graphics/GraphicsEngineVulkan/src/BufferVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/BufferVkImpl.cpp
index d157afb8..96f26de4 100644
--- a/Graphics/GraphicsEngineVulkan/src/BufferVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/BufferVkImpl.cpp
@@ -191,7 +191,9 @@ BufferVkImpl :: BufferVkImpl(IReferenceCounters *pRefCounters,
         err = LogicalDevice.BindBufferMemory(StagingBuffer, StagingBufferMemory, 0 /*offset*/);
         CHECK_VK_ERROR_AND_THROW(err, "Failed to bind staging bufer memory");
 
-        auto vkCmdBuff = pRenderDeviceVk->AllocateCommandBuffer("Transient cmd buff to copy staging data to a device buffer");
+        VulkanUtilities::CommandPoolWrapper CmdPool;
+        VkCommandBuffer vkCmdBuff;
+        pRenderDeviceVk->AllocateTransientCmdPool(CmdPool, vkCmdBuff, "Transient command pool to copy staging data to a device buffer");
 
         VulkanUtilities::VulkanCommandBuffer::BufferMemoryBarrier(vkCmdBuff, StagingBuffer, 0, VK_ACCESS_TRANSFER_READ_BIT);
         m_AccessFlags = VK_ACCESS_TRANSFER_WRITE_BIT;
@@ -224,7 +226,9 @@ BufferVkImpl :: BufferVkImpl(IReferenceCounters *pRefCounters,
         //                  |     was added to the delete queue      |                                   |
         //                  |     with value N                       |                                   |
 	    pRenderDeviceVk->ExecuteCommandBuffer(vkCmdBuff, false);
-        pRenderDeviceVk->DisposeCommandBuffer(vkCmdBuff);
+        // Dispose command pool. No need to dispose cmd buffer as the 
+        // pool will be reset and all buffer resources will be reclaimed
+        pRenderDeviceVk->DisposeTransientCmdPool(std::move(CmdPool));
 
         // Add reference to the object to the release queue to keep it alive
         // until copy operation is complete. This must be done after
diff --git a/Graphics/GraphicsEngineVulkan/src/CommandPoolManager.cpp b/Graphics/GraphicsEngineVulkan/src/CommandPoolManager.cpp
new file mode 100644
index 00000000..638b749a
--- /dev/null
+++ b/Graphics/GraphicsEngineVulkan/src/CommandPoolManager.cpp
@@ -0,0 +1,89 @@
+/*     Copyright 2015-2018 Egor Yusov
+ *  
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ * 
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF ANY PROPRIETARY RIGHTS.
+ *
+ *  In no event and under no legal theory, whether in tort (including negligence), 
+ *  contract, or otherwise, unless required by applicable law (such as deliberate 
+ *  and grossly negligent acts) or agreed to in writing, shall any Contributor be
+ *  liable for any damages, including any direct, indirect, special, incidental, 
+ *  or consequential damages of any character arising as a result of this License or 
+ *  out of the use or inability to use the software (including but not limited to damages 
+ *  for loss of goodwill, work stoppage, computer failure or malfunction, or any and 
+ *  all other commercial damages or losses), even if such Contributor has been advised 
+ *  of the possibility of such damages.
+ */
+
+#include "pch.h"
+#include "CommandPoolManager.h"
+
+namespace Diligent
+{
+
+CommandPoolManager ::CommandPoolManager(const VulkanUtilities::VulkanLogicalDevice& LogicalDevice, 
+                                        uint32_t                                    queueFamilyIndex, 
+                                        VkCommandPoolCreateFlags                    flags)noexcept:
+    m_LogicalDevice   (LogicalDevice),
+    m_QueueFamilyIndex(queueFamilyIndex),
+    m_CmdPoolFlags    (flags),
+    m_CmdPools(STD_ALLOCATOR_RAW_MEM(CmdPoolQueueElemType, GetRawAllocator(), "Allocator for deque< std::pair<uint64_t, CommandPoolWrapper > >"))
+{
+}
+
+VulkanUtilities::CommandPoolWrapper CommandPoolManager::AllocateCommandPool(uint64_t CompletedFenceValue, const char *DebugName)
+{
+    std::lock_guard<std::mutex> LockGuard(m_Mutex);
+
+    VulkanUtilities::CommandPoolWrapper CmdPool;
+    if(!m_CmdPools.empty() && m_CmdPools.front().first <= CompletedFenceValue)
+    {
+        CmdPool = std::move(m_CmdPools.front().second);
+        m_CmdPools.pop_front();
+    }
+
+    if(CmdPool == VK_NULL_HANDLE)
+    {
+        VkCommandPoolCreateInfo CmdPoolCI = {};
+        CmdPoolCI.sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO;
+        CmdPoolCI.pNext = nullptr;
+        CmdPoolCI.queueFamilyIndex = m_QueueFamilyIndex;
+        CmdPoolCI.flags = m_CmdPoolFlags;
+        CmdPool = m_LogicalDevice.CreateCommandPool(CmdPoolCI);
+        VERIFY_EXPR(CmdPool != VK_NULL_HANDLE);
+    }
+
+    m_LogicalDevice.ResetCommandPool(CmdPool);
+
+    return std::move(CmdPool);
+}
+
+void CommandPoolManager::DisposeCommandPool(VulkanUtilities::CommandPoolWrapper&& CmdPool, uint64_t FenceValue)
+{
+    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));
+}
+
+void CommandPoolManager::DestroyPools(uint64_t CompletedFenceValue)
+{
+    std::lock_guard<std::mutex> LockGuard(m_Mutex);
+    while(!m_CmdPools.empty() && m_CmdPools.front().first <= CompletedFenceValue)
+        m_CmdPools.pop_front();
+}
+
+CommandPoolManager::~CommandPoolManager()
+{
+    VERIFY(m_CmdPools.empty(), "Command pools have not been destroyed");
+}
+
+}
diff --git a/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp
index 2e9ae589..5744329c 100644
--- a/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/RenderDeviceVkImpl.cpp
@@ -59,8 +59,8 @@ RenderDeviceVkImpl :: RenderDeviceVkImpl(IReferenceCounters *pRefCounters,
     m_StaleVkObjects(STD_ALLOCATOR_RAW_MEM(ReleaseQueueElemType, GetRawAllocator(), "Allocator for queue<ReleaseQueueElemType>")),
     m_DescriptorPools(STD_ALLOCATOR_RAW_MEM(DescriptorPoolManager, GetRawAllocator(), "Allocator for vector<DescriptorPoolManager>")),
     m_UploadHeaps(STD_ALLOCATOR_RAW_MEM(UploadHeapPoolElemType, GetRawAllocator(), "Allocator for vector<unique_ptr<VulkanDynamicHeap>>")),
-    m_CmdBufferPool(m_LogicalVkDevice->GetSharedPtr(), pCmdQueue->GetQueueFamilyIndex(), VK_COMMAND_POOL_CREATE_TRANSIENT_BIT | VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT),
-    m_FramebufferCache(*this)
+    m_FramebufferCache(*this),
+    m_TransientCmdPoolMgr(*LogicalDevice, pCmdQueue->GetQueueFamilyIndex(), VK_COMMAND_POOL_CREATE_TRANSIENT_BIT)
 {
     m_DeviceCaps.DevType = DeviceType::Vulkan;
     m_DeviceCaps.MajorVersion = 1;
@@ -139,6 +139,8 @@ RenderDeviceVkImpl::~RenderDeviceVkImpl()
     VERIFY(m_StaleVkObjects.empty(), "Not all stale objects were destroyed");
     VERIFY(m_VkObjReleaseQueue.empty(), "Release queue is not empty");
     
+    m_TransientCmdPoolMgr.DestroyPools(m_pCommandQueue->GetCompletedFenceValue());
+
     //if(m_PhysicalDevice)
     //{
     //    // If m_PhysicalDevice is empty, the device does not own vulkan logical device and must not
@@ -148,12 +150,6 @@ RenderDeviceVkImpl::~RenderDeviceVkImpl()
 }
 
 
-void RenderDeviceVkImpl::DisposeCommandBuffer(VkCommandBuffer CmdBuff)
-{
-	std::lock_guard<std::mutex> LockGuard(m_CmdPoolMutex);
-    m_CmdBufferPool.DisposeCommandBuffer(CmdBuff, GetNextFenceValue());
-}
-
 
 void RenderDeviceVkImpl::ExecuteCommandBuffer(VkCommandBuffer CmdBuff, bool DiscardStaleObjects)
 {
@@ -360,13 +356,41 @@ void RenderDeviceVkImpl::FinishFrame(bool ReleaseAllResources)
     Atomics::AtomicIncrement(m_FrameNumber);
 }
 
-VkCommandBuffer RenderDeviceVkImpl::AllocateCommandBuffer(const Char *DebugName)
+void RenderDeviceVkImpl::AllocateTransientCmdPool(VulkanUtilities::CommandPoolWrapper& CmdPool, VkCommandBuffer& vkCmdBuff, const Char *DebugPoolName)
 {
-	std::lock_guard<std::mutex> LockGuard(m_CmdPoolMutex);
-    auto CmdBuffer = m_CmdBufferPool.GetCommandBuffer(GetCompletedFenceValue(), DebugName);
-    return CmdBuffer;
+    auto CompletedFenceValue = GetCompletedFenceValue();
+    CmdPool = m_TransientCmdPoolMgr.AllocateCommandPool(CompletedFenceValue, DebugPoolName);
+
+    // Allocate command buffer from the cmd pool
+    VkCommandBufferAllocateInfo BuffAllocInfo = {};
+    BuffAllocInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO;
+    BuffAllocInfo.pNext = nullptr;
+    BuffAllocInfo.commandPool = CmdPool;
+    BuffAllocInfo.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY;
+    BuffAllocInfo.commandBufferCount = 1;
+    vkCmdBuff = m_LogicalVkDevice->AllocateVkCommandBuffer(BuffAllocInfo);
+    VERIFY_EXPR(vkCmdBuff != VK_NULL_HANDLE);
+        
+    VkCommandBufferBeginInfo CmdBuffBeginInfo = {};
+    CmdBuffBeginInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO;
+    CmdBuffBeginInfo.pNext = nullptr;
+    CmdBuffBeginInfo.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT; // Each recording of the command buffer will only be 
+                                                                            // submitted once, and the command buffer will be reset 
+                                                                            // and recorded again between each submission.
+    CmdBuffBeginInfo.pInheritanceInfo = nullptr; // Ignored for a primary command buffer
+    vkBeginCommandBuffer(vkCmdBuff, &CmdBuffBeginInfo);
 }
 
+void RenderDeviceVkImpl::DisposeTransientCmdPool(VulkanUtilities::CommandPoolWrapper&& CmdPool)
+{
+    // Command pools must be disposed after the corresponding command buffer has been submitted to the queue.
+    // At this point the fence value has already been incremented, so the pool can be added to the queue.
+    // There is no need to go through stale objects queue as GetNextFenceValue() will be at least
+    // one greater than the fence value when the cmd buffer was submitted
+    m_TransientCmdPoolMgr.DisposeCommandPool(std::move(CmdPool), m_pCommandQueue->GetNextFenceValue());
+}
+
+
 
 template<typename VulkanObjectType>
 class RenderDeviceVkImpl::StaleVulkanObject : public StaleVulkanObjectBase
diff --git a/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanLogicalDevice.cpp b/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanLogicalDevice.cpp
index 39ec1011..46365c51 100644
--- a/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanLogicalDevice.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanLogicalDevice.cpp
@@ -434,4 +434,12 @@ namespace VulkanUtilities
     {
         vkUpdateDescriptorSets(m_VkDevice, descriptorWriteCount, pDescriptorWrites, descriptorCopyCount, pDescriptorCopies);
     }
+
+    VkResult VulkanLogicalDevice::ResetCommandPool(VkCommandPool            vkCmdPool,
+                                                   VkCommandPoolResetFlags  flags)const
+    {
+        auto err = vkResetCommandPool(m_VkDevice, vkCmdPool, flags);
+        VERIFY(err == VK_SUCCESS, "Failed to reset command pool");
+        return err;
+    }
 }