[drape][Vulkan] Improve descriptor sets allocation #8276

2024-05-24T15:04:47Z

renderexpert commented

2024-05-24 15:04:47 +00:00

(Migrated from github.com)

Motivation:

Current implementation call vkAllocateDescriptorSets multiple times until it finishes successfully. It's not efficient and may cause validation issues.
Example:

OMcore                  app.organicmaps.debug                E  vulkan/vulkan_layers.cpp:162 DebugReportCallbackImpl(): Vulkan Diagnostics [ Validation ] [ DESCRIPTOR_POOL ] [OBJ: 18065711617638662146 LOC: 1434762579 ]: Validation Error: [ VUID-VkDescriptorSetAllocateInfo-apiVersion-07896 ] Object 0: handle = 0xfab64d0000000002, type = VK_OBJECT_TYPE_DESCRIPTOR_POOL; | MessageID = 0x5584bd53 | vkAllocateDescriptorSets():  Unable to allocate 1 descriptors of type VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC from VkDescriptorPool 0xfab64d0000000002[]. This pool only has 0 descriptors of this type remaining. The Vulkan spec states: If the VK_KHR_maintenance1 extension is not enabled and VkPhysicalDeviceProperties::apiVersion is less than Vulkan 1.1, descriptorPool must have enough free descriptor capacity remaining to allocate the descriptor sets of the specified layouts (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkDescriptorSetAllocateInfo-apiVersion-07896)

New approach:

We calculate descriptor's pool capacity and check if there is enough space in it.
According to Vulkan's spec (https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkDescriptorPoolCreateInfo.html):

If a descriptor pool has not had any descriptor sets freed since it was created or most recently reset then fragmentation must not cause an allocation failure (note that this is always the case for a pool created without the VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT bit set). Additionally, if all sets allocated from the pool since it was created or most recently reset use the same number of descriptors (of each type) and the requested allocation also uses that same number of descriptors (of each type), then fragmentation must not cause an allocation failure.

It means that we need to maintain the same descriptors set size to avoid fragmentation.

# Motivation: Current implementation call `vkAllocateDescriptorSets` multiple times until it finishes successfully. It's not efficient and may cause validation issues. Example: ``` OMcore app.organicmaps.debug E vulkan/vulkan_layers.cpp:162 DebugReportCallbackImpl(): Vulkan Diagnostics [ Validation ] [ DESCRIPTOR_POOL ] [OBJ: 18065711617638662146 LOC: 1434762579 ]: Validation Error: [ VUID-VkDescriptorSetAllocateInfo-apiVersion-07896 ] Object 0: handle = 0xfab64d0000000002, type = VK_OBJECT_TYPE_DESCRIPTOR_POOL; | MessageID = 0x5584bd53 | vkAllocateDescriptorSets(): Unable to allocate 1 descriptors of type VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC from VkDescriptorPool 0xfab64d0000000002[]. This pool only has 0 descriptors of this type remaining. The Vulkan spec states: If the VK_KHR_maintenance1 extension is not enabled and VkPhysicalDeviceProperties::apiVersion is less than Vulkan 1.1, descriptorPool must have enough free descriptor capacity remaining to allocate the descriptor sets of the specified layouts (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkDescriptorSetAllocateInfo-apiVersion-07896) ``` # New approach: We calculate descriptor's pool capacity and check if there is enough space in it. According to Vulkan's spec (https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkDescriptorPoolCreateInfo.html): > If a descriptor pool has not had any descriptor sets freed since it was created or most recently reset then fragmentation must not cause an allocation failure (note that this is always the case for a pool created without the VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT bit set). Additionally, if all sets allocated from the pool since it was created or most recently reset use the same number of descriptors (of each type) and the requested allocation also uses that same number of descriptors (of each type), then fragmentation must not cause an allocation failure. It means that we need to maintain the same descriptors set size to avoid fragmentation.

🚀 1

Jean-BaptisteC (Migrated from github.com) approved these changes 2024-06-02 06:50:33 +00:00

Jean-BaptisteC (Migrated from github.com) left a comment

✅ Pixel 6 - Android 14

biodranik (Migrated from github.com) reviewed 2024-06-05 17:40:11 +00:00

biodranik (Migrated from github.com) left a comment

This commit was tested in the alpha version, looks like it works.

@vng WDYT?

This commit was tested in the alpha version, looks like it works. @vng WDYT?

vng (Migrated from github.com) approved these changes 2024-06-05 22:52:05 +00:00

vng (Migrated from github.com) reviewed 2024-06-06 13:32:53 +00:00

shaders/vulkan_program_pool.cpp

vng (Migrated from github.com) commented

2024-06-06 13:32:19 +00:00

ranges ?

biodranik (Migrated from github.com) reviewed 2024-06-06 17:17:37 +00:00

shaders/vulkan_program_pool.cpp

biodranik (Migrated from github.com) commented

2024-06-06 17:17:37 +00:00

@vng Yes, do you have any issues with it in mind?

biodranik (Migrated from github.com) reviewed 2024-06-06 20:08:59 +00:00

shaders/vulkan_program_pool.cpp

					
				@ -204,2 +225,4 @@

				  auto packFileData = ReadShadersPackFile(base::JoinPath(kShadersDir, kShadersPackFile));

				  for (size_t i = 0; i < static_cast<size_t>(Program::ProgramsCount); ++i)

				    m_maxImageSamplers = std::max(m_maxImageSamplers, static_cast<uint32_t>(reflection[i].m_info.m_textures.size()));

biodranik (Migrated from github.com) commented

2024-06-06 20:08:59 +00:00

@organicmaps/core note this cool C++20 syntax to fill struct info.

Although for simpler structs with less fields more efficient way could be to use

result.emplace_back(bindingIndex, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER);

@organicmaps/core note this cool C++20 syntax to fill struct info. Although for simpler structs with less fields more efficient way could be to use ``` result.emplace_back(bindingIndex, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER); ```