Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimizing Vulkan descriptors sets management #3

Open
xoofx opened this issue Jul 16, 2024 · 0 comments
Open

Optimizing Vulkan descriptors sets management #3

xoofx opened this issue Jul 16, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@xoofx
Copy link
Member

xoofx commented Jul 16, 2024

I'm planning to slightly revisit the way descriptor sets were managed in Veldrid.

The existing implementation is divided between the following classes:

  • VkResourceLayout.cs for creating vkCreateDescriptorSetLayout
  • VkResourceSet.cs for allocating and vkUpdateDescriptorSets updating descriptor sets
  • VkDescriptorPoolManager.cs that manages descriptor pools vkCreateDescriptorPool/vkDestroyDescriptorPool and vkAllocateDescriptorSets allocation/free of descriptor sets
  • ResourceSetDescription.cs is used as an abstraction to create a VkResourceSet by associating the layout and the bindables.

A couple of remarks on the existing implementation that I might revisit:

  • A VkDescriptorPoolManager is created per GraphicsDevice. It might be more adequate to create one per CommandList so that we could have one per thread and avoid contention in case of multithreading.
  • VkDescriptorPoolManager is maintaining the state of a pool by subtracting what has been allocated so far. This should not be necessary anymore with Vulkan 1.1 as vkAllocateDescriptorSets properly returns an error if the pool cannot satisfy the request.
  • Similarly, VkDescriptorPoolManager is creating descriptor pools with VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT, but it seems not recommended from the various posts about it. This would require implementing a hashing/matching of previously available descriptor sets. Hashing cannot be done directly with Vulkan handles because there are no guarantees that they cannot overlap between types or that they are not reused after being destroyed.
  • ResourceSetDescription is a bit problematic because it is using the interface IBindableResource to convey the various way to bind resources (buffer, buffer range, texture views...etc.). The problem is that it requires boxing for some of these resources (e.g buffer range is a struct) and matching a resource set description could be costly. Wondering if a hashing technique wouldn't be more efficient (e.g via XXHash128)
  • Things are still handled efficiently in VkCommandList here to not update the bindings via vkCmdBindDescriptorSets if nothing has changed here which seems good
  • The creation of a managed object VkResourceSet for each combination of bindings could be problematic. For workflows that need to change input/output frequently (a postprocess pipeline where textures in/out are changing on each draw calls). But the design is trying to force the usage of resource set as a block, instead of suggesting that partial updates would be cheap, so maybe that's an acceptable tradeoff. E.g we could expect all VkResourceSet to be created upfront, but not during a frame.
  • The existing design allows the usual split and binding of resource set per frequency, so might not need anything special here.
  • When setting a resource set, Dynamic offsets per binding are already supported here, which is good.
  • There is no API for allowing to use vkCmdPushConstants which can give an important last bit of optimization per draw call - to avoid having to modify a descriptor set, so it is something that should be probably added.

Notes about extensions:

  • Using VK_KHR_push_descriptor could be an opportunity for simplifying the management, but it is unclear if it would be better over a management at a higher level. If descriptor sets are known upfront, it seems more beneficial to have the VkWriteDescriptorSet prepared upfront from the descriptor set instead of building them on the fly. Will have to evaluate this more.
  • There is definitely a case for also supporting VK_EXT_descriptor_indexing for bindless pipeline but not sure it is a good idea to only rely on it, as it would force to develop shaders in a very specific way, but this could be acceptable for a reboot of Veldrid, so this is on the table. Because a variable array has to be last in the set, it requires to use one set per binding type, and there is a limit maxBoundDescriptorSets that could be problematic, but looking at it, it seems that all desktops (including MoltenVk on macOS) are supporting at least >= 8 descriptor sets bound, so it might be ok.
  • There is another opportunity with VK_EXT_descriptor_buffer (spec) but its availability might be problematic, and it seems to be cause trouble/crashes or not being enough stable to safely rely on it.
  • The usage of VK_KHR_buffer_device_address is also interesting as it would avoid to have to modify descriptor sets to simply pass different buffers, but it might not be well supported according to https://vulkan.gpuinfo.org/ and it requires changing quite dramatically the way shaders are written to access such resource via pointers.

Some useful posts I have visited:

@xoofx xoofx added the enhancement New feature or request label Jul 16, 2024
@xoofx xoofx mentioned this issue Jul 16, 2024
23 tasks
@xoofx xoofx changed the title Optimizing descriptors sets management Optimizing Vulkan descriptors sets management Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant