From 642009261e270d1cb5a18999de74d887aca08cb0 Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Thu, 23 Apr 2020 19:56:32 +0000 Subject: [PATCH] Reland "Slab-allocate VkDescriptorSets" This is a reland of 96c4019214e0b2d7c0843eacf96398a6a1198d1f 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 > Commit-Queue: Austin Eng Bug: dawn:340 Change-Id: Iabb744f110d0cab442bb857b31c87ba46bf0ad7a Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/20321 Commit-Queue: Austin Eng Reviewed-by: Corentin Wallez --- src/dawn_native/BUILD.gn | 5 +- src/dawn_native/CMakeLists.txt | 5 +- src/dawn_native/vulkan/BindGroupLayoutVk.cpp | 97 ++-------- src/dawn_native/vulkan/BindGroupLayoutVk.h | 30 +-- src/dawn_native/vulkan/BindGroupVk.cpp | 3 +- src/dawn_native/vulkan/BindGroupVk.h | 1 + .../vulkan/DescriptorSetAllocation.h | 29 +++ .../vulkan/DescriptorSetAllocator.cpp | 179 ++++++++++++++++++ .../vulkan/DescriptorSetAllocator.h | 70 +++++++ .../vulkan/DescriptorSetService.cpp | 41 ---- src/dawn_native/vulkan/DescriptorSetService.h | 53 ------ src/dawn_native/vulkan/DeviceVk.cpp | 18 +- src/dawn_native/vulkan/DeviceVk.h | 7 +- src/tests/end2end/BindGroupTests.cpp | 28 +++ src/tests/end2end/DeprecatedAPITests.cpp | 8 - 15 files changed, 352 insertions(+), 222 deletions(-) create mode 100644 src/dawn_native/vulkan/DescriptorSetAllocation.h create mode 100644 src/dawn_native/vulkan/DescriptorSetAllocator.cpp create mode 100644 src/dawn_native/vulkan/DescriptorSetAllocator.h delete mode 100644 src/dawn_native/vulkan/DescriptorSetService.cpp delete mode 100644 src/dawn_native/vulkan/DescriptorSetService.h diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn index 432d016164..42511960bb 100644 --- a/src/dawn_native/BUILD.gn +++ b/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/DescriptorSetService.h", + "vulkan/DescriptorSetAllocation.h", + "vulkan/DescriptorSetAllocator.cpp", + "vulkan/DescriptorSetAllocator.h", "vulkan/DeviceVk.cpp", "vulkan/DeviceVk.h", "vulkan/ExternalHandle.h", diff --git a/src/dawn_native/CMakeLists.txt b/src/dawn_native/CMakeLists.txt index a1d59ab35a..9d78325508 100644 --- a/src/dawn_native/CMakeLists.txt +++ b/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/DescriptorSetService.h" + "vulkan/DescriptorSetAllocation.h" + "vulkan/DescriptorSetAllocator.cpp" + "vulkan/DescriptorSetAllocator.h" "vulkan/DeviceVk.cpp" "vulkan/DeviceVk.h" "vulkan/ExternalHandle.h" diff --git a/src/dawn_native/vulkan/BindGroupLayoutVk.cpp b/src/dawn_native/vulkan/BindGroupLayoutVk.cpp index 11463b56ef..ba41c38545 100644 --- a/src/dawn_native/vulkan/BindGroupLayoutVk.cpp +++ b/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()); - for (const auto& it : descriptorCountPerType) { - mPoolSizes.push_back(VkDescriptorPoolSize{it.first, it.second}); - } - + // TODO(enga): Consider deduping allocators for layouts with the same descriptor type + // counts. + mDescriptorSetAllocator = + std::make_unique(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 BindGroupLayout::AllocateOneDescriptorSet() { - Device* device = ToBackend(GetDevice()); - - // 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(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); + void BindGroupLayout::FinishDeallocation(Serial completedSerial) { + mDescriptorSetAllocator->FinishDeallocation(completedSerial); } }} // namespace dawn_native::vulkan diff --git a/src/dawn_native/vulkan/BindGroupLayoutVk.h b/src/dawn_native/vulkan/BindGroupLayoutVk.h index e06b36423a..31dd1fad77 100644 --- a/src/dawn_native/vulkan/BindGroupLayoutVk.h +++ b/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 AllocateBindGroup(Device* device, const BindGroupDescriptor* descriptor); - void DeallocateBindGroup(BindGroup* bindGroup); - - ResultOrError AllocateOneDescriptorSet(); - void DeallocateDescriptorSet(DescriptorSetAllocation* descriptorSetAllocation); - - // Interaction with the DescriptorSetService. - void FinishDeallocation(size_t index); + void DeallocateBindGroup(BindGroup* bindGroup, + DescriptorSetAllocation* descriptorSetAllocation); + void FinishDeallocation(Serial completedSerial); private: ~BindGroupLayout() override; MaybeError Initialize(); - std::vector mPoolSizes; - - struct SingleDescriptorSetAllocation { - VkDescriptorPool pool = VK_NULL_HANDLE; - // Descriptor sets are freed when the pool is destroyed. - VkDescriptorSet set = VK_NULL_HANDLE; - }; - std::vector mAllocations; - std::vector mAvailableAllocations; - VkDescriptorSetLayout mHandle = VK_NULL_HANDLE; SlabAllocator mBindGroupAllocator; + std::unique_ptr mDescriptorSetAllocator; }; }} // namespace dawn_native::vulkan diff --git a/src/dawn_native/vulkan/BindGroupVk.cpp b/src/dawn_native/vulkan/BindGroupVk.cpp index 3ae94cd0a0..a936f20b60 100644 --- a/src/dawn_native/vulkan/BindGroupVk.cpp +++ b/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); + ToBackend(GetLayout())->DeallocateBindGroup(this, &mDescriptorSetAllocation); } VkDescriptorSet BindGroup::GetHandle() const { diff --git a/src/dawn_native/vulkan/BindGroupVk.h b/src/dawn_native/vulkan/BindGroupVk.h index ce6644589f..411c114940 100644 --- a/src/dawn_native/vulkan/BindGroupVk.h +++ b/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 { diff --git a/src/dawn_native/vulkan/DescriptorSetAllocation.h b/src/dawn_native/vulkan/DescriptorSetAllocation.h new file mode 100644 index 0000000000..c1f4310bbf --- /dev/null +++ b/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_ diff --git a/src/dawn_native/vulkan/DescriptorSetAllocator.cpp b/src/dawn_native/vulkan/DescriptorSetAllocator.cpp new file mode 100644 index 0000000000..9f1999c3a5 --- /dev/null +++ b/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 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 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(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 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 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 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 diff --git a/src/dawn_native/vulkan/DescriptorSetAllocator.h b/src/dawn_native/vulkan/DescriptorSetAllocator.h new file mode 100644 index 0000000000..44ea4a4325 --- /dev/null +++ b/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 +#include + +namespace dawn_native { namespace vulkan { + + class BindGroupLayout; + + class DescriptorSetAllocator { + using PoolIndex = uint32_t; + using SetIndex = uint16_t; + + public: + DescriptorSetAllocator(BindGroupLayout* layout, + std::map descriptorCountPerType); + ~DescriptorSetAllocator(); + + ResultOrError Allocate(); + void Deallocate(DescriptorSetAllocation* allocationInfo); + void FinishDeallocation(Serial completedSerial); + + private: + MaybeError AllocateDescriptorPool(); + + BindGroupLayout* mLayout; + + std::vector mPoolSizes; + SetIndex mMaxSets; + + struct DescriptorPool { + VkDescriptorPool vkPool; + std::vector sets; + std::vector freeSetIndices; + }; + + std::vector mAvailableDescriptorPoolIndices; + std::vector mDescriptorPools; + + struct Deallocation { + PoolIndex poolIndex; + SetIndex setIndex; + }; + SerialQueue mPendingDeallocations; + Serial mLastDeallocationSerial = 0; + }; + +}} // namespace dawn_native::vulkan + +#endif // DAWNNATIVE_VULKAN_DESCRIPTORSETALLOCATOR_H_ diff --git a/src/dawn_native/vulkan/DescriptorSetService.cpp b/src/dawn_native/vulkan/DescriptorSetService.cpp deleted file mode 100644 index 6aa26bbcfa..0000000000 --- a/src/dawn_native/vulkan/DescriptorSetService.cpp +++ /dev/null @@ -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 diff --git a/src/dawn_native/vulkan/DescriptorSetService.h b/src/dawn_native/vulkan/DescriptorSetService.h deleted file mode 100644 index c898b051bd..0000000000 --- a/src/dawn_native/vulkan/DescriptorSetService.h +++ /dev/null @@ -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 - -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 layout; - size_t index; - }; - SerialQueue mDeallocations; - }; - -}} // namespace dawn_native::vulkan - -#endif // DAWNNATIVE_VULKAN_DESCRIPTORSETSERVICE_H_ diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp index f3b176b1ef..4d2040a1b7 100644 --- a/src/dawn_native/vulkan/DeviceVk.cpp +++ b/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(this); } - mDescriptorSetService = std::make_unique(this); mMapRequestTracker = std::make_unique(this); mRenderPassCache = std::make_unique(this); mResourceMemoryAllocator = std::make_unique(this); @@ -173,7 +171,12 @@ namespace dawn_native { namespace vulkan { CheckPassedFences(); RecycleCompletedCommands(); - mDescriptorSetService->Tick(mCompletedSerial); + for (Ref& 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. diff --git a/src/dawn_native/vulkan/DeviceVk.h b/src/dawn_native/vulkan/DeviceVk.h index 63758c1f76..cd08aede7f 100644 --- a/src/dawn_native/vulkan/DeviceVk.h +++ b/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 mDescriptorSetService; + SerialQueue> mBindGroupLayoutsPendingDeallocation; std::unique_ptr mDeleter; std::unique_ptr mMapRequestTracker; std::unique_ptr mResourceMemoryAllocator; diff --git a/src/tests/end2end/BindGroupTests.cpp b/src/tests/end2end/BindGroupTests.cpp index e3289cf944..7efe9f4679 100644 --- a/src/tests/end2end/BindGroupTests.cpp +++ b/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()); diff --git a/src/tests/end2end/DeprecatedAPITests.cpp b/src/tests/end2end/DeprecatedAPITests.cpp index 17d5e14ffd..d7ea7541c5 100644 --- a/src/tests/end2end/DeprecatedAPITests.cpp +++ b/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 = {