Vulkan: Remove VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT

Some virtual machine drivers have performance cost with
VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT. This CL keeps the
individually freed descriptorSet in the pool and try to reuse rather
than free it.

This CL also removes descriptorSetCount from some API since it always
allocate one at a time.

Bug: b/237848471
Change-Id: I029d651101fa1050770eba9e733a166e56a69684
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3749797
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Ian Elliott <ianelliott@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>

src/libANGLE/renderer/vulkan/ProgramExecutableVk.cpp

@@ -1447,9 +1447,9 @@ angle::Result ProgramExecutableVk::bindDescriptorSets(
             // later sets to misbehave.
             if (mEmptyDescriptorSets[descriptorSetIndex] == VK_NULL_HANDLE)
             {
-                ANGLE_TRY(mDescriptorPools[descriptorSetIndex].get().allocateDescriptorSets(
+                ANGLE_TRY(mDescriptorPools[descriptorSetIndex].get().allocateDescriptorSet(
                     context, commandBufferHelper, mDescriptorSetLayouts[descriptorSetIndex].get(),
-                    1, &mDescriptorPoolBindings[descriptorSetIndex],
+                    &mDescriptorPoolBindings[descriptorSetIndex],
                     &mEmptyDescriptorSets[descriptorSetIndex]));
             }
             descSet = mEmptyDescriptorSets[descriptorSetIndex];

src/libANGLE/renderer/vulkan/UtilsVk.cpp

@@ -3659,9 +3659,9 @@ angle::Result UtilsVk::allocateDescriptorSet(ContextVk *contextVk,
                                              vk::RefCountedDescriptorPoolBinding *bindingOut,
                                              VkDescriptorSet *descriptorSetOut)
 {
-    ANGLE_TRY(mDescriptorPools[function].allocateDescriptorSets(
+    ANGLE_TRY(mDescriptorPools[function].allocateDescriptorSet(
         contextVk, commandBufferHelper,
-        mDescriptorSetLayouts[function][DescriptorSetIndex::Internal].get(), 1, bindingOut,
+        mDescriptorSetLayouts[function][DescriptorSetIndex::Internal].get(), bindingOut,
         descriptorSetOut));
 
     return angle::Result::Continue;

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -3184,7 +3184,7 @@ angle::Result DescriptorPoolHelper::init(Context *context,
 
     // If there are descriptorSet garbage, they no longer relevant since the entire pool is going to
     // be destroyed.
-    resetGarbageList();
+    mDescriptorSetGarbageList.clear();
 
     if (mDescriptorPool.valid())
     {
@@ -3202,7 +3202,7 @@ angle::Result DescriptorPoolHelper::init(Context *context,
 
     VkDescriptorPoolCreateInfo descriptorPoolInfo = {};
     descriptorPoolInfo.sType                      = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO;
-    descriptorPoolInfo.flags         = VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT;
+    descriptorPoolInfo.flags                      = 0;
     descriptorPoolInfo.maxSets                    = maxSets;
     descriptorPoolInfo.poolSizeCount              = static_cast<uint32_t>(poolSizes.size());
     descriptorPoolInfo.pPoolSizes                 = poolSizes.data();
@@ -3226,65 +3226,62 @@ void DescriptorPoolHelper::release(ContextVk *contextVk, VulkanCacheType cacheTy
     contextVk->addGarbage(&mDescriptorPool);
 }
 
-void DescriptorPoolHelper::resetGarbageList()
+bool DescriptorPoolHelper::allocateDescriptorSet(Context *context,
+                                                 CommandBufferHelperCommon *commandBufferHelper,
+                                                 const DescriptorSetLayout &descriptorSetLayout,
+                                                 VkDescriptorSet *descriptorSetsOut)
 {
-    mFreeDescriptorSets += mDescriptorSetGarbageList.size();
-    mDescriptorSetGarbageList.clear();
-}
-
-void DescriptorPoolHelper::cleanupGarbage(Context *context)
+    // Try to reuse descriptorSet garbage first
+    if (!mDescriptorSetGarbageList.empty())
     {
         RendererVk *rendererVk          = context->getRenderer();
         Serial lastCompletedQueueSerial = rendererVk->getLastCompletedQueueSerial();
-    while (!mDescriptorSetGarbageList.empty())
-    {
+
         DescriptorSetHelper &garbage = mDescriptorSetGarbageList.front();
-        if (garbage.isCurrentlyInUse(lastCompletedQueueSerial))
+        if (!garbage.isCurrentlyInUse(lastCompletedQueueSerial))
         {
-            break;
-        }
-        garbage.destroy(rendererVk->getDevice(), mDescriptorPool);
+            *descriptorSetsOut = garbage.getDescriptorSet();
+            commandBufferHelper->retainResource(this);
             mDescriptorSetGarbageList.pop_front();
-        mFreeDescriptorSets++;
+            return true;
         }
     }
 
-angle::Result DescriptorPoolHelper::allocateDescriptorSets(
-    Context *context,
-    CommandBufferHelperCommon *commandBufferHelper,
-    const DescriptorSetLayout &descriptorSetLayout,
-    uint32_t descriptorSetCount,
-    VkDescriptorSet *descriptorSetsOut)
+    if (mFreeDescriptorSets > 0)
     {
         VkDescriptorSetAllocateInfo allocInfo = {};
         allocInfo.sType                       = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO;
         allocInfo.descriptorPool              = mDescriptorPool.getHandle();
-    allocInfo.descriptorSetCount          = descriptorSetCount;
+        allocInfo.descriptorSetCount          = 1;
         allocInfo.pSetLayouts                 = descriptorSetLayout.ptr();
 
-    ASSERT(mFreeDescriptorSets >= descriptorSetCount);
-    mFreeDescriptorSets -= descriptorSetCount;
-
-    ANGLE_VK_TRY(context, mDescriptorPool.allocateDescriptorSets(context->getDevice(), allocInfo,
-                                                                 descriptorSetsOut));
+        VkResult result = mDescriptorPool.allocateDescriptorSets(context->getDevice(), allocInfo,
+                                                                 descriptorSetsOut);
+        // If fail, it means our own accounting has a bug.
+        ASSERT(result == VK_SUCCESS);
+        mFreeDescriptorSets--;
 
         // The pool is still in use every time a new descriptor set is allocated from it.
         commandBufferHelper->retainResource(this);
-
-    return angle::Result::Continue;
+        return true;
     }
 
-angle::Result DescriptorPoolHelper::allocateAndCacheDescriptorSet(
+    return false;
+}
+
+bool DescriptorPoolHelper::allocateAndCacheDescriptorSet(
     Context *context,
     CommandBufferHelperCommon *commandBufferHelper,
     const DescriptorSetDesc &desc,
     const DescriptorSetLayout &descriptorSetLayout,
     VkDescriptorSet *descriptorSetOut)
 {
-    ANGLE_TRY(allocateDescriptorSets(context, commandBufferHelper, descriptorSetLayout, 1,
-                                     descriptorSetOut));
+    if (allocateDescriptorSet(context, commandBufferHelper, descriptorSetLayout, descriptorSetOut))
+    {
         mDescriptorSetCache.insertDescriptorSet(desc, *descriptorSetOut);
-    return angle::Result::Continue;
+        return true;
+    }
+    return false;
 }
 
 bool DescriptorPoolHelper::getCachedDescriptorSet(const DescriptorSetDesc &desc,
@@ -3391,41 +3388,35 @@ void DynamicDescriptorPool::release(ContextVk *contextVk, VulkanCacheType cacheT
     mCachedDescriptorSetLayout = VK_NULL_HANDLE;
 }
 
-angle::Result DynamicDescriptorPool::allocateDescriptorSets(
+angle::Result DynamicDescriptorPool::allocateDescriptorSet(
     Context *context,
     CommandBufferHelperCommon *commandBufferHelper,
     const DescriptorSetLayout &descriptorSetLayout,
-    uint32_t descriptorSetCount,
     RefCountedDescriptorPoolBinding *bindingOut,
     VkDescriptorSet *descriptorSetsOut)
 {
     ASSERT(!mDescriptorPools.empty());
     ASSERT(descriptorSetLayout.getHandle() == mCachedDescriptorSetLayout);
 
-    bool hasCapacity = false;
-    if (bindingOut->valid())
+    if (!bindingOut->valid() ||
+        !bindingOut->get().allocateDescriptorSet(context, commandBufferHelper, descriptorSetLayout,
+                                                 descriptorSetsOut))
     {
-        // Free descriptorSet garbage before check
-        bindingOut->get().cleanupGarbage(context);
-        hasCapacity = bindingOut->get().hasCapacity(descriptorSetCount);
-    }
-
-    if (!hasCapacity)
-    {
-        mDescriptorPools[mCurrentPoolIndex]->get().cleanupGarbage(context);
-        hasCapacity = mDescriptorPools[mCurrentPoolIndex]->get().hasCapacity(descriptorSetCount);
-        if (!hasCapacity)
+        ASSERT(mDescriptorPools[mCurrentPoolIndex]->get().valid());
+        if (!mDescriptorPools[mCurrentPoolIndex]->get().allocateDescriptorSet(
+                context, commandBufferHelper, descriptorSetLayout, descriptorSetsOut))
         {
             ANGLE_TRY(allocateNewPool(context));
+            bool success = mDescriptorPools[mCurrentPoolIndex]->get().allocateDescriptorSet(
+                context, commandBufferHelper, descriptorSetLayout, descriptorSetsOut);
+            // Allocation in new pool must succeed
+            ASSERT(success);
         }
-
         bindingOut->set(mDescriptorPools[mCurrentPoolIndex]);
     }
 
     ++context->getPerfCounters().descriptorSetAllocations;
-
-    return bindingOut->get().allocateDescriptorSets(
-        context, commandBufferHelper, descriptorSetLayout, descriptorSetCount, descriptorSetsOut);
+    return angle::Result::Continue;
 }
 
 angle::Result DynamicDescriptorPool::getOrAllocateDescriptorSet(
@@ -3454,29 +3445,22 @@ angle::Result DynamicDescriptorPool::getOrAllocateDescriptorSet(
     ASSERT(!mDescriptorPools.empty());
     ASSERT(descriptorSetLayout.getHandle() == mCachedDescriptorSetLayout);
 
-    constexpr uint32_t kDescriptorSetCount = 1;
-
-    bool hasCapacity = false;
-    if (bindingOut->valid())
+    if (!bindingOut->valid() ||
+        !bindingOut->get().allocateAndCacheDescriptorSet(context, commandBufferHelper, desc,
+                                                         descriptorSetLayout, descriptorSetOut))
     {
-        // Free descriptorSet garbage before check
-        bindingOut->get().cleanupGarbage(context);
-        hasCapacity = bindingOut->get().hasCapacity(kDescriptorSetCount);
-    }
-
-    if (!hasCapacity)
-    {
-        mDescriptorPools[mCurrentPoolIndex]->get().cleanupGarbage(context);
-        hasCapacity = mDescriptorPools[mCurrentPoolIndex]->get().hasCapacity(kDescriptorSetCount);
-        if (!hasCapacity)
+        ASSERT(mDescriptorPools[mCurrentPoolIndex]->get().valid());
+        if (!mDescriptorPools[mCurrentPoolIndex]->get().allocateAndCacheDescriptorSet(
+                context, commandBufferHelper, desc, descriptorSetLayout, descriptorSetOut))
         {
             ANGLE_TRY(allocateNewPool(context));
+            bool success = mDescriptorPools[mCurrentPoolIndex]->get().allocateAndCacheDescriptorSet(
+                context, commandBufferHelper, desc, descriptorSetLayout, descriptorSetOut);
+            ASSERT(success);
         }
         bindingOut->set(mDescriptorPools[mCurrentPoolIndex]);
     }
 
-    ANGLE_TRY(bindingOut->get().allocateAndCacheDescriptorSet(
-        context, commandBufferHelper, desc, descriptorSetLayout, descriptorSetOut));
     *cacheResultOut = DescriptorCacheResult::NewAllocation;
     ++context->getPerfCounters().descriptorSetAllocations;
     return angle::Result::Continue;

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -148,11 +148,7 @@ class DescriptorSetHelper final : public Resource
         other.mDescriptorSet = VK_NULL_HANDLE;
     }
 
-    void destroy(VkDevice device, DescriptorPool &pool)
-    {
-        pool.freeDescriptorSets(device, 1, &mDescriptorSet);
-        mDescriptorSet = VK_NULL_HANDLE;
-    }
+    VkDescriptorSet getDescriptorSet() const { return mDescriptorSet; }
 
   private:
     VkDescriptorSet mDescriptorSet;
@@ -182,13 +178,12 @@ class DescriptorPoolHelper final : public Resource
     void destroy(RendererVk *renderer, VulkanCacheType cacheType);
     void release(ContextVk *contextVk, VulkanCacheType cacheType);
 
-    angle::Result allocateDescriptorSets(Context *context,
+    bool allocateDescriptorSet(Context *context,
                                CommandBufferHelperCommon *commandBufferHelper,
                                const DescriptorSetLayout &descriptorSetLayout,
-                                         uint32_t descriptorSetCount,
                                VkDescriptorSet *descriptorSetsOut);
 
-    angle::Result allocateAndCacheDescriptorSet(Context *context,
+    bool allocateAndCacheDescriptorSet(Context *context,
                                        CommandBufferHelperCommon *commandBufferHelper,
                                        const DescriptorSetDesc &desc,
                                        const DescriptorSetLayout &descriptorSetLayout,
@@ -199,8 +194,6 @@ class DescriptorPoolHelper final : public Resource
     void releaseCachedDescriptorSet(ContextVk *contextVk, const DescriptorSetDesc &desc);
     void destroyCachedDescriptorSet(const DescriptorSetDesc &desc);
     void resetCache();
-    // Scan descriptorSet garbage list and destroy all GPU completed garbage
-    void cleanupGarbage(Context *context);
 
     size_t getTotalCacheSize() const { return mDescriptorSetCache.getTotalCacheSize(); }
     size_t getTotalCacheKeySizeBytes() const
@@ -214,9 +207,6 @@ class DescriptorPoolHelper final : public Resource
     }
 
   private:
-    // Reset entire descriptorSet garbage list. This should only used when pool gets reset.
-    void resetGarbageList();
-
     uint32_t mFreeDescriptorSets;
     DescriptorPool mDescriptorPool;
     DescriptorSetCache mDescriptorSetCache;
@@ -254,10 +244,9 @@ class DynamicDescriptorPool final : angle::NonCopyable
 
     // We use the descriptor type to help count the number of free sets.
     // By convention, sets are indexed according to the constants in vk_cache_utils.h.
-    angle::Result allocateDescriptorSets(Context *context,
+    angle::Result allocateDescriptorSet(Context *context,
                                         CommandBufferHelperCommon *commandBufferHelper,
                                         const DescriptorSetLayout &descriptorSetLayout,
-                                         uint32_t descriptorSetCount,
                                         RefCountedDescriptorPoolBinding *bindingOut,
                                         VkDescriptorSet *descriptorSetsOut);