Vulkan backend performance validation layer warning

[Makogan]

Version: 18307
Branch: master
Back-ends: imgui_impl_vulkan.cpp
Compiler: g++ 10
Operating System: Ubuntu 20.04

Imgui rendering causes performance warnings with the performance validation layers enabled (i.e. vk::ValidationFeatureEnableEXT::eBestPractices). For example the function ImGui_ImplVulkan_RenderDrawData will give the following warnings:

Message ID name: UNASSIGNED-BestPractices-vkAllocateMemory-small-allocation
Message: Validation Performance Warning: [ UNASSIGNED-BestPractices-vkAllocateMemory-small-allocation ] Object 0: handle = 0x5555568a5218, name = Logical device: NVIDIA GeForce GT 1030, type = VK_OBJECT_TYPE_DEVICE; | MessageID = 0xdc18ad6b | vkAllocateMemory(): Allocating a VkDeviceMemory of size 2304. This is a very small allocation (current threshold is 262144 bytes). You should make large allocations and sub-allocate from one large VkDeviceMemory.
Severity: VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT

The problem is these errors drown errors within the parent program, making it more difficult to asses performance bugs for release builds, and vulkan doesn't have a mechanism to delete valitation in specific snippets.

For example, create the instance like this:

    auto enabled = vk::ValidationFeatureEnableEXT::eBestPractices;
    vk::ValidationFeaturesEXT features;
    features.enabledValidationFeatureCount = 1;
    features.pEnabledValidationFeatures = &enabled;

    // Create Vulkan instance to communicate with the loader
    vk::InstanceCreateInfo create_info = {};
    create_info.pNext = &features;
    create_info.pApplicationInfo = &program_info,
    create_info.enabledLayerCount = static_cast<uint32_t>(VALIDATION_LAYERS.size()),
    create_info.ppEnabledLayerNames = VALIDATION_LAYERS.data(),
    create_info.enabledExtensionCount = static_cast<uint32_t>(required_extensions.size()),
    create_info.ppEnabledExtensionNames = required_extensions.data();

    auto [result, instance] = vk::createInstanceUnique(create_info);

And call ImGui_ImplVulkan_RenderDrawData in the render loop.

[PathogenDavid]

While there are probably some improvements that could be made to the way memory is allocated in the backend (I know I've been sitting on some improvements to the D3D12 backend for a while), properly following the intent of that best practice would require either adding a lot of complexity or constraining the design of the Vulkan backend for what is probably marginal performance gains at best.

vulkan doesn't have a mechanism to delete valitation in specific snippets.

I'm not super familiar with the Vulkan validation layer, but my understanding is that you can use debug message callbacks to accomplish this.

Otherwise, you could just temporarily modify CreateOrResizeBuffer to over-allocate and effectively suppress the message. Or if you aren't concerned about UNASSIGNED-BestPractices-vkAllocateMemory-small-allocation appearing in your app, maybe just suppress it entirely.

[ocornut]

Posted by @rickyjames35 in #7027

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?

I believe this warning layer is idiotic. In principle it makes senses to not over-allocate small fragmented blocks, but a single warning layer enabled across a whole application doesn't seem sane and it only drives people to blindly think in terms of satisfying a warning layer at a point in their development path where they probably don't need to.

That said:

• On the specific suggestion of using VMA. We are not going to force a dependence on extra libraries like this one, for sure. But if that libraries follows the VkAllocationCallbacks API (whatever that is) it may be already to use it by filling this field in the ImGui_ImplVulkan_InitInfo structure.
• If that warning layer happens to be popular, we can consider adding an opt-in flag to pad some allocation sizes in other to satisfy it, which is a rather stupid answer to a warning. But we would like to know exactly WHICH allocation are flagged as such.

On a personal level, when you say "I'm still new to Vulkan" my sincere and well-intended suggestion is that you should not be enabling a warning of this kind as it would lead you to over-engineer and distract you from learning.

Perhaps the warning layer in question has a system to toggle specific warnings?

[PathogenDavid]

• But if that libraries follows the VkAllocationCallbacks API (whatever that is) it may be already to use it by filling this field in the ImGui_ImplVulkan_InitInfo structure.

VkAllocationCallbacks is for CPU-side allocations, it's not relevant in this context.

But we would like to know exactly WHICH allocation are flagged as such.

Pretty sure it's the mesh data, which is allocated somewhat suboptimally, but not for lack of honoring this specific validation layer warning. (IE: If we're gonna get pedantic about how the D3D12/Vulkan backends allocate mesh data, there's easier wins without pulling in AMD's memory allocator libraries--this is what I was getting at in my 2021 comment.)

Since the buffers are only allocated when they need to grow and there's only two of them, anything to be gained here will be extraordinarily marginal performance-wise.

On a personal level, when you say "I'm still new to Vulkan" my sincere and well-intended suggestion is that you should not be enabling a warning of this kind as it would lead you to over-engineer and distract you from learning.

I'm inclined to agree. If @rickyjames35 is already using Vulkan Memory Allocator this warning isn't going to be super useful anyway.

[PathogenDavid]

Here's Nvidia's take on it which is likely the reason for the validation layer warning

It's not that we don't understand the issue, it's that solving it properly isn't really worthwhile.

In the backends simplicity and easy of integration is paramount, which makes anything like Vulkan Memory Allocator a non-starter. We could just up our minimum allocation to make the validation layer happy just for the sake of it, but that's not following best practices--it's just making the validation layer happy.

I've only done minimal testing but removing that line seems to fix the warning without causing any issues.

I don't have a strong opinion on this change (seems sensible at a glance), but this is sort-of the core issue with coming to an open source project and decrying the fact that it isn't following best practices.

It'd be one thing if you knew this was a fine change because you were experienced with Vulkan--no guess and check--and were submitting a short and simple PR. But by raising issues this way it's just creating more work for everyone who works on Dear ImGui...all for something that happens once during initialization in most apps (and only occasionally in others.)

[ocornut]

Should be fixed by #7189