Skip to content

Commit 55ca7fc

Browse files
committed
Split Component Ticks (#6547)
# Objective Fixes #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
1 parent 210979f commit 55ca7fc

File tree

11 files changed

+385
-257
lines changed

11 files changed

+385
-257
lines changed

crates/bevy_ecs/src/bundle.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{
99
Archetype, ArchetypeId, Archetypes, BundleComponentStatus, ComponentStatus,
1010
SpawnBundleStatus,
1111
},
12-
component::{Component, ComponentId, ComponentTicks, Components, StorageType},
12+
component::{Component, ComponentId, Components, StorageType, Tick},
1313
entity::{Entities, Entity, EntityLocation},
1414
storage::{SparseSetIndex, SparseSets, Storages, Table},
1515
};
@@ -394,11 +394,7 @@ impl BundleInfo {
394394
// SAFETY: bundle_component is a valid index for this bundle
395395
match bundle_component_status.get_status(bundle_component) {
396396
ComponentStatus::Added => {
397-
column.initialize(
398-
table_row,
399-
component_ptr,
400-
ComponentTicks::new(change_tick),
401-
);
397+
column.initialize(table_row, component_ptr, Tick::new(change_tick));
402398
}
403399
ComponentStatus::Mutated => {
404400
column.replace(table_row, component_ptr, change_tick);

crates/bevy_ecs/src/change_detection.rs

Lines changed: 59 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
//! Types that detect when their internal data mutate.
22
3-
use crate::{component::ComponentTicks, ptr::PtrMut, system::Resource};
3+
use crate::{
4+
component::{Tick, TickCells},
5+
ptr::PtrMut,
6+
system::Resource,
7+
};
8+
use bevy_ptr::UnsafeCellDeref;
49
use std::ops::{Deref, DerefMut};
510

611
/// The (arbitrarily chosen) minimum number of world tick increments between `check_tick` scans.
@@ -95,21 +100,21 @@ macro_rules! change_detection_impl {
95100
#[inline]
96101
fn is_added(&self) -> bool {
97102
self.ticks
98-
.component_ticks
99-
.is_added(self.ticks.last_change_tick, self.ticks.change_tick)
103+
.added
104+
.is_older_than(self.ticks.last_change_tick, self.ticks.change_tick)
100105
}
101106

102107
#[inline]
103108
fn is_changed(&self) -> bool {
104109
self.ticks
105-
.component_ticks
106-
.is_changed(self.ticks.last_change_tick, self.ticks.change_tick)
110+
.changed
111+
.is_older_than(self.ticks.last_change_tick, self.ticks.change_tick)
107112
}
108113

109114
#[inline]
110115
fn set_changed(&mut self) {
111116
self.ticks
112-
.component_ticks
117+
.changed
113118
.set_changed(self.ticks.change_tick);
114119
}
115120

@@ -224,11 +229,30 @@ macro_rules! impl_debug {
224229
}
225230

226231
pub(crate) struct Ticks<'a> {
227-
pub(crate) component_ticks: &'a mut ComponentTicks,
232+
pub(crate) added: &'a mut Tick,
233+
pub(crate) changed: &'a mut Tick,
228234
pub(crate) last_change_tick: u32,
229235
pub(crate) change_tick: u32,
230236
}
231237

238+
impl<'a> Ticks<'a> {
239+
/// # Safety
240+
/// This should never alias the underlying ticks. All access must be unique.
241+
#[inline]
242+
pub(crate) unsafe fn from_tick_cells(
243+
cells: TickCells<'a>,
244+
last_change_tick: u32,
245+
change_tick: u32,
246+
) -> Self {
247+
Self {
248+
added: cells.added.deref_mut(),
249+
changed: cells.changed.deref_mut(),
250+
last_change_tick,
251+
change_tick,
252+
}
253+
}
254+
}
255+
232256
/// Unique mutable borrow of a [`Resource`].
233257
///
234258
/// See the [`Resource`] documentation for usage.
@@ -381,22 +405,20 @@ impl<'a> DetectChanges for MutUntyped<'a> {
381405
#[inline]
382406
fn is_added(&self) -> bool {
383407
self.ticks
384-
.component_ticks
385-
.is_added(self.ticks.last_change_tick, self.ticks.change_tick)
408+
.added
409+
.is_older_than(self.ticks.last_change_tick, self.ticks.change_tick)
386410
}
387411

388412
#[inline]
389413
fn is_changed(&self) -> bool {
390414
self.ticks
391-
.component_ticks
392-
.is_changed(self.ticks.last_change_tick, self.ticks.change_tick)
415+
.changed
416+
.is_older_than(self.ticks.last_change_tick, self.ticks.change_tick)
393417
}
394418

395419
#[inline]
396420
fn set_changed(&mut self) {
397-
self.ticks
398-
.component_ticks
399-
.set_changed(self.ticks.change_tick);
421+
self.ticks.changed.set_changed(self.ticks.change_tick);
400422
}
401423

402424
#[inline]
@@ -429,10 +451,8 @@ mod tests {
429451

430452
use crate::{
431453
self as bevy_ecs,
432-
change_detection::{
433-
ComponentTicks, Mut, NonSendMut, ResMut, Ticks, CHECK_TICK_THRESHOLD, MAX_CHANGE_AGE,
434-
},
435-
component::Component,
454+
change_detection::{Mut, NonSendMut, ResMut, Ticks, CHECK_TICK_THRESHOLD, MAX_CHANGE_AGE},
455+
component::{Component, ComponentTicks, Tick},
436456
query::ChangeTrackers,
437457
system::{IntoSystem, Query, System},
438458
world::World,
@@ -514,8 +534,8 @@ mod tests {
514534

515535
let mut query = world.query::<ChangeTrackers<C>>();
516536
for tracker in query.iter(&world) {
517-
let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added);
518-
let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed);
537+
let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added.tick);
538+
let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed.tick);
519539
assert!(ticks_since_insert > MAX_CHANGE_AGE);
520540
assert!(ticks_since_change > MAX_CHANGE_AGE);
521541
}
@@ -524,8 +544,8 @@ mod tests {
524544
world.check_change_ticks();
525545

526546
for tracker in query.iter(&world) {
527-
let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added);
528-
let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed);
547+
let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added.tick);
548+
let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed.tick);
529549
assert!(ticks_since_insert == MAX_CHANGE_AGE);
530550
assert!(ticks_since_change == MAX_CHANGE_AGE);
531551
}
@@ -534,11 +554,12 @@ mod tests {
534554
#[test]
535555
fn mut_from_res_mut() {
536556
let mut component_ticks = ComponentTicks {
537-
added: 1,
538-
changed: 2,
557+
added: Tick::new(1),
558+
changed: Tick::new(2),
539559
};
540560
let ticks = Ticks {
541-
component_ticks: &mut component_ticks,
561+
added: &mut component_ticks.added,
562+
changed: &mut component_ticks.changed,
542563
last_change_tick: 3,
543564
change_tick: 4,
544565
};
@@ -549,20 +570,21 @@ mod tests {
549570
};
550571

551572
let into_mut: Mut<R> = res_mut.into();
552-
assert_eq!(1, into_mut.ticks.component_ticks.added);
553-
assert_eq!(2, into_mut.ticks.component_ticks.changed);
573+
assert_eq!(1, into_mut.ticks.added.tick);
574+
assert_eq!(2, into_mut.ticks.changed.tick);
554575
assert_eq!(3, into_mut.ticks.last_change_tick);
555576
assert_eq!(4, into_mut.ticks.change_tick);
556577
}
557578

558579
#[test]
559580
fn mut_from_non_send_mut() {
560581
let mut component_ticks = ComponentTicks {
561-
added: 1,
562-
changed: 2,
582+
added: Tick::new(1),
583+
changed: Tick::new(2),
563584
};
564585
let ticks = Ticks {
565-
component_ticks: &mut component_ticks,
586+
added: &mut component_ticks.added,
587+
changed: &mut component_ticks.changed,
566588
last_change_tick: 3,
567589
change_tick: 4,
568590
};
@@ -573,8 +595,8 @@ mod tests {
573595
};
574596

575597
let into_mut: Mut<R> = non_send_mut.into();
576-
assert_eq!(1, into_mut.ticks.component_ticks.added);
577-
assert_eq!(2, into_mut.ticks.component_ticks.changed);
598+
assert_eq!(1, into_mut.ticks.added.tick);
599+
assert_eq!(2, into_mut.ticks.changed.tick);
578600
assert_eq!(3, into_mut.ticks.last_change_tick);
579601
assert_eq!(4, into_mut.ticks.change_tick);
580602
}
@@ -584,13 +606,14 @@ mod tests {
584606
use super::*;
585607
struct Outer(i64);
586608

609+
let (last_change_tick, change_tick) = (2, 3);
587610
let mut component_ticks = ComponentTicks {
588-
added: 1,
589-
changed: 2,
611+
added: Tick::new(1),
612+
changed: Tick::new(2),
590613
};
591-
let (last_change_tick, change_tick) = (2, 3);
592614
let ticks = Ticks {
593-
component_ticks: &mut component_ticks,
615+
added: &mut component_ticks.added,
616+
changed: &mut component_ticks.changed,
594617
last_change_tick,
595618
change_tick,
596619
};

crates/bevy_ecs/src/component.rs

Lines changed: 80 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ use crate::{
66
system::Resource,
77
};
88
pub use bevy_ecs_macros::Component;
9-
use bevy_ptr::OwningPtr;
9+
use bevy_ptr::{OwningPtr, UnsafeCellDeref};
10+
use std::cell::UnsafeCell;
1011
use std::{
1112
alloc::Layout,
1213
any::{Any, TypeId},
@@ -517,58 +518,108 @@ impl Components {
517518
}
518519
}
519520

520-
/// Records when a component was added and when it was last mutably dereferenced (or added).
521+
/// Used to track changes in state between system runs, e.g. components being added or accessed mutably.
521522
#[derive(Copy, Clone, Debug)]
522-
pub struct ComponentTicks {
523-
pub(crate) added: u32,
524-
pub(crate) changed: u32,
523+
pub struct Tick {
524+
pub(crate) tick: u32,
525525
}
526526

527-
impl ComponentTicks {
527+
impl Tick {
528+
pub const fn new(tick: u32) -> Self {
529+
Self { tick }
530+
}
531+
528532
#[inline]
529-
/// Returns `true` if the component was added after the system last ran.
530-
pub fn is_added(&self, last_change_tick: u32, change_tick: u32) -> bool {
533+
/// Returns `true` if the tick is older than the system last's run.
534+
pub fn is_older_than(&self, last_change_tick: u32, change_tick: u32) -> bool {
531535
// This works even with wraparound because the world tick (`change_tick`) is always "newer" than
532-
// `last_change_tick` and `self.added`, and we scan periodically to clamp `ComponentTicks` values
536+
// `last_change_tick` and `self.tick`, and we scan periodically to clamp `ComponentTicks` values
533537
// so they never get older than `u32::MAX` (the difference would overflow).
534538
//
535539
// The clamp here ensures determinism (since scans could differ between app runs).
536-
let ticks_since_insert = change_tick.wrapping_sub(self.added).min(MAX_CHANGE_AGE);
540+
let ticks_since_insert = change_tick.wrapping_sub(self.tick).min(MAX_CHANGE_AGE);
537541
let ticks_since_system = change_tick
538542
.wrapping_sub(last_change_tick)
539543
.min(MAX_CHANGE_AGE);
540544

541545
ticks_since_system > ticks_since_insert
542546
}
543547

548+
pub(crate) fn check_tick(&mut self, change_tick: u32) {
549+
let age = change_tick.wrapping_sub(self.tick);
550+
// This comparison assumes that `age` has not overflowed `u32::MAX` before, which will be true
551+
// so long as this check always runs before that can happen.
552+
if age > MAX_CHANGE_AGE {
553+
self.tick = change_tick.wrapping_sub(MAX_CHANGE_AGE);
554+
}
555+
}
556+
557+
/// Manually sets the change tick.
558+
///
559+
/// This is normally done automatically via the [`DerefMut`](std::ops::DerefMut) implementation
560+
/// on [`Mut<T>`](crate::change_detection::Mut), [`ResMut<T>`](crate::change_detection::ResMut), etc.
561+
/// However, components and resources that make use of interior mutability might require manual updates.
562+
///
563+
/// # Example
564+
/// ```rust,no_run
565+
/// # use bevy_ecs::{world::World, component::ComponentTicks};
566+
/// let world: World = unimplemented!();
567+
/// let component_ticks: ComponentTicks = unimplemented!();
568+
///
569+
/// component_ticks.set_changed(world.read_change_tick());
570+
/// ```
571+
#[inline]
572+
pub fn set_changed(&mut self, change_tick: u32) {
573+
self.tick = change_tick;
574+
}
575+
}
576+
577+
/// Wrapper around [`Tick`]s for a single component
578+
#[derive(Copy, Clone, Debug)]
579+
pub struct TickCells<'a> {
580+
pub added: &'a UnsafeCell<Tick>,
581+
pub changed: &'a UnsafeCell<Tick>,
582+
}
583+
584+
impl<'a> TickCells<'a> {
585+
/// # Safety
586+
/// All cells contained within must uphold the safety invariants of [`UnsafeCellDeref::read`].
587+
#[inline]
588+
pub(crate) unsafe fn read(&self) -> ComponentTicks {
589+
ComponentTicks {
590+
added: self.added.read(),
591+
changed: self.changed.read(),
592+
}
593+
}
594+
}
595+
596+
/// Records when a component was added and when it was last mutably dereferenced (or added).
597+
#[derive(Copy, Clone, Debug)]
598+
pub struct ComponentTicks {
599+
pub(crate) added: Tick,
600+
pub(crate) changed: Tick,
601+
}
602+
603+
impl ComponentTicks {
604+
#[inline]
605+
/// Returns `true` if the component was added after the system last ran.
606+
pub fn is_added(&self, last_change_tick: u32, change_tick: u32) -> bool {
607+
self.added.is_older_than(last_change_tick, change_tick)
608+
}
609+
544610
#[inline]
545611
/// Returns `true` if the component was added or mutably dereferenced after the system last ran.
546612
pub fn is_changed(&self, last_change_tick: u32, change_tick: u32) -> bool {
547-
// This works even with wraparound because the world tick (`change_tick`) is always "newer" than
548-
// `last_change_tick` and `self.changed`, and we scan periodically to clamp `ComponentTicks` values
549-
// so they never get older than `u32::MAX` (the difference would overflow).
550-
//
551-
// The clamp here ensures determinism (since scans could differ between app runs).
552-
let ticks_since_change = change_tick.wrapping_sub(self.changed).min(MAX_CHANGE_AGE);
553-
let ticks_since_system = change_tick
554-
.wrapping_sub(last_change_tick)
555-
.min(MAX_CHANGE_AGE);
556-
557-
ticks_since_system > ticks_since_change
613+
self.changed.is_older_than(last_change_tick, change_tick)
558614
}
559615

560616
pub(crate) fn new(change_tick: u32) -> Self {
561617
Self {
562-
added: change_tick,
563-
changed: change_tick,
618+
added: Tick::new(change_tick),
619+
changed: Tick::new(change_tick),
564620
}
565621
}
566622

567-
pub(crate) fn check_ticks(&mut self, change_tick: u32) {
568-
check_tick(&mut self.added, change_tick);
569-
check_tick(&mut self.changed, change_tick);
570-
}
571-
572623
/// Manually sets the change tick.
573624
///
574625
/// This is normally done automatically via the [`DerefMut`](std::ops::DerefMut) implementation
@@ -585,15 +636,6 @@ impl ComponentTicks {
585636
/// ```
586637
#[inline]
587638
pub fn set_changed(&mut self, change_tick: u32) {
588-
self.changed = change_tick;
589-
}
590-
}
591-
592-
fn check_tick(last_change_tick: &mut u32, change_tick: u32) {
593-
let age = change_tick.wrapping_sub(*last_change_tick);
594-
// This comparison assumes that `age` has not overflowed `u32::MAX` before, which will be true
595-
// so long as this check always runs before that can happen.
596-
if age > MAX_CHANGE_AGE {
597-
*last_change_tick = change_tick.wrapping_sub(MAX_CHANGE_AGE);
639+
self.changed.set_changed(change_tick);
598640
}
599641
}

0 commit comments

Comments
 (0)