Add option to disallow increasing capacity when allocating#267
Open
ftilde wants to merge 1 commit intoTraverse-Research:mainfrom
Open
Add option to disallow increasing capacity when allocating#267ftilde wants to merge 1 commit intoTraverse-Research:mainfrom
ftilde wants to merge 1 commit intoTraverse-Research:mainfrom
Conversation
Member
|
Can't you just use the dedicated memory feature at that point? |
Member
|
Just noting that for the title, I'd have expected this to say something like |
This allows the user to bound the growth of the capacity without relying on the driver to return ERROR_OUT_OF_DEVICE_MEMORY (which they sometimes don't).
ftilde
force-pushed
the
control-capacity-increase
branch
from
562b463 to
0f0661d
Compare
MarijnS95
changed the title
Contributor
Author
|
On 28.02.2025 05:09, Jasper Bekkers wrote:
Can't you just use the dedicated memory feature at that point?
Hm, I'm not sure what you mean. Could you expand on that? Do you suggest
using VK_KHR_dedicated_allocation/the respective Vk1.1 functionality/the
respective AllocationScheme? If that's the case I'm not sure how it
helps my problem, since (1) I'm not sure how much overhead the driver
will have due to fragmentation and (2) the number of allocations
(maxMemoryAllocationCount) can be quite limited apparently (e.g. 4096).
|
Comment on lines
+584
to
+587
| if !desc.allow_capacity_increase { | ||
| return Err(AllocationError::OutOfMemory); | ||
| } | ||
|
|
Member
There was a problem hiding this comment.
This would need to be implemented on all backends for feature parity before merging.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As it turns out, exposing the capacity (#266) is not quite enough for my use case to restrict the size of the capacity (because I don't know when a new allocation would increase the capacity).
There are multiple ways I can think of on how to achieve that:
I the PR I (for now) implemented the latter option since I consider it to be the most flexible. I'm open to changing the implementation to one of the other options (or another idea), however.
Also note that I have only implemented the proposed change for Vulkan so far. If you like the proposed change I can add the same for D3D12 and metal.
As another motivation for my use case: I just tried to use the allocator without any upper bound on the total allocation size or capacity, and it just keeps allocating until, first, VRAM memory is swapped to GTT (with huge slowdowns) and finally (when the GTT is full) the compositor becomes unresponsive, i.e. my graphical session freezes. It seems like some drivers just never return ERROR_OUT_OF_DEVICE_MEMORY.