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

Add reserved_bytes method for Allocator #266

Merged
merged 2 commits into from
Feb 27, 2025

Conversation

ftilde
Copy link
Contributor

@ftilde ftilde commented Feb 24, 2025

This fills the use case where one needs the total number of reserved bytes from an Allocator, but compiling a complete report (in particular due to the calls to report_allocations from the SubAllocator) would be too costly.

I've encountered this in a (not yet open) project of mine and would be happy if you would accept this contribution. My workaround of just counting the allocation size myself underestimates the total allocated memory due to fragmentation.

@Jasper-Bekkers
Copy link
Member

Jasper-Bekkers commented Feb 24, 2025

Thanks for the contribution, please don't forget to sign the commits.

What's the intended use-case for this, and would it be OK to chose another name so it doesn't conflict with the Dx12 feature of reserved memory (CreateReservedResource)? I tend to want to stay away from overloading terms as much as possible (though not too extreme).

@ftilde
Copy link
Contributor Author

ftilde commented Feb 24, 2025 via email

@MarijnS95
Copy link
Member

Sure! Do you have a preference?

How about picking the capacity term from Rust? @Jasper-Bekkers wdyt?

I picked this name due to the similar name of the property of AllocatorReport and was not aware of the Dx12 feature.

Ouch, maybe we should update that as well in-line with this choice.

@Jasper-Bekkers
Copy link
Member

Capacity would be the proper name here indeed. We should also update the reporting api in that case.

The use of "reserved" may be confused with the concept of reserved
memory in Dx12. Capacity is in line with the similar concept from (e.g.)
std::vec::Vec.
This fills the use case where one needs the total capacity given by the
sum of sizes of all device-allocated memory blocks, but compiling a
complete report (in particular due to the calls to report_allocations
from the SubAllocator) would be too costly.
@ftilde
Copy link
Contributor Author

ftilde commented Feb 25, 2025

Alright, I've added a commit to change AllocatorReport, changed the name of the added method and signed the commits.

@Jasper-Bekkers Jasper-Bekkers merged commit 642dc91 into Traverse-Research:main Feb 27, 2025
13 checks passed
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