From 7d8f5979a22038aac622a303be75e72bc3c0ec30 Mon Sep 17 00:00:00 2001 From: Egor Yusov Date: Sat, 16 Jun 2018 20:37:37 -0700 Subject: Performance optimizations in Vulkan backend --- .../GraphicsEngineVulkan/include/PipelineLayout.h | 14 ++++++--- .../include/ShaderResourceCacheVk.h | 2 +- .../src/DeviceContextVkImpl.cpp | 2 +- .../GraphicsEngineVulkan/src/PipelineLayout.cpp | 35 ++++++++++++++-------- .../src/ShaderResourceCacheVk.cpp | 4 +-- 5 files changed, 37 insertions(+), 20 deletions(-) (limited to 'Graphics/GraphicsEngineVulkan') diff --git a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h index a0810ab7..947f27dc 100644 --- a/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h +++ b/Graphics/GraphicsEngineVulkan/include/PipelineLayout.h @@ -87,20 +87,26 @@ public: std::vector DynamicOffsets; ShaderResourceCacheVk* pResourceCache = nullptr; VkPipelineBindPoint BindPoint = VK_PIPELINE_BIND_POINT_MAX_ENUM; - + Uint32 SetCout = 0; + Uint32 DynamicOffsetCount = 0; #ifdef _DEBUG const PipelineLayout *pDbgPipelineLayout = nullptr; #endif - DescriptorSetBindInfo() + DescriptorSetBindInfo() : + vkSets(2), + DynamicOffsets(64) { - vkSets.reserve(2); - DynamicOffsets.reserve(64); } void Reset() { + SetCout = 0; + DynamicOffsetCount = 0; +#ifdef _DEBUG + // In release mode, do not clear vectors as this causes unnecessary work vkSets.clear(); DynamicOffsets.clear(); +#endif pResourceCache = nullptr; BindPoint = VK_PIPELINE_BIND_POINT_MAX_ENUM; #ifdef _DEBUG diff --git a/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h b/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h index 92f01236..5bd7eebc 100644 --- a/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h +++ b/Graphics/GraphicsEngineVulkan/include/ShaderResourceCacheVk.h @@ -173,7 +173,7 @@ public: template void TransitionResources(DeviceContextVkImpl *pCtxVkImpl); - void GetDynamicBufferOffsets(Uint32 CtxId, std::vector& Offsets)const; + Uint32 GetDynamicBufferOffsets(Uint32 CtxId, std::vector& Offsets)const; private: diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp index 7709a821..d1f88d0d 100644 --- a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp +++ b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp @@ -444,7 +444,7 @@ namespace Diligent else CommitVkVertexBuffers(); - if(!m_DesrSetBindInfo.DynamicOffsets.empty()) + if(m_DesrSetBindInfo.DynamicOffsetCount != 0) pPipelineStateVk->BindDescriptorSetsWithDynamicOffsets(this, m_DesrSetBindInfo); #if 0 #ifdef _DEBUG diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp index 8c746f5d..e3d88771 100644 --- a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp +++ b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp @@ -430,18 +430,25 @@ void PipelineLayout::PrepareDescriptorSets(DeviceContextVkImpl* pCtxVkImpl, ShaderResourceCacheVk& ResourceCache, DescriptorSetBindInfo& BindInfo)const { +#ifdef _DEBUG BindInfo.vkSets.clear(); +#endif + + // Do not use vector::resize for BindInfo.vkSets and BindInfo.DynamicOffsets as this + // causes unnecessary work to zero-initialize new elements VERIFY(m_LayoutMgr.GetDescriptorSet(SHADER_VARIABLE_TYPE_STATIC).SetIndex == m_LayoutMgr.GetDescriptorSet(SHADER_VARIABLE_TYPE_MUTABLE).SetIndex, "Static and mutable variables are expected to share the same descriptor set"); Uint32 TotalDynamicDescriptors = 0; + BindInfo.SetCout = 0; for(SHADER_VARIABLE_TYPE VarType = SHADER_VARIABLE_TYPE_MUTABLE; VarType <= SHADER_VARIABLE_TYPE_DYNAMIC; VarType = static_cast(VarType+1)) { const auto &Set = m_LayoutMgr.GetDescriptorSet(VarType); if(Set.SetIndex >= 0) { - if(Set.SetIndex >= BindInfo.vkSets.size()) - BindInfo.vkSets.resize(Set.SetIndex + 1); + BindInfo.SetCout = std::max(BindInfo.SetCout, static_cast(Set.SetIndex + 1)); + if(BindInfo.SetCout > BindInfo.vkSets.size()) + BindInfo.vkSets.resize(BindInfo.SetCout); VERIFY_EXPR(BindInfo.vkSets[Set.SetIndex] == VK_NULL_HANDLE); BindInfo.vkSets[Set.SetIndex] = ResourceCache.GetDescriptorSet(Set.SetIndex).GetVkDescriptorSet(); VERIFY(BindInfo.vkSets[Set.SetIndex] != VK_NULL_HANDLE, "Descriptor set must not be null"); @@ -454,7 +461,9 @@ void PipelineLayout::PrepareDescriptorSets(DeviceContextVkImpl* pCtxVkImpl, VERIFY(set != VK_NULL_HANDLE, "Descriptor set must not be null"); #endif - BindInfo.DynamicOffsets.resize(TotalDynamicDescriptors); + BindInfo.DynamicOffsetCount = TotalDynamicDescriptors; + if(TotalDynamicDescriptors > BindInfo.DynamicOffsets.size()) + BindInfo.DynamicOffsets.resize(TotalDynamicDescriptors); BindInfo.BindPoint = IsCompute ? VK_PIPELINE_BIND_POINT_COMPUTE : VK_PIPELINE_BIND_POINT_GRAPHICS; BindInfo.pResourceCache = &ResourceCache; #ifdef _DEBUG @@ -468,8 +477,8 @@ void PipelineLayout::PrepareDescriptorSets(DeviceContextVkImpl* pCtxVkImpl, CmdBuffer.BindDescriptorSets(BindInfo.BindPoint, m_LayoutMgr.GetVkPipelineLayout(), 0, // First set - static_cast(BindInfo.vkSets.size()), - !BindInfo.vkSets.empty() ? BindInfo.vkSets.data() : nullptr, + BindInfo.SetCout, + BindInfo.vkSets.data(), // BindInfo.vkSets is never empty 0, nullptr); } @@ -480,7 +489,7 @@ void PipelineLayout::BindDescriptorSetsWithDynamicOffsets(DeviceContextVkImpl* { VERIFY(BindInfo.pDbgPipelineLayout != nullptr, "Pipeline layout is not initialized, which most likely means that CommitShaderResources() has never been called"); VERIFY(BindInfo.pDbgPipelineLayout->IsSameAs(*this), "Inconsistent pipeline layout"); - VERIFY(!BindInfo.DynamicOffsets.empty(), "This function should only be called for pipelines that contain dynamic descriptors"); + VERIFY(BindInfo.DynamicOffsetCount > 0, "This function should only be called for pipelines that contain dynamic descriptors"); VERIFY_EXPR(BindInfo.pResourceCache != nullptr); #ifdef _DEBUG @@ -490,10 +499,12 @@ void PipelineLayout::BindDescriptorSetsWithDynamicOffsets(DeviceContextVkImpl* const auto &Set = m_LayoutMgr.GetDescriptorSet(VarType); TotalDynamicDescriptors += Set.NumDynamicDescriptors; } - VERIFY(BindInfo.DynamicOffsets.size() == TotalDynamicDescriptors, "Incosistent dynamic buffer size"); + VERIFY(BindInfo.DynamicOffsetCount == TotalDynamicDescriptors, "Incosistent dynamic buffer size"); + VERIFY_EXPR(BindInfo.DynamicOffsets.size() >= BindInfo.DynamicOffsetCount); #endif - BindInfo.pResourceCache->GetDynamicBufferOffsets(pCtxVkImpl->GetContextId(), BindInfo.DynamicOffsets); + auto NumOffsetsWritten = BindInfo.pResourceCache->GetDynamicBufferOffsets(pCtxVkImpl->GetContextId(), BindInfo.DynamicOffsets); + VERIFY_EXPR(NumOffsetsWritten == BindInfo.DynamicOffsetCount); auto& CmdBuffer = pCtxVkImpl->GetCommandBuffer(); // vkCmdBindDescriptorSets causes the sets numbered [firstSet .. firstSet+descriptorSetCount-1] to use the @@ -503,11 +514,11 @@ void PipelineLayout::BindDescriptorSetsWithDynamicOffsets(DeviceContextVkImpl* CmdBuffer.BindDescriptorSets(BindInfo.BindPoint, m_LayoutMgr.GetVkPipelineLayout(), 0, // First set - static_cast(BindInfo.vkSets.size()), - !BindInfo.vkSets.empty() ? BindInfo.vkSets.data() : nullptr, + BindInfo.SetCout, + BindInfo.vkSets.data(), // BindInfo.vkSets is never empty // dynamicOffsetCount must equal the total number of dynamic descriptors in the sets being bound (13.2.5) - static_cast(BindInfo.DynamicOffsets.size()), - !BindInfo.DynamicOffsets.empty() ? BindInfo.DynamicOffsets.data() : nullptr); + BindInfo.DynamicOffsetCount, + BindInfo.DynamicOffsets.data()); } } diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp index 4b16539e..f282426e 100644 --- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp +++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp @@ -284,7 +284,7 @@ VkDescriptorImageInfo ShaderResourceCacheVk::Resource::GetSamplerDescriptorWrite return DescrImgInfo; } -void ShaderResourceCacheVk::GetDynamicBufferOffsets(Uint32 CtxId, std::vector& Offsets)const +Uint32 ShaderResourceCacheVk::GetDynamicBufferOffsets(Uint32 CtxId, std::vector& Offsets)const { // If any of the sets being bound include dynamic uniform or storage buffers, then // pDynamicOffsets includes one element for each array element in each dynamic descriptor @@ -337,7 +337,7 @@ void ShaderResourceCacheVk::GetDynamicBufferOffsets(Uint32 CtxId, std::vector