Add a public API to expose allocation reports #226
Conversation
There was a problem hiding this comment.
Nice stuff, as discussed in the wgpu Matrix! This would definitely help us long-term with generalizing the visualizer completely, after adding more flags like the relevant heap and other backend-specific items in a somewhat generic way 😅
Would love this to have some doc-comments from the start though, given that -for example- allocations are stored globally instead of per-block and findable via a stored Range. Maybe derive(Debug) would help as well, but that might be complicated for the top-level AllocatorReport.
| total_reserved_bytes += block.size; | ||
| let first_allocation = allocations.len(); | ||
| allocations.extend(block.sub_allocator.report_allocations()); | ||
| blocks.push(MemoryBlockReport { |
There was a problem hiding this comment.
Is it not useful to you yet to report what memory type/heap this block was allocated on?
There was a problem hiding this comment.
Absolutely! I was being cautious about exposing internals but I'll take every bit of useful information you are confortable with exposing.
|
I made a few notable changes:
|
|
I added metal and made a few other adjustments from the review comments. I still haven't added information about the memory blocks to the report. To do that I need to spend a bit of time looking at each backend, finding the common denominators and storing the right info in the right places for the report to pick up. It's not much but because I am currently juggling a lot of tasks on different projects I'm having some difficulty finding the time to wrap my head around that bit. Would you be OK with landing the current feature set? |
I'm sure We currently have a CI failure related to the whole |
I don't think there's a specific rule about that but since wgpu's release process is fair bit more involved than most crates, it indeed tends to be done in isolation.
I wasn't aware of this. I'm not involved with the metal stuff and reading through #224 and linked issues it looks like the interesting stuff might be happening upstream of the metal-rs crate but let me know if I can help by following up with other folks in the gfx-rs org. |
So is ours, and I think for every crate it's a much nicer policy to have an explicit "Release XXX" commit in their history rather than doing it within a feature PR. |
|
@nical would you mind to sign your commits? |
|
@nical JFYI, despite being subscribed to this repo and PR I don't receive any notifications for your pushes, and just happened to see the update at the right moment by accident. Please ping me once I have to approve the CI again, since GitHub delegates their anti-CI-abuse measures to repository owners and maintainers 😅 |
|
Good to know, @MarijnS95. I just pushed another commit to fix the fmt issue |
This PR adds a public API that exposes enough information for users to build something like the block visualizer without the egui integration. It also adds an offset field in the allocation report.
I would like to integrate (and expose in some form) this in wgpu to debug memory leaks.
The report could expose more information (for example properties of the memory and stack traces) and I haven't added any documentation yet, but it's a good basis for discussing whether the project is OK with exposing something like this and whether the approach is good.