Skip to content

Do query for default components only once per view #8424

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

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Dec 11, 2024

Moves query of default components to the "execute query" stage which we perform for every view at the start of the frame.
Previously, we've done this as part of blueprint resolve on every

Gives me about 2ms cpu frame time in the 2h airtraffic demo on my windows machine running with single thread (since it's easier to profile, pixi run rerun-release --threads 1) with the 3d view maximized and the timline minimized.

Change is expected to have high impact for:

  • many entities
  • no or few transforms (because the transform cost shadows this win too much and adds a lot of noise)
  • few threads (/web) since the cost is in visualizer execution which is parallel

profile snapshots for said scenario:
before
image

after:
image

Related future work:

  • need short circuiting for overrides as well - we can just know when there's none (and that's naturally very common!!)
  • short circuit when there's no (fitting) default components?
  • reduce the amount of query objects being passed around, we have too many of those at this point!

@Wumpf Wumpf added 📉 performance Optimization, memory use, etc include in changelog labels Dec 11, 2024
Copy link

github-actions bot commented Dec 11, 2024

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
1125a7d https://rerun.io/viewer/pr/8424 +nightly +main

Note: This comment is updated whenever you push a commit.

@Wumpf Wumpf added the 📺 re_viewer affects re_viewer itself label Dec 11, 2024
@Wumpf Wumpf force-pushed the andreas/fix-redundant-default-blueprint-queries branch from 8df4b67 to 1125a7d Compare December 11, 2024 19:24
@teh-cmc teh-cmc self-requested a review December 12, 2024 08:29
@teh-cmc teh-cmc merged commit 71afae7 into main Dec 12, 2024
31 checks passed
@teh-cmc teh-cmc deleted the andreas/fix-redundant-default-blueprint-queries branch December 12, 2024 08:39
@@ -208,7 +208,7 @@ impl QueryCache {
///
/// Use [`LatestAtResults::get`] and/or [`LatestAtResults::get_required`] in order to access
/// the results for each individual component.
#[derive(Debug)]
#[derive(Debug, Clone)]
Copy link
Member Author

Choose a reason for hiding this comment

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

ended up not needing this actually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include in changelog 📉 performance Optimization, memory use, etc 📺 re_viewer affects re_viewer itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants