Skip to content

Column change detection for immutable components #18788

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

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

notmd
Copy link
Contributor

@notmd notmd commented Apr 10, 2025

Objective

  • Implement column structure change detection for immutable component. This will allow to filter at table/archetype level for Query<(), Changed<SomeImmutableComponent>>

Solution

  • Adopt change from column/table level change detection #11120 but for column of immutable component only. This change doesn't need to store an atomic because immutable components can only be changed by inserting so performance overhead is minimal.
  • Add change_tick field to Column/ThinColumn. Change tick will be updated when inserting, replacing or removing elements. Updating when removing seems unnecessary because change detection doesn't detect removal. I am still updating it because I want to use this somehow to help implement query.is_changed(). We may consider introducing a separate tick for removal in the future.. Change tick will be update when inserting or replacing elements.

Testing

  • I have added a few new tests to test the archetype_filter_fetch
  • I also update the change detection benchmark for immutable component
  • Here the benchmark result for Query<Entity, Changed<T>>. You can reproduce this by running cargo bench -p benches --bench ecs -- --save-baseline current none_changed_detection and critcmp benches current -g '^(none_changed_detection/\d+\w+::\w+::[A-Z][a-z]+)'
    image
  • Unfortunately, I can't benchmark the overall ecs overhead on my laptop, too much noise. But I believe the overhead is very small. Really appreciate someone with good computer can verify my words.

Future work

  • Stop allocating changed_ticks for immutable component. Because it can't be mutated, use added_ticks for is enough.
  • Expand this to mutable components without hurting performance.

Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-18788

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events M-Needs-Release-Note Work that should be called out in the blog due to impact S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Apr 10, 2025
Copy link
Contributor

It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note.

Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes.

@notmd notmd marked this pull request as ready for review April 11, 2025 15:51
@notmd
Copy link
Contributor Author

notmd commented Apr 11, 2025

I have no idea what miri is complaining. Help is appreciated.

@notmd
Copy link
Contributor Author

notmd commented Apr 11, 2025

Benchmark for update_ui_context_system, yellow is main, red is current branch in many_buttons. This is the only system I can find in Bevy that benefit from the changes. Maybe we can push for more immutable components in futures
cargo run --example many_buttons --release --features bevy/trace_tracy
image

@notmd notmd changed the title Column structure change detection Column change detection for immutable components Apr 13, 2025
@hymm hymm self-requested a review May 1, 2025 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible M-Needs-Release-Note Work that should be called out in the blog due to impact S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants