Vulkan Memory Allocator (VMA) Support

[rickyjames35]

Back-ends: imgui_impl_vulkan.cpp

My Issue/Question:
As reported in this issue #4238 when enabling Vulkan eBestPractices validation layers the imgui_impl_vulkan backend implementation will cause some warnings. Like so...

[warning] Validation Performance Warning: [ UNASSIGNED-BestPractices-vkAllocateMemory-small-allocation ] Object 0: handle = 0x55a52c8b36e0, type = VK_OBJECT_TYPE_DEVICE; | MessageID = 0xdc18ad6b | vkAllocateMemory(): Allocating a VkDeviceMemory of size 77568. This is a very small allocation (current threshold is 262144 bytes). You should make large allocations and sub-allocate from one large VkDeviceMemory.

The fix for this would be to use a more sophisticated memory allocator like VMA. I'm wondering if it would be somehow possible to hook it up to the imgui_impl_vulkan.cpp. I haven't found any examples online of how to do this and I'm still new to Vulkan and Dear IMGUI so I'm wondering if anyone else has already looked into doing this?

[ocornut]

Moving this conversation to existing topic #4238.

[ocornut]

Should be fixed by #7189

[DexterHaxxor]

@ocornut
That's not a fix, it's just hiding the issue. The real issue here is what the code is actually doing: allocating one VkDeviceMemory per resource instead of using a proper memory allocator. Since the vast majority of Vulkan users are using VMA, I think the Vulkan backend should support (but not require) supplying a VMA allocator, which it would then use instead of allocating a VkDeviceMemory for every resource.

[ocornut]

The real issue here is what the code is actually doing: allocating one VkDeviceMemory per resource instead of using a proper memory allocator.

We are talking about 2 VkDeviceMemory per native window, which tends to grow big enough anyway if you are actually using dear imgui. That's not an actual issue. It's just that it's not following your own coding style and convention. The real issue here is that if you want third-party software to follow your arbitrary convention even if it's provably doesn't bring any tangible value.

Since the vast majority of Vulkan users are using VMA

I don't agree nor care much about this statement. If a non-trivial fraction of Vulkan users are using VMA I am open to support their need if there is a problem to solve. I'm open to discuss/review a PR that allowed some use of VMA allocators ASSUMING it doesn't make the backend codebase sensibly worse.

[DexterHaxxor]

Sorry, by the amount of lines that VL are spewing at me about allocations being too small, I did not get the impression that it was allocating only 2 VkDeviceMemory objects.

Anyway, yes, adding support for VMA without breaking anything is possible to do with a couple of macros, by:

• adding a VmaAllocator parameter to ImGui_ImplVulkan_InitInfo (possibly replacing MinAllocationSize)
• replacing VkDeviceMemory with VmaAllocation in structs
• disabling the code which allocates/deallocates the memory
• using VMA's functions to allocate and deallocate the resources instead of Vulkan's, directly giving it the pointer to VmaAllocation

I'll try to make something, but let me know your thoughts.

[ocornut]

I did not get the impression that it was allocating only 2 VkDeviceMemory objects.

It's allocating 1 font texture (can be shared between contexts and multiple viewports) + 1 vertex buffer + 1 index buffer for each visible viewport. As the buffer are likely to grow in the first few frames it tends to allocate twice a few in first few frames, and then generally the "contents growth" frames only when buffer needs to grow.

I'm open to said PR, but understand that my tolerance level for increased cognitive load in the codebase (for both end-users and backend editors) will be made lower by the fact this is not solving anything important. It would be merely a development to satisfy a niggle that other VMA users will inevitably (and somehow irrationally) have.

[DexterHaxxor]

Alright then, probably not worth it.