Consider splitting ComponentTicks #4884
Labels
A-ECS
Entities, components, systems, and events
C-Performance
A change motivated by improving speed, memory usage or compile times
S-Needs-Benchmarking
This set of changes needs performance benchmarking to double-check that they help
Comments
james7132
added
A-ECS
|
I'm fully on board with this plan. Before we merge a PR for this, I'd like a benchmark for change detection so we can see the impact. |
alice-i-cecile
added
the
S-Needs-Benchmarking
|
Updated the issue to mention this is blocked on #4883. |
3 tasks
taiyoungjang
pushed a commit
to taiyoungjang/bevy
that referenced
this issue
Dec 15, 2022
# Objective Fixes bevyengine#4884. `ComponentTicks` stores both added and changed ticks contiguously in the same 8 bytes. This is convenient when passing around both together, but causes half the bytes fetched from memory for the purposes of change detection to effectively go unused. This is inefficient when most queries (no filter, mutating *something*) only write out to the changed ticks. ## Solution Split the storage for change detection ticks into two separate `Vec`s inside `Column`. Fetch only what is needed during iteration. This also potentially also removes one blocker from autovectorization of dense queries. EDIT: This is confirmed to enable autovectorization of dense queries in `for_each` and `par_for_each` where possible. Unfortunately `iter` has other blockers that prevent it. ### TODO - [x] Microbenchmark - [x] Check if this allows query iteration to autovectorize simple loops. - [x] Clean up all of the spurious tuples now littered throughout the API ### Open Questions - ~~Is `Mut::is_added` absolutely necessary? Can we not just use `Added` or `ChangeTrackers`?~~ It's optimized out if unused. - ~~Does the fetch of the added ticks get optimized out if not used?~~ Yes it is. --- ## Changelog Added: `Tick`, a wrapper around a single change detection tick. Added: `Column::get_added_ticks` Added: `Column::get_column_ticks` Added: `SparseSet::get_added_ticks` Added: `SparseSet::get_column_ticks` Changed: `Column` now stores added and changed ticks separately internally. Changed: Most APIs returning `&UnsafeCell<ComponentTicks>` now returns `TickCells` instead, which contains two separate `&UnsafeCell<Tick>` for either component ticks. Changed: `Query::for_each(_mut)`, `Query::par_for_each(_mut)` will now leverage autovectorization to speed up query iteration where possible. ## Migration Guide TODO
alradish
pushed a commit
to alradish/bevy
that referenced
this issue
Jan 22, 2023
# Objective Fixes bevyengine#4884. `ComponentTicks` stores both added and changed ticks contiguously in the same 8 bytes. This is convenient when passing around both together, but causes half the bytes fetched from memory for the purposes of change detection to effectively go unused. This is inefficient when most queries (no filter, mutating *something*) only write out to the changed ticks. ## Solution Split the storage for change detection ticks into two separate `Vec`s inside `Column`. Fetch only what is needed during iteration. This also potentially also removes one blocker from autovectorization of dense queries. EDIT: This is confirmed to enable autovectorization of dense queries in `for_each` and `par_for_each` where possible. Unfortunately `iter` has other blockers that prevent it. ### TODO - [x] Microbenchmark - [x] Check if this allows query iteration to autovectorize simple loops. - [x] Clean up all of the spurious tuples now littered throughout the API ### Open Questions - ~~Is `Mut::is_added` absolutely necessary? Can we not just use `Added` or `ChangeTrackers`?~~ It's optimized out if unused. - ~~Does the fetch of the added ticks get optimized out if not used?~~ Yes it is. --- ## Changelog Added: `Tick`, a wrapper around a single change detection tick. Added: `Column::get_added_ticks` Added: `Column::get_column_ticks` Added: `SparseSet::get_added_ticks` Added: `SparseSet::get_column_ticks` Changed: `Column` now stores added and changed ticks separately internally. Changed: Most APIs returning `&UnsafeCell<ComponentTicks>` now returns `TickCells` instead, which contains two separate `&UnsafeCell<Tick>` for either component ticks. Changed: `Query::for_each(_mut)`, `Query::par_for_each(_mut)` will now leverage autovectorization to speed up query iteration where possible. ## Migration Guide TODO
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this issue
Feb 1, 2023
# Objective Fixes bevyengine#4884. `ComponentTicks` stores both added and changed ticks contiguously in the same 8 bytes. This is convenient when passing around both together, but causes half the bytes fetched from memory for the purposes of change detection to effectively go unused. This is inefficient when most queries (no filter, mutating *something*) only write out to the changed ticks. ## Solution Split the storage for change detection ticks into two separate `Vec`s inside `Column`. Fetch only what is needed during iteration. This also potentially also removes one blocker from autovectorization of dense queries. EDIT: This is confirmed to enable autovectorization of dense queries in `for_each` and `par_for_each` where possible. Unfortunately `iter` has other blockers that prevent it. ### TODO - [x] Microbenchmark - [x] Check if this allows query iteration to autovectorize simple loops. - [x] Clean up all of the spurious tuples now littered throughout the API ### Open Questions - ~~Is `Mut::is_added` absolutely necessary? Can we not just use `Added` or `ChangeTrackers`?~~ It's optimized out if unused. - ~~Does the fetch of the added ticks get optimized out if not used?~~ Yes it is. --- ## Changelog Added: `Tick`, a wrapper around a single change detection tick. Added: `Column::get_added_ticks` Added: `Column::get_column_ticks` Added: `SparseSet::get_added_ticks` Added: `SparseSet::get_column_ticks` Changed: `Column` now stores added and changed ticks separately internally. Changed: Most APIs returning `&UnsafeCell<ComponentTicks>` now returns `TickCells` instead, which contains two separate `&UnsafeCell<Tick>` for either component ticks. Changed: `Query::for_each(_mut)`, `Query::par_for_each(_mut)` will now leverage autovectorization to speed up query iteration where possible. ## Migration Guide TODO
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What problem does this solve or what need does it fill?
ComponentTicksis a lopsided datatype. The added ticks are immutable over the lifetime of the component and the changed ticks may be updated every frame. When using them inChanged<T>orAdded<T>filters, half the memory fetched goes unused. When writing to it viaMut, cache lines are written out twice as often as they could.This will likely be blocked on #4883 being resolved first before making any changes here.
What solution would you like?
Split
Vec<UnsafeCell<ComponentTicks>>into twoVec<UnsafeCell<u32>>for added and changed ticks. Update related structs (i.e.Added<T>,Changed<T>,ChangeTrackers<T>,Mut<T>) to fetch only the ones needed.Ideally, this should also enable autovectorization in query code for these filters, which should further speed up change detection filters.
What alternative(s) have you considered?
Leaving change detection as is.
The text was updated successfully, but these errors were encountered: