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

GenericMemoryAllocatorCreateInfo set prefers_dedicated_allocation to false if dedicated_allocation is false #2214

Closed

Conversation

charles-r-earp
Copy link
Contributor

If GenericMemoryAllocator was created with dedicated_allocation = false, then the prefers_dedicated_allocation from the buffer memory requirements will be set to false. This seems like expected behavior given the docs for GenericMemoryAllocatorCreateInfo:

"
dedicated_allocation: bool

Whether the allocator should use the dedicated allocation APIs.

This means that when the allocator decides that an allocation should not be suballocated, but rather have its own block of DeviceMemory, that that allocation will be made a dedicated allocation. Otherwise they are still made free-standing (root) allocations, just not dedicated ones.

Dedicated allocations are an optimization which may result in better performance, so there really is no reason to disable this option, unless the restrictions that they bring with them are a problem. Namely, a dedicated allocation must only be used for the resource it was created for. Meaning that reusing the memory for something else is not possible, suballocating it is not possible, and aliasing it is also not possible.

This option is silently ignored (treated as false) if the device API version is below 1.1 and the khr_dedicated_allocation extension is not enabled on the device.

The default value is true.
"

This change keeps current behavior of using a dedicated allocation if required or as a fallback. The docs imply imo that dedicated allocations won't be used if false, so potentially the docs could be clarified or the behavior modified more drastically.

Note that dedicated allocations can be really expensive (because they can't be reused) and undermine the utility of the allocator, so at least allowing the user to opt out can dramatically improve performance when dynamically allocating in a hot loop.

@marc0246
Copy link
Contributor

Quoting the docs:

Whether the allocator should use the dedicated allocation APIs.

This is talking specifically about Vulkan's dedicated allocation. That is a specific Vulkan term referring to a specific kind of allocation, not simply an allocation made exclusively for a resource, which can be done without Vulkan's dedicated allocation. That is why the docs are specifically talking about dedicated allocation APIs as opposed to regular allocation.

This means that when the allocator decides that an allocation should not be suballocated, but rather have its own block of DeviceMemory, that that allocation will be made a dedicated allocation. Otherwise they are still made free-standing (root) allocations, just not dedicated ones.

This next paragraph explains the difference: this setting doesn't change whether or not a resource gets its own block of memory, all it does it disable dedicated allocation.

Dedicated allocations are an optimization which may result in better performance, so there really is no reason to disable this option, unless the restrictions that they bring with them are a problem. Namely, a dedicated allocation must only be used for the resource it was created for. Meaning that reusing the memory for something else is not possible, suballocating it is not possible, and aliasing it is also not possible.

The paragraph after that describes what limitations a dedicated allocation imposes: unlike regular allocations, they must only be used for the resource they are dedicated to. This is a limitation I can almost guarantee you you don't care about, and as the docs say, this shouldn't be turned off unless you specifically are trying to suballocate blocks yourself, or reusing allocations yourself, or etc. and not just use the allocator indirectly e.g. Buffer::new and the like.

The change you propose isn't desirable, as the allocation of DeviceMemory should be decided on a per-allocation basis, rather than for the whole allocator, as is already possible using MemoryAllocatePreference.

All in all, I'll gladly accept a change to the docs if you know how to make this clearer. =)

@marc0246
Copy link
Contributor

Will you fix the docs? They are indeed confusing so it would be much appreciated. ❤️

I hope I managed to convey the original idea behind the docs somewhat coherently (knowing me I probably didn't) -- if there's anything you need please ask away!

@Rua
Copy link
Contributor

Rua commented Aug 26, 2023

@charles-r-earp do you still plan to work on this?

@Rua Rua closed this Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants