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

Workload simulator: update timestamp query API usage #484

Merged
merged 2 commits into from
Jan 3, 2025

Conversation

jdarpinian
Copy link
Contributor

The API for timestamp queries has changed since the workload simulator was written. In addition to fixing the timestamps, this also fixes a red error message when loading the workload simulator.

Copy link
Collaborator

@greggman greggman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far I understand you can not compare timestamps across render passes. 2 render passes can run in parallel. Of course dependencies might prevent that but there is no guarantee. Using them as substitutes for timestampWrite won't work.

@jdarpinian
Copy link
Contributor Author

Is there a recommended way to measure a whole frame?

@greggman
Copy link
Collaborator

greggman commented Jan 3, 2025

Unfortunately, not AFAICT. I ran into a similar issue here.

gpuweb/gpuweb#4995

And it was basically confirmed that timestamp results are implementation defined. Maybe something like what you have is fine? Though maybe some comments that it might not be reliable?

Reading over the code it's not entirely clear to me what the code expects timestamp queries to return. IIUC:

  1. make and queue.submit command buffer with an empty compute pass to write a beginning time stamp
  2. queue.writeBuffer to a uniform buffer
  3. optionally queue.writeBuffer to a vertex buffer
  4. optionally make and queue.submit a command buffer to copy a staging buffer
  5. make and queue.submit a command buffer to render with an end time stamp.

I don't think timestamp-queries guarantee to time all of those steps 1 to 5. Step 1 asks the GPU to write a timestamp. That happens asynchronously. Steps 2 and 3 asks the CPU to copy data into a buffer which is unrelated to when the GPU writes the beginning timestamp, since those 2 things, updating a buffer and writing the stamp, are happening separately, one on the GPU, one on the CPU. Step 4 should block step 5 given the buffer being updated is used by step 5.

I'm not really sure what to suggest. You can potentially use timestamps to check if one pass technique is faster than another. Tho, see issue linked above for limitations. But, I don't think you can use them to time a frame and all the stuff that happens in a frame.

Maybe others have ideas. @toji ?

Unrelated, 3 command buffers is slower than 1. (not sure how much slower though. Maybe not enough to measure with only 3?) Would it be good to try to switch to 1 command buffer?

One more thing, end - begin can be negative. You might want to check for that?

The timestamp-query feature as currently specced cannot reliably measure anything other than the length of a single render or compute pass.
@jdarpinian
Copy link
Contributor Author

In the case that end - begin is negative there's nothing useful for us to do. We can't assume wraparound at a defined value so the reported values are simply garbage.

Putting everything in one command buffer would be better in theory but I doubt it would have a measurable impact with such a small number of command buffers. And it would be very difficult to measure any such impact given that timestamp queries seemingly can't help with measuring anything more than single passes.

It seems like timestamp queries as specced are not useful to measure things I want to measure. It works in practice on Win11 but not macOS (honestly I think the macOS behavior is buggy even given the very limited guarantees of the spec). I'll remove timestamp query support as it's kinda half baked anyway, I never implemented it for WebGL for comparison. Hopefully there will be future timestamp features that have a stronger spec.

@greggman greggman merged commit 0980f2f into webgpu:main Jan 3, 2025
1 check passed
@greggman
Copy link
Collaborator

greggman commented Jan 3, 2025

Just fyi, dragging around the data upload sliders I got this error

workloadSimulator/?animate&usePostMessage&canvasOptions:566 Uncaught (in promise) RangeError: offset is out of bounds
at Float32Array.set ()
at render (workloadSimulator/?animate&usePostMessage&canvasOptions:566:19)
at workloadSimulator/?animate&usePostMessage&canvasOptions:471:49

@jdarpinian
Copy link
Contributor Author

Good catch, thanks!

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