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

WebGPU "Queries" implementation (for profiling purposes) #721

Closed
z2oh opened this issue Jun 14, 2020 · 13 comments · Fixed by #1128
Closed

WebGPU "Queries" implementation (for profiling purposes) #721

z2oh opened this issue Jun 14, 2020 · 13 comments · Fixed by #1128
Assignees
Labels
area: api Issues related to API surface type: enhancement New feature or request

Comments

@z2oh
Copy link

z2oh commented Jun 14, 2020

I am strictly a hobbyist when it comes to graphics programming, so please forgive any misinformed assumptions that I make :)

I am trying to benchmark individual render passes in an application built on wgpu-rs. In my research to resolve this, I stumbled upon the "timestamp" GPUQueryType. This API seems like it exposes data which could be used to benchmark any particular command, render passes among them (a very useful feature indeed!).

However, I could not find any APIs for interfacing with these Queries in wgpu-rs. I followed the trail and it seems that gfx-hal does provide this interface, which (in theory) means it can be propagated to my application code with the only necessary changes being contained within wgpu-rs and wgpu-core. I have two theories as to why this feature is not already implemented:

  1. Software projects must allocate their limited resources judiciously; profiling isn't very useful if there is nothing to profile!
  2. This open issue on gfx-hal precludes it from being spec-compliant with WebGPU, namely this restriction:

Timestamp query requires timestamp-query is available on the device.

I am willing to put in some legwork to make this happen, but I want to make sure my understanding of the situation is correct before I invest too much time. Any guidance would also be appreciated. Thanks for working on wgpu-rs!

@z2oh z2oh added the type: enhancement New feature or request label Jun 14, 2020
@cwfitzgerald
Copy link
Member

I believe your theory 1 is the most accurate :) Queries just recently got added to the upstream spec, so no one has gotten around to implementing them yet. (@kvark please correct me if any of this is wrong)

I don't know of anything at the moment that would predicate that they are unable to be implemented.

Hop on our matrix (https://matrix.to/#/#wgpu:matrix.org) and lets chat!

@kvark
Copy link
Member

kvark commented Jun 15, 2020

Thank you for the initiative, @z2oh ! I'm looking forward to use the GPU time queries in https://github.com/kvark/wgpu-bench/ .

@z2oh
Copy link
Author

z2oh commented Jun 17, 2020

progress

I have added the necessary interfacing to query for timestamp information in the hello-compute example. There is still some more implementation work to do, as well as a healthy amount of cleanup, but the steel thread implementation is there :)

Unfortunately, applications driven by an event loop will still need to manually poll the future that maps the buffer with timestamp information into host memory (and I couldn't find an easy way to do this). I'm also not sure how this will work with the buffer being overwritten every frame, but these are problems that can be solved after the initial implementation.

@kvark
Copy link
Member

kvark commented Jun 17, 2020

Great progress here!
The questions you are asking about async polling and overwriting the buffer are not specific to queries, they equally apply to buffer mapping and buffer copies, so it's nothing that you should be worried about in scope of the PR.

Minor nit: let's keep hello-compute as simple as possible. Could you pick a different example to add the queries in it?

@z2oh
Copy link
Author

z2oh commented Jun 18, 2020

The questions you are asking about async polling and overwriting the buffer are not specific to queries, they equally apply to buffer mapping and buffer copies, so it's nothing that you should be worried about in scope of the PR.

Yep yep, I was just making a note for posterity that while the upcoming PR will add the ability to query for timestamps, it is not very useful without additional async integrations.

Minor nit: let's keep hello-compute as simple as possible. Could you pick a different example to add the queries in it?

I was planning on adding an explicit query example; I just put it in hello-compute locally to for my own testing :)

@kvark
Copy link
Member

kvark commented Jun 18, 2020

@z2oh please don't rush into making a new example. If we end up having an example per feature, there is going to be too much redundancy between them, and more code to maintain. Instead, pick an existing example for which it would make sense.

We'll need to make a table showing which features are present in which examples.

@rukai
Copy link
Contributor

rukai commented Jun 18, 2020

"We'll need to make a table showing which features are present in which examples."
Oooh that's a great idea

@cwfitzgerald
Copy link
Member

Any progress on this? This would be a very useful feature to have.

@z2oh
Copy link
Author

z2oh commented Jul 15, 2020

@cwfitzgerald,
I have made some additional progress since my last update, but I had shelved this PR temporarily after being distracted by some other things. Your comment has re-motivated me though :)

Pipeline statistics queries are wired through, but every time I read the buffer it's all 0s so I clearly have done something wrong! I'll rebase and continue the PR this week, and hopefully I can submit a draft PR this weekend. I may ping you on matrix if I can't figure out this last remaining issue. Cheers!

@cwfitzgerald
Copy link
Member

Glad to hear it! Please do!

@cwfitzgerald
Copy link
Member

hey! I know it's been.... over two months, but if you still are interested in helping with this, I can help out. If you don't have enough time or don't want to, no worries, I can continue where you left off. :)

@cwfitzgerald
Copy link
Member

Bump :)

@cwfitzgerald cwfitzgerald added the area: api Issues related to API surface label Dec 1, 2020
@z2oh
Copy link
Author

z2oh commented Dec 16, 2020

Sorry for the (severely) delayed response! It's been a busy time. Unfortunately, my efforts are being expended elsewhere and I am unlikely to make any more progress on this issue.

I will share my WIP commits, but they may be too far behind master to be of much use. In these commits, timestamp queries are working, but I was never able to locate the bug that was preventing pipeline statistics queries from functioning correctly.

z2oh@dade740
z2oh/wgpu-rs@8c53da9

I hope you will have more success than I had in getting pipeline statistics queries working. Thanks for your work on wgpu! I can't wait to come back to this library the next time I need to do GPU programming in Rust.

@kvark kvark pinned this issue Dec 17, 2020
@cwfitzgerald cwfitzgerald self-assigned this Jan 3, 2021
@bors bors bot closed this as completed in 9dce0af Jan 16, 2021
@kvark kvark unpinned this issue Jan 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api Issues related to API surface type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants