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

Add pipeline statistics #9135

Merged
merged 26 commits into from
Mar 17, 2024
Merged

Add pipeline statistics #9135

merged 26 commits into from
Mar 17, 2024

Conversation

LeshaInc
Copy link
Contributor

@LeshaInc LeshaInc commented Jul 12, 2023

Objective

It's useful to have access to render pipeline statistics, since they provide more information than FPS alone. For example, the number of drawn triangles can be used to debug culling and LODs. The number of fragment shader invocations can provide a more stable alternative metric than GPU elapsed time.

See also: Render node GPU timing overlay #8067, which doesn't provide pipeline statistics, but adds a nice overlay.

Solution

Add RenderDiagnosticsPlugin, which enables collecting pipeline statistics and CPU & GPU timings.


Changelog

  • Add RenderDiagnosticsPlugin
  • Add RenderContext::diagnostic_recorder method

@github-actions
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Diagnostics Logging, crash handling, error reporting and performance analysis labels Jul 12, 2023
@alice-i-cecile
Copy link
Member

Disable statistics when the required features aren't available.

Can you say more about this? It sounds like we should block on fixing that before merging this PR to avoid unexpected problems, but I'm not fully sure I understand.

@LeshaInc
Copy link
Contributor Author

Can you say more about this? It sounds like we should block on fixing that before merging this PR to avoid unexpected problems, but I'm not fully sure I understand.

I have addressed that. Now when timestamp queries are unsupported, no GPU timing will be recorded. Same for pipeline queries. CPU timings are always recorded though, since they don't require any features.

Also, I have written the docs.

I have the following question: should I keep the RenderStatistics resource, or perhaps they should be stored as diagnostics instead? I would imagine having diagnostics like render.main_opaque_pass_3d.gpu_time. This would give us smoothing basically for free. But I see a few problems with this approach:

  • How to generate diagnostic UUIDs, given there can be any number of render passes (at least one for each shadow caster, for instance)
  • How to assign diagnostic instants? They can either share one instant at which they became available, or I could try to approximate instants given GPU timestamps and the instant when the frame was submitted.
  • Missing ergonomics for nested diagnostics (a render pass would have, like, 7 separate diagnostics, and there's no way to easily iterate through all of them).

I would say it's simpler to leave render statistics as a resource, and a future PR could address copying those into diagnostics and/or providing a GUI overlay, like in #8067.

@LeshaInc
Copy link
Contributor Author

I've gated all recording functionality behind RenderStatisticsPlugin, which is disabled by default.

Additionally, now I reuse buffers instead of creating them every frame. There are 2 query sets, and 2 buffers (one for resolving the query set, and the other for cpu-readback) per frame-in-flight (of which there are 3, apparently).

This makes performance of statistics recording basically negligible (given there are only a couple render passes). I ran many_sprites benchmark and saw no measurable regressions.

Now supports arbitrarily nested time spans and supports multithreading.
@LeshaInc LeshaInc marked this pull request as draft July 20, 2023 22:01
@alice-i-cecile
Copy link
Member

I think so: it's valuable for discovery, and to ensure that other diagnostics can be built based on the public API.

@JMS55
Copy link
Contributor

JMS55 commented Mar 8, 2024

Btw I'm unfortunately busy with IRL stuff, sorry for the delay on reviewing this. I still want to get this in as soon as I can.

Copy link
Contributor

@JMS55 JMS55 left a comment

Choose a reason for hiding this comment

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

I didn't have time to do a super thorough review, but on first glance this looks very good to me. Will let rendering-SME's review this in detail. Super excited to have this, and start hooking it up to the other rendering features!

The other thing I'd like to see is tracy integration: https://docs.rs/tracy-client/latest/tracy_client/struct.GpuContext.html
EDIT: We talked about this earlier, I forgot. Can be a future PR.

examples/3d/3d_scene.rs Outdated Show resolved Hide resolved
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Looks solid: I'm happy to merge this now and enhance this in future work.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it M-Needs-Release-Note Work that should be called out in the blog due to impact labels Mar 17, 2024
@alice-i-cecile
Copy link
Member

@LeshaInc if you can get CI passing I'll merge this in for you :) I think you may need to adjust some imports.

auto-merge was automatically disabled March 17, 2024 19:51

Head branch was pushed to by a user without write access

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 17, 2024
Merged via the queue into bevyengine:main with commit 737b719 Mar 17, 2024
30 checks passed
@NthTensor NthTensor mentioned this pull request Apr 1, 2024
@alice-i-cecile alice-i-cecile removed the M-Needs-Release-Note Work that should be called out in the blog due to impact label Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Dev-Tools Tools used to debug Bevy applications. A-Diagnostics Logging, crash handling, error reporting and performance analysis A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants