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

WebGPURenderer: Introduce TimestampQueryPool #30359

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

RenaudRohlinger
Copy link
Collaborator

@RenaudRohlinger RenaudRohlinger commented Jan 18, 2025

Description
Refactor of timestamp and timer query handling in WebGPU/WebGL for the WebGPURenderer.

It is now more stable and performant through the use of TimestampQueryPool, which maintains a single querySet per type (render/compute) and global buffers. This resolves multiple errors and limitations that users have previously reported.

The API now supports multiple render passes (postprocessing) through a new resolveAllTimestampsAsync method in the Renderer, which resolves all timestamps appended in the loop from different render/compute calls.

Another significant improvement addresses a previous limitation by separating compute and render contexts. Compute operations can now be monitored independently of render calls, as they are often isolated for tasks such as simulations and other complex calculations (with the exception of material.geometryNode).

Example of the new API:

// Render to collision position render target
renderer.setRenderTarget( collisionPosRT );
renderer.render( scene, collisionCamera ); // doesn't require to use renderAsync anymore to calculate timestamp, everything gets calculated in resolveAllTimestampsAsync

// Independent compute pass
renderer.computeAsync( computeParticles );
renderer.resolveAllTimestampsAsync( 'compute' ); // Resolve compute-specific timing data

// Final render with post-processing for example
renderer.setRenderTarget( null );
await postProcessing.renderAsync();
renderer.resolveAllTimestampsAsync(); // Resolve all render pass timings

// Performance monitoring or do something with renderer.info
stats.update();

While testing the tool I took the opportunity to improve one of the main example that seems to be affected by excessive buffer access within the loop, resolved by using const and vars:
Before (in WebGPU velocity affects performances 8/9ms per compute):
Screenshot 2025-01-19 at 15 34 29
After quick optimization tests (velocity compute program speed up by ~50%):
Screenshot 2025-01-19 at 15 51 25

If that's ok this refactor will introduce breaking changes. Rather than attempting to maintain backwards compatibility, which would significantly pollute the codebase, I will document the necessary updates in the migration guide for affected components.

This contribution is funded by Utsubo

Copy link

github-actions bot commented Jan 18, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 336.25
78.28
336.25
78.28
+0 B
+0 B
WebGPU 503.66
139.88
506.64
140.73
+2.98 kB
+846 B
WebGPU Nodes 503.13
139.77
506.11
140.63
+2.98 kB
+860 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 465.25
112.13
465.25
112.13
+0 B
+0 B
WebGPU 577.44
156.5
580.43
157.34
+2.99 kB
+839 B
WebGPU Nodes 532.82
146.06
535.82
146.94
+2.99 kB
+873 B

@RenaudRohlinger RenaudRohlinger changed the title WebGPURenderer: Introduce renderer.resolveAllTimestampsAsync(type) WebGPURenderer: Introduce TimestampQueryPool Jan 18, 2025
@sunag sunag added this to the r173 milestone Jan 18, 2025
@RenaudRohlinger RenaudRohlinger marked this pull request as ready for review January 19, 2025 07:04
@@ -0,0 +1,29 @@
class TimestampQueryPool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mind adding the remaining documentation for the the ctor, properties the dispose() method and the class itself? It would be also good to document the derived classes (at least the bits which are added to the base implementation)

*/
async resolveTimestampAsync( /*renderContext, type*/ ) { }
async resolveAllTimestampsAsync( type = 'render' ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you we need the *All* term in the name? How about resolveTimestampsAsync()?


if ( renderContextData.activeQuery !== null ) {

this.gl.beginQuery( this.disjoint.TIME_ELAPSED_EXT, renderContextData.activeQuery );
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's nice to see the timestamp related code is moved into specific modules. That makes the backends easier to maintain.

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