Reland "Slab-allocate VkDescriptorSets"

This is a reland of 96c4019214

It includes a fix to add a dummy descriptor count if the
VkDescriptorPool would be empty, and adds a test that a bind group
with an empty bind group layout may be created and used.

Original change's description:
> Slab-allocate VkDescriptorSets
>
> This introduces a slab allocator for VkDescriptorSets which creates
> a VkDescriptorPool pre-allocated with multiple VkDescriptorSets per
> BindGroupLayout. In the future, we can deduplicate pools that have
> the same, or roughly the same, descriptor counts.
>
> This CL also removes the old DescriptorSetService and moves most of
> the functionality onto the DescriptorSetAllocator itself to keep
> the tracking logic in one place.
>
> Bug: dawn:340
> Change-Id: I785b17f4353fb3d40c9ccc33746600d6794efe7c
> Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19320
> Reviewed-by: Austin Eng <enga@chromium.org>
> Commit-Queue: Austin Eng <enga@chromium.org>

Bug: dawn:340
Change-Id: Iabb744f110d0cab442bb857b31c87ba46bf0ad7a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/20321
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/BUILD.gn

@@ -472,8 +472,9 @@ source_set("dawn_native_sources") {
       "vulkan/CommandRecordingContext.h",
       "vulkan/ComputePipelineVk.cpp",
       "vulkan/ComputePipelineVk.h",
-      "vulkan/DescriptorSetService.cpp",
+      "vulkan/DescriptorSetAllocation.h",
-      "vulkan/DescriptorSetService.h",
+      "vulkan/DescriptorSetAllocator.cpp",
+      "vulkan/DescriptorSetAllocator.h",
       "vulkan/DeviceVk.cpp",
       "vulkan/DeviceVk.h",
       "vulkan/ExternalHandle.h",

src/dawn_native/CMakeLists.txt

@@ -365,8 +365,9 @@ if (DAWN_ENABLE_VULKAN)
         "vulkan/CommandRecordingContext.h"
         "vulkan/ComputePipelineVk.cpp"
         "vulkan/ComputePipelineVk.h"
-        "vulkan/DescriptorSetService.cpp"
+        "vulkan/DescriptorSetAllocation.h"
-        "vulkan/DescriptorSetService.h"
+        "vulkan/DescriptorSetAllocator.cpp"
+        "vulkan/DescriptorSetAllocator.h"
         "vulkan/DeviceVk.cpp"
         "vulkan/DeviceVk.h"
         "vulkan/ExternalHandle.h"

src/dawn_native/vulkan/BindGroupLayoutVk.cpp

@@ -16,7 +16,7 @@
 
 #include "common/BitSetIterator.h"
 #include "dawn_native/vulkan/BindGroupVk.h"
-#include "dawn_native/vulkan/DescriptorSetService.h"
+#include "dawn_native/vulkan/DescriptorSetAllocator.h"
 #include "dawn_native/vulkan/DeviceVk.h"
 #include "dawn_native/vulkan/FencedDeleter.h"
 #include "dawn_native/vulkan/VulkanError.h"
@@ -127,11 +127,10 @@ namespace dawn_native { namespace vulkan {
             descriptorCountPerType[vulkanType]++;
         }
 
-        mPoolSizes.reserve(descriptorCountPerType.size());
+        // TODO(enga): Consider deduping allocators for layouts with the same descriptor type
-        for (const auto& it : descriptorCountPerType) {
+        // counts.
-            mPoolSizes.push_back(VkDescriptorPoolSize{it.first, it.second});
+        mDescriptorSetAllocator =
-        }
+            std::make_unique<DescriptorSetAllocator>(this, std::move(descriptorCountPerType));
-
         return {};
     }
 
@@ -145,17 +144,15 @@ namespace dawn_native { namespace vulkan {
         Device* device = ToBackend(GetDevice());
 
         // DescriptorSetLayout aren't used by execution on the GPU and can be deleted at any time,
-        // so we destroy mHandle immediately instead of using the FencedDeleter
+        // so we can destroy mHandle immediately instead of using the FencedDeleter.
+        // (Swiftshader implements this wrong b/154522740).
+        // In practice, the GPU is done with all descriptor sets because bind group deallocation
+        // refs the bind group layout so that once the bind group is finished being used, we can
+        // recycle its descriptor set.
         if (mHandle != VK_NULL_HANDLE) {
             device->fn.DestroyDescriptorSetLayout(device->GetVkDevice(), mHandle, nullptr);
             mHandle = VK_NULL_HANDLE;
         }
-
-        FencedDeleter* deleter = device->GetFencedDeleter();
-        for (const SingleDescriptorSetAllocation& allocation : mAllocations) {
-            deleter->DeleteWhenUnused(allocation.pool);
-        }
-        mAllocations.clear();
     }
 
     VkDescriptorSetLayout BindGroupLayout::GetHandle() const {
@@ -166,79 +163,19 @@ namespace dawn_native { namespace vulkan {
         Device* device,
         const BindGroupDescriptor* descriptor) {
         DescriptorSetAllocation descriptorSetAllocation;
-        DAWN_TRY_ASSIGN(descriptorSetAllocation, AllocateOneDescriptorSet());
+        DAWN_TRY_ASSIGN(descriptorSetAllocation, mDescriptorSetAllocator->Allocate());
+
         return mBindGroupAllocator.Allocate(device, descriptor, descriptorSetAllocation);
     }
 
-    void BindGroupLayout::DeallocateBindGroup(BindGroup* bindGroup) {
+    void BindGroupLayout::DeallocateBindGroup(BindGroup* bindGroup,
+                                              DescriptorSetAllocation* descriptorSetAllocation) {
+        mDescriptorSetAllocator->Deallocate(descriptorSetAllocation);
         mBindGroupAllocator.Deallocate(bindGroup);
     }
 
-    ResultOrError<DescriptorSetAllocation> BindGroupLayout::AllocateOneDescriptorSet() {
+    void BindGroupLayout::FinishDeallocation(Serial completedSerial) {
-        Device* device = ToBackend(GetDevice());
+        mDescriptorSetAllocator->FinishDeallocation(completedSerial);
-
-        // Reuse a previous allocation if available.
-        if (!mAvailableAllocations.empty()) {
-            size_t index = mAvailableAllocations.back();
-            mAvailableAllocations.pop_back();
-            return {{index, mAllocations[index].set}};
-        }
-
-        // Create a pool to hold our descriptor set.
-        // TODO(cwallez@chromium.org): This horribly inefficient, have more than one descriptor
-        // set per pool.
-        VkDescriptorPoolCreateInfo createInfo;
-        createInfo.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO;
-        createInfo.pNext = nullptr;
-        createInfo.flags = 0;
-        createInfo.maxSets = 1;
-        createInfo.poolSizeCount = static_cast<uint32_t>(mPoolSizes.size());
-        createInfo.pPoolSizes = mPoolSizes.data();
-
-        VkDescriptorPool descriptorPool;
-        DAWN_TRY(CheckVkSuccess(device->fn.CreateDescriptorPool(device->GetVkDevice(), &createInfo,
-                                                                nullptr, &*descriptorPool),
-                                "CreateDescriptorPool"));
-
-        // Allocate our single set.
-        VkDescriptorSetAllocateInfo allocateInfo;
-        allocateInfo.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO;
-        allocateInfo.pNext = nullptr;
-        allocateInfo.descriptorPool = descriptorPool;
-        allocateInfo.descriptorSetCount = 1;
-        allocateInfo.pSetLayouts = &*mHandle;
-
-        VkDescriptorSet descriptorSet;
-        MaybeError result =
-            CheckVkSuccess(device->fn.AllocateDescriptorSets(device->GetVkDevice(), &allocateInfo,
-                                                             &*descriptorSet),
-                           "AllocateDescriptorSets");
-
-        if (result.IsError()) {
-            // On an error we can destroy the pool immediately because no command references it.
-            device->fn.DestroyDescriptorPool(device->GetVkDevice(), descriptorPool, nullptr);
-            return result.AcquireError();
-        }
-
-        mAllocations.push_back({descriptorPool, descriptorSet});
-        return {{mAllocations.size() - 1, descriptorSet}};
-    }
-
-    void BindGroupLayout::DeallocateDescriptorSet(
-        DescriptorSetAllocation* descriptorSetAllocation) {
-        // We can't reuse the descriptor set right away because the Vulkan spec says in the
-        // documentation for vkCmdBindDescriptorSets that the set may be consumed any time between
-        // host execution of the command and the end of the draw/dispatch.
-        ToBackend(GetDevice())
-            ->GetDescriptorSetService()
-            ->AddDeferredDeallocation(this, descriptorSetAllocation->index);
-
-        // Clear the content of allocation so that use after frees are more visible.
-        *descriptorSetAllocation = {};
-    }
-
-    void BindGroupLayout::FinishDeallocation(size_t index) {
-        mAvailableAllocations.push_back(index);
     }
 
 }}  // namespace dawn_native::vulkan

src/dawn_native/vulkan/BindGroupLayoutVk.h

@@ -17,6 +17,7 @@
 
 #include "dawn_native/BindGroupLayout.h"
 
+#include "common/Serial.h"
 #include "common/SlabAllocator.h"
 #include "common/vulkan_platform.h"
 
@@ -25,16 +26,12 @@
 namespace dawn_native { namespace vulkan {
 
     class BindGroup;
+    struct DescriptorSetAllocation;
+    class DescriptorSetAllocator;
     class Device;
 
     VkDescriptorType VulkanDescriptorType(wgpu::BindingType type, bool isDynamic);
 
-    // Contains a descriptor set along with data necessary to track its allocation.
-    struct DescriptorSetAllocation {
-        size_t index = 0;
-        VkDescriptorSet set = VK_NULL_HANDLE;
-    };
-
     // In Vulkan descriptor pools have to be sized to an exact number of descriptors. This means
     // it's hard to have something where we can mix different types of descriptor sets because
     // we don't know if their vector of number of descriptors will be similar.
@@ -58,31 +55,18 @@ namespace dawn_native { namespace vulkan {
 
         ResultOrError<BindGroup*> AllocateBindGroup(Device* device,
                                                     const BindGroupDescriptor* descriptor);
-        void DeallocateBindGroup(BindGroup* bindGroup);
+        void DeallocateBindGroup(BindGroup* bindGroup,
-
+                                 DescriptorSetAllocation* descriptorSetAllocation);
-        ResultOrError<DescriptorSetAllocation> AllocateOneDescriptorSet();
+        void FinishDeallocation(Serial completedSerial);
-        void DeallocateDescriptorSet(DescriptorSetAllocation* descriptorSetAllocation);
-
-        // Interaction with the DescriptorSetService.
-        void FinishDeallocation(size_t index);
 
       private:
         ~BindGroupLayout() override;
         MaybeError Initialize();
 
-        std::vector<VkDescriptorPoolSize> mPoolSizes;
-
-        struct SingleDescriptorSetAllocation {
-            VkDescriptorPool pool = VK_NULL_HANDLE;
-            // Descriptor sets are freed when the pool is destroyed.
-            VkDescriptorSet set = VK_NULL_HANDLE;
-        };
-        std::vector<SingleDescriptorSetAllocation> mAllocations;
-        std::vector<size_t> mAvailableAllocations;
-
         VkDescriptorSetLayout mHandle = VK_NULL_HANDLE;
 
         SlabAllocator<BindGroup> mBindGroupAllocator;
+        std::unique_ptr<DescriptorSetAllocator> mDescriptorSetAllocator;
     };
 
 }}  // namespace dawn_native::vulkan

src/dawn_native/vulkan/BindGroupVk.cpp

@@ -115,8 +115,7 @@ namespace dawn_native { namespace vulkan {
     }
 
     BindGroup::~BindGroup() {
-        ToBackend(GetLayout())->DeallocateDescriptorSet(&mDescriptorSetAllocation);
+        ToBackend(GetLayout())->DeallocateBindGroup(this, &mDescriptorSetAllocation);
-        ToBackend(GetLayout())->DeallocateBindGroup(this);
     }
 
     VkDescriptorSet BindGroup::GetHandle() const {

src/dawn_native/vulkan/BindGroupVk.h

@@ -20,6 +20,7 @@
 #include "common/PlacementAllocated.h"
 #include "common/vulkan_platform.h"
 #include "dawn_native/vulkan/BindGroupLayoutVk.h"
+#include "dawn_native/vulkan/DescriptorSetAllocation.h"
 
 namespace dawn_native { namespace vulkan {
 

src/dawn_native/vulkan/DescriptorSetAllocation.h

@@ -0,0 +1,29 @@
+// Copyright 2020 The Dawn Authors
+//
+// 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
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef DAWNNATIVE_VULKAN_DESCRIPTORSETALLOCATION_H_
+#define DAWNNATIVE_VULKAN_DESCRIPTORSETALLOCATION_H_
+
+namespace dawn_native { namespace vulkan {
+
+    // Contains a descriptor set along with data necessary to track its allocation.
+    struct DescriptorSetAllocation {
+        VkDescriptorSet set = VK_NULL_HANDLE;
+        uint32_t poolIndex;
+        uint16_t setIndex;
+    };
+
+}}  // namespace dawn_native::vulkan
+
+#endif  // DAWNNATIVE_VULKAN_DESCRIPTORSETALLOCATION_H_

src/dawn_native/vulkan/DescriptorSetAllocator.cpp

@@ -0,0 +1,179 @@
+// Copyright 2020 The Dawn Authors
+//
+// 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
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "dawn_native/vulkan/DescriptorSetAllocator.h"
+
+#include "dawn_native/vulkan/BindGroupLayoutVk.h"
+#include "dawn_native/vulkan/DeviceVk.h"
+#include "dawn_native/vulkan/FencedDeleter.h"
+#include "dawn_native/vulkan/VulkanError.h"
+
+namespace dawn_native { namespace vulkan {
+
+    // TODO(enga): Figure out this value.
+    static constexpr uint32_t kMaxDescriptorsPerPool = 512;
+
+    DescriptorSetAllocator::DescriptorSetAllocator(
+        BindGroupLayout* layout,
+        std::map<VkDescriptorType, uint32_t> descriptorCountPerType)
+        : mLayout(layout) {
+        ASSERT(layout != nullptr);
+
+        // Compute the total number of descriptors for this layout.
+        uint32_t totalDescriptorCount = 0;
+        mPoolSizes.reserve(descriptorCountPerType.size());
+        for (const auto& it : descriptorCountPerType) {
+            ASSERT(it.second > 0);
+            totalDescriptorCount += it.second;
+            mPoolSizes.push_back(VkDescriptorPoolSize{it.first, it.second});
+        }
+        ASSERT(totalDescriptorCount <= kMaxBindingsPerGroup);
+
+        if (totalDescriptorCount == 0) {
+            // Vulkan requires that valid usage of vkCreateDescriptorPool must have a non-zero
+            // number of pools, each of which has non-zero descriptor counts.
+            // Since the descriptor set layout is empty, we should be able to allocate
+            // |kMaxDescriptorsPerPool| sets from this 1-sized descriptor pool.
+            // The type of this descriptor pool doesn't matter because it is never used.
+            mPoolSizes.push_back(VkDescriptorPoolSize{VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, 1});
+            mMaxSets = kMaxDescriptorsPerPool;
+        } else {
+            // Compute the total number of descriptors sets that fits given the max.
+            mMaxSets = kMaxDescriptorsPerPool / totalDescriptorCount;
+            ASSERT(mMaxSets > 0);
+
+            // Grow the number of desciptors in the pool to fit the computed |mMaxSets|.
+            for (auto& poolSize : mPoolSizes) {
+                poolSize.descriptorCount *= mMaxSets;
+            }
+        }
+    }
+
+    DescriptorSetAllocator::~DescriptorSetAllocator() {
+        for (auto& pool : mDescriptorPools) {
+            ASSERT(pool.freeSetIndices.size() == mMaxSets);
+            if (pool.vkPool != VK_NULL_HANDLE) {
+                Device* device = ToBackend(mLayout->GetDevice());
+                device->GetFencedDeleter()->DeleteWhenUnused(pool.vkPool);
+            }
+        }
+    }
+
+    ResultOrError<DescriptorSetAllocation> DescriptorSetAllocator::Allocate() {
+        if (mAvailableDescriptorPoolIndices.empty()) {
+            DAWN_TRY(AllocateDescriptorPool());
+        }
+
+        ASSERT(!mAvailableDescriptorPoolIndices.empty());
+
+        const PoolIndex poolIndex = mAvailableDescriptorPoolIndices.back();
+        DescriptorPool* pool = &mDescriptorPools[poolIndex];
+
+        ASSERT(!pool->freeSetIndices.empty());
+
+        SetIndex setIndex = pool->freeSetIndices.back();
+        pool->freeSetIndices.pop_back();
+
+        if (pool->freeSetIndices.empty()) {
+            mAvailableDescriptorPoolIndices.pop_back();
+        }
+
+        return DescriptorSetAllocation{pool->sets[setIndex], poolIndex, setIndex};
+    }
+
+    void DescriptorSetAllocator::Deallocate(DescriptorSetAllocation* allocationInfo) {
+        ASSERT(allocationInfo != nullptr);
+        ASSERT(allocationInfo->set != VK_NULL_HANDLE);
+
+        // We can't reuse the descriptor set right away because the Vulkan spec says in the
+        // documentation for vkCmdBindDescriptorSets that the set may be consumed any time between
+        // host execution of the command and the end of the draw/dispatch.
+        Device* device = ToBackend(mLayout->GetDevice());
+        const Serial serial = device->GetPendingCommandSerial();
+        mPendingDeallocations.Enqueue({allocationInfo->poolIndex, allocationInfo->setIndex},
+                                      serial);
+
+        if (mLastDeallocationSerial != serial) {
+            device->EnqueueDeferredDeallocation(mLayout);
+            mLastDeallocationSerial = serial;
+        }
+
+        // Clear the content of allocation so that use after frees are more visible.
+        *allocationInfo = {};
+    }
+
+    void DescriptorSetAllocator::FinishDeallocation(Serial completedSerial) {
+        for (const Deallocation& dealloc : mPendingDeallocations.IterateUpTo(completedSerial)) {
+            ASSERT(dealloc.poolIndex < mDescriptorPools.size());
+
+            auto& freeSetIndices = mDescriptorPools[dealloc.poolIndex].freeSetIndices;
+            if (freeSetIndices.empty()) {
+                mAvailableDescriptorPoolIndices.emplace_back(dealloc.poolIndex);
+            }
+            freeSetIndices.emplace_back(dealloc.setIndex);
+        }
+        mPendingDeallocations.ClearUpTo(completedSerial);
+    }
+
+    MaybeError DescriptorSetAllocator::AllocateDescriptorPool() {
+        VkDescriptorPoolCreateInfo createInfo;
+        createInfo.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO;
+        createInfo.pNext = nullptr;
+        createInfo.flags = 0;
+        createInfo.maxSets = mMaxSets;
+        createInfo.poolSizeCount = static_cast<PoolIndex>(mPoolSizes.size());
+        createInfo.pPoolSizes = mPoolSizes.data();
+
+        Device* device = ToBackend(mLayout->GetDevice());
+
+        VkDescriptorPool descriptorPool;
+        DAWN_TRY(CheckVkSuccess(device->fn.CreateDescriptorPool(device->GetVkDevice(), &createInfo,
+                                                                nullptr, &*descriptorPool),
+                                "CreateDescriptorPool"));
+
+        std::vector<VkDescriptorSetLayout> layouts(mMaxSets, mLayout->GetHandle());
+
+        VkDescriptorSetAllocateInfo allocateInfo;
+        allocateInfo.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO;
+        allocateInfo.pNext = nullptr;
+        allocateInfo.descriptorPool = descriptorPool;
+        allocateInfo.descriptorSetCount = mMaxSets;
+        allocateInfo.pSetLayouts = AsVkArray(layouts.data());
+
+        std::vector<VkDescriptorSet> sets(mMaxSets);
+        MaybeError result =
+            CheckVkSuccess(device->fn.AllocateDescriptorSets(device->GetVkDevice(), &allocateInfo,
+                                                             AsVkArray(sets.data())),
+                           "AllocateDescriptorSets");
+        if (result.IsError()) {
+            // On an error we can destroy the pool immediately because no command references it.
+            device->fn.DestroyDescriptorPool(device->GetVkDevice(), descriptorPool, nullptr);
+            DAWN_TRY(std::move(result));
+        }
+
+        std::vector<SetIndex> freeSetIndices;
+        freeSetIndices.reserve(mMaxSets);
+
+        for (SetIndex i = 0; i < mMaxSets; ++i) {
+            freeSetIndices.push_back(i);
+        }
+
+        mAvailableDescriptorPoolIndices.push_back(mDescriptorPools.size());
+        mDescriptorPools.emplace_back(
+            DescriptorPool{descriptorPool, std::move(sets), std::move(freeSetIndices)});
+
+        return {};
+    }
+
+}}  // namespace dawn_native::vulkan

src/dawn_native/vulkan/DescriptorSetAllocator.h

@@ -0,0 +1,70 @@
+// Copyright 2020 The Dawn Authors
+//
+// 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
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef DAWNNATIVE_VULKAN_DESCRIPTORSETALLOCATOR_H_
+#define DAWNNATIVE_VULKAN_DESCRIPTORSETALLOCATOR_H_
+
+#include "common/SerialQueue.h"
+#include "common/vulkan_platform.h"
+#include "dawn_native/Error.h"
+#include "dawn_native/vulkan/DescriptorSetAllocation.h"
+
+#include <map>
+#include <vector>
+
+namespace dawn_native { namespace vulkan {
+
+    class BindGroupLayout;
+
+    class DescriptorSetAllocator {
+        using PoolIndex = uint32_t;
+        using SetIndex = uint16_t;
+
+      public:
+        DescriptorSetAllocator(BindGroupLayout* layout,
+                               std::map<VkDescriptorType, uint32_t> descriptorCountPerType);
+        ~DescriptorSetAllocator();
+
+        ResultOrError<DescriptorSetAllocation> Allocate();
+        void Deallocate(DescriptorSetAllocation* allocationInfo);
+        void FinishDeallocation(Serial completedSerial);
+
+      private:
+        MaybeError AllocateDescriptorPool();
+
+        BindGroupLayout* mLayout;
+
+        std::vector<VkDescriptorPoolSize> mPoolSizes;
+        SetIndex mMaxSets;
+
+        struct DescriptorPool {
+            VkDescriptorPool vkPool;
+            std::vector<VkDescriptorSet> sets;
+            std::vector<SetIndex> freeSetIndices;
+        };
+
+        std::vector<PoolIndex> mAvailableDescriptorPoolIndices;
+        std::vector<DescriptorPool> mDescriptorPools;
+
+        struct Deallocation {
+            PoolIndex poolIndex;
+            SetIndex setIndex;
+        };
+        SerialQueue<Deallocation> mPendingDeallocations;
+        Serial mLastDeallocationSerial = 0;
+    };
+
+}}  // namespace dawn_native::vulkan
+
+#endif  // DAWNNATIVE_VULKAN_DESCRIPTORSETALLOCATOR_H_

src/dawn_native/vulkan/DescriptorSetService.cpp

@@ -1,41 +0,0 @@
-// Copyright 2019 The Dawn Authors
-//
-// 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
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "dawn_native/vulkan/DescriptorSetService.h"
-
-#include "dawn_native/vulkan/BindGroupLayoutVk.h"
-#include "dawn_native/vulkan/DeviceVk.h"
-
-namespace dawn_native { namespace vulkan {
-
-    DescriptorSetService::DescriptorSetService(Device* device) : mDevice(device) {
-    }
-
-    DescriptorSetService::~DescriptorSetService() {
-        ASSERT(mDeallocations.Empty());
-    }
-
-    void DescriptorSetService::AddDeferredDeallocation(BindGroupLayout* layout, size_t index) {
-        mDeallocations.Enqueue({layout, index}, mDevice->GetPendingCommandSerial());
-    }
-
-    void DescriptorSetService::Tick(Serial completedSerial) {
-        for (Deallocation& dealloc : mDeallocations.IterateUpTo(completedSerial)) {
-            dealloc.layout->FinishDeallocation(dealloc.index);
-        }
-
-        mDeallocations.ClearUpTo(completedSerial);
-    }
-
-}}  // namespace dawn_native::vulkan

src/dawn_native/vulkan/DescriptorSetService.h

@@ -1,53 +0,0 @@
-// Copyright 2019 The Dawn Authors
-//
-// 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
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#ifndef DAWNNATIVE_VULKAN_DESCRIPTORSETSERVICE_H_
-#define DAWNNATIVE_VULKAN_DESCRIPTORSETSERVICE_H_
-
-#include "common/SerialQueue.h"
-
-#include "dawn_native/vulkan/BindGroupLayoutVk.h"
-
-#include <vector>
-
-namespace dawn_native { namespace vulkan {
-
-    class BindGroupLayout;
-    class Device;
-
-    // Handles everything related to descriptor sets that isn't tied to a particular
-    // BindGroupLayout.
-    class DescriptorSetService {
-      public:
-        DescriptorSetService(Device* device);
-        ~DescriptorSetService();
-
-        // Will call layout->FinishDeallocation when the serial is passed.
-        void AddDeferredDeallocation(BindGroupLayout* layout, size_t index);
-
-        void Tick(Serial completedSerial);
-
-      private:
-        Device* mDevice;
-
-        struct Deallocation {
-            Ref<BindGroupLayout> layout;
-            size_t index;
-        };
-        SerialQueue<Deallocation> mDeallocations;
-    };
-
-}}  // namespace dawn_native::vulkan
-
-#endif  // DAWNNATIVE_VULKAN_DESCRIPTORSETSERVICE_H_

src/dawn_native/vulkan/DeviceVk.cpp

@@ -27,7 +27,6 @@
 #include "dawn_native/vulkan/BufferVk.h"
 #include "dawn_native/vulkan/CommandBufferVk.h"
 #include "dawn_native/vulkan/ComputePipelineVk.h"
-#include "dawn_native/vulkan/DescriptorSetService.h"
 #include "dawn_native/vulkan/FencedDeleter.h"
 #include "dawn_native/vulkan/PipelineLayoutVk.h"
 #include "dawn_native/vulkan/QueueVk.h"
@@ -83,7 +82,6 @@ namespace dawn_native { namespace vulkan {
             mDeleter = std::make_unique<FencedDeleter>(this);
         }
 
-        mDescriptorSetService = std::make_unique<DescriptorSetService>(this);
         mMapRequestTracker = std::make_unique<MapRequestTracker>(this);
         mRenderPassCache = std::make_unique<RenderPassCache>(this);
         mResourceMemoryAllocator = std::make_unique<ResourceMemoryAllocator>(this);
@@ -173,7 +171,12 @@ namespace dawn_native { namespace vulkan {
         CheckPassedFences();
         RecycleCompletedCommands();
 
-        mDescriptorSetService->Tick(mCompletedSerial);
+        for (Ref<BindGroupLayout>& bgl :
+             mBindGroupLayoutsPendingDeallocation.IterateUpTo(mCompletedSerial)) {
+            bgl->FinishDeallocation(mCompletedSerial);
+        }
+        mBindGroupLayoutsPendingDeallocation.ClearUpTo(mCompletedSerial);
+
         mMapRequestTracker->Tick(mCompletedSerial);
         mResourceMemoryAllocator->Tick(mCompletedSerial);
         mDeleter->Tick(mCompletedSerial);
@@ -213,10 +216,6 @@ namespace dawn_native { namespace vulkan {
         return mMapRequestTracker.get();
     }
 
-    DescriptorSetService* Device::GetDescriptorSetService() const {
-        return mDescriptorSetService.get();
-    }
-
     FencedDeleter* Device::GetFencedDeleter() const {
         return mDeleter.get();
     }
@@ -225,6 +224,10 @@ namespace dawn_native { namespace vulkan {
         return mRenderPassCache.get();
     }
 
+    void Device::EnqueueDeferredDeallocation(BindGroupLayout* bindGroupLayout) {
+        mBindGroupLayoutsPendingDeallocation.Enqueue(bindGroupLayout, GetPendingCommandSerial());
+    }
+
     CommandRecordingContext* Device::GetPendingRecordingContext() {
         ASSERT(mRecordingContext.commandBuffer != VK_NULL_HANDLE);
         mRecordingContext.used = true;
@@ -808,7 +811,6 @@ namespace dawn_native { namespace vulkan {
         mDeleter->Tick(mCompletedSerial);
 
         mMapRequestTracker = nullptr;
-        mDescriptorSetService = nullptr;
 
         // The VkRenderPasses in the cache can be destroyed immediately since all commands referring
         // to them are guaranteed to be finished executing.

src/dawn_native/vulkan/DeviceVk.h

@@ -34,8 +34,8 @@
 namespace dawn_native { namespace vulkan {
 
     class Adapter;
+    class BindGroupLayout;
     class BufferUploader;
-    class DescriptorSetService;
     class FencedDeleter;
     class MapRequestTracker;
     class RenderPassCache;
@@ -58,7 +58,6 @@ namespace dawn_native { namespace vulkan {
         VkQueue GetQueue() const;
 
         BufferUploader* GetBufferUploader() const;
-        DescriptorSetService* GetDescriptorSetService() const;
         FencedDeleter* GetFencedDeleter() const;
         MapRequestTracker* GetMapRequestTracker() const;
         RenderPassCache* GetRenderPassCache() const;
@@ -67,6 +66,8 @@ namespace dawn_native { namespace vulkan {
         Serial GetPendingCommandSerial() const override;
         MaybeError SubmitPendingCommands();
 
+        void EnqueueDeferredDeallocation(BindGroupLayout* bindGroupLayout);
+
         // Dawn Native API
 
         TextureBase* CreateTextureWrappingVulkanImage(
@@ -147,7 +148,7 @@ namespace dawn_native { namespace vulkan {
         uint32_t mQueueFamily = 0;
         VkQueue mQueue = VK_NULL_HANDLE;
 
-        std::unique_ptr<DescriptorSetService> mDescriptorSetService;
+        SerialQueue<Ref<BindGroupLayout>> mBindGroupLayoutsPendingDeallocation;
         std::unique_ptr<FencedDeleter> mDeleter;
         std::unique_ptr<MapRequestTracker> mMapRequestTracker;
         std::unique_ptr<ResourceMemoryAllocator> mResourceMemoryAllocator;

src/tests/end2end/BindGroupTests.cpp

@@ -892,4 +892,32 @@ TEST_P(BindGroupTests, LastReferenceToBindGroupLayout) {
     }
 }
 
+// Test that bind groups with an empty bind group layout may be created and used.
+TEST_P(BindGroupTests, EmptyLayout) {
+    wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(device, {});
+    wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {});
+    wgpu::ComputePipelineDescriptor pipelineDesc = {
+        .layout = utils::MakeBasicPipelineLayout(device, &bgl),
+        .computeStage =
+            {
+                .module = utils::CreateShaderModule(device, utils::SingleShaderStage::Compute, R"(
+                    #version 450
+                    void main() {
+                    })"),
+                .entryPoint = "main",
+            },
+    };
+    wgpu::ComputePipeline pipeline = device.CreateComputePipeline(&pipelineDesc);
+
+    wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+    wgpu::ComputePassEncoder pass = encoder.BeginComputePass();
+    pass.SetPipeline(pipeline);
+    pass.SetBindGroup(0, bg);
+    pass.Dispatch(1);
+    pass.EndPass();
+
+    wgpu::CommandBuffer commands = encoder.Finish();
+    queue.Submit(1, &commands);
+}
+
 DAWN_INSTANTIATE_TEST(BindGroupTests, D3D12Backend(), MetalBackend(), OpenGLBackend(), VulkanBackend());

src/tests/end2end/DeprecatedAPITests.cpp

@@ -185,10 +185,6 @@ TEST_P(DeprecationTests, BGLDescBindingAndEntriesIsInvalid) {
 
 // Test that creating a BGL with both entries and bindings to 0 doesn't emit warnings
 TEST_P(DeprecationTests, BGLDescBindingAndEntriesBothZeroEmitsNoWarning) {
-    // TODO(cwallez@chromium.org): In Vulkan it is disallowed to create 0-sized descriptor pools
-    // but the Vulkan backend doesn't special case it yet.
-    DAWN_SKIP_TEST_IF(IsVulkan());
-
     wgpu::BindGroupLayoutDescriptor bglDesc = {
         .bindingCount = 0,
         .bindings = nullptr,
@@ -270,10 +266,6 @@ TEST_P(DeprecationTests, BGDescBindingAndEntriesIsInvalid) {
 
 // Test that creating a BG with both entries and bindings to 0 doesn't emit warnings
 TEST_P(DeprecationTests, BGDescBindingAndEntriesBothZeroEmitsNoWarning) {
-    // TODO(cwallez@chromium.org): In Vulkan it is disallowed to create 0-sized descriptor pools
-    // but the Vulkan backend doesn't special case it yet.
-    DAWN_SKIP_TEST_IF(IsVulkan());
-
     wgpu::BindGroupLayout layout = utils::MakeBindGroupLayout(device, {});
 
     wgpu::BindGroupDescriptor bgDesc = {