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

Show memory report in debug builds #187

Merged
merged 3 commits into from
May 8, 2022

Conversation

sqaxomonophonen
Copy link
Contributor

log::debug!() after wgpuSwapChainPresent() which is assumed to be called roughly once per frame. This is useful for detecting memory leaks.

See also: #186

log::debug!() after wgpuSwapChainPresent(); assumed to be called roughly once
per frame; useful for detecting memory leaks
@kvark
Copy link
Member

kvark commented Apr 24, 2022

I suggested doing it in debug only, and now I'm having second thoughts. Sorry about this!
This memory dump can be quite verbose if done every frame.
Perhaps, we should just expose it as a C function and let the users call it when needed?

@sqaxomonophonen
Copy link
Contributor Author

It could be argued that the WebGPU specification itself is lacking a way to get a resource usage report (for the same reason it has .destroy() methods; it implies the possibility of resource management problems even in pure JS+WebGPU applications). So any solution we come with is likely going to be temporary? I'm also fine with adding something like this to wgpu.h:

void wgpuLogResourceUsage(); // calls your WGPULogCallback with WGPULogLevel=0 regardless of log level
// *OR*:
char* wgpuGetResourceUsageString(); // caller "owns" the returned C-string and must free() it

Compared to the implicit logging after wgpuSwapChainPresent() it has a higher risk of breaking forward compatibility, but it's easy to fix (compared with all the other forward compatibility issues). It doesn't add unavoidable verbosity, and it also works better with multi-window programs, or programs that don't want a report every frame.

Anything more complicated than this (like getting the report in a C struct), and I think the effort will we wasted here and better spent discussing it with the WebGPU people. Thankfully, running a custom build of wgpu-native is easy (:+1:), and I'm fine with it for the time being, so feel free to .destroy() this PR if neither solution sits well with you 😃

@kvark
Copy link
Member

kvark commented Apr 28, 2022

wgpuGetResourceUsageString sounds good to land here

@sqaxomonophonen
Copy link
Contributor Author

Does this look good?

I tested with the triangle example, and this works as expected:

diff --git a/examples/triangle/main.c b/examples/triangle/main.c
index 0de6675..4c2a8b0 100644
--- a/examples/triangle/main.c
+++ b/examples/triangle/main.c
@@ -342,6 +342,10 @@ int main() {
     wgpuSwapChainPresent(swapChain);
 
     glfwPollEvents();
+
+    char* usage = wgpuGetResourceUsageString();
+    printf("ru: %s\n", usage);
+    free(usage);
   }
 
   glfwDestroyWindow(window);

@kvark kvark merged commit 3a6d1d9 into gfx-rs:master May 8, 2022
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.

2 participants