From e2eb78a1191ffe27bacf07748c29b8a262e2c230 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 10 Nov 2022 11:53:06 -0800 Subject: [PATCH 01/17] Split ComponentTicks internally --- crates/bevy_ecs/src/component.rs | 91 ++++++++++++++++++++------------ 1 file changed, 58 insertions(+), 33 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index c59a1e2028737..40c2cb4af47e0 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -517,23 +517,24 @@ impl Components { } } -/// Records when a component was added and when it was last mutably dereferenced (or added). #[derive(Copy, Clone, Debug)] -pub struct ComponentTicks { - pub(crate) added: u32, - pub(crate) changed: u32, +pub struct Tick { + pub(crate) tick: u32, } -impl ComponentTicks { +impl Tick { + const fn new(tick: u32) -> Self { + Self { tick } + } + #[inline] - /// Returns `true` if the component was added after the system last ran. - pub fn is_added(&self, last_change_tick: u32, change_tick: u32) -> bool { + pub fn is_changed(&self, last_change_tick: u32, change_tick: u32) -> bool { // This works even with wraparound because the world tick (`change_tick`) is always "newer" than // `last_change_tick` and `self.added`, and we scan periodically to clamp `ComponentTicks` values // so they never get older than `u32::MAX` (the difference would overflow). // // The clamp here ensures determinism (since scans could differ between app runs). - let ticks_since_insert = change_tick.wrapping_sub(self.added).min(MAX_CHANGE_AGE); + let ticks_since_insert = change_tick.wrapping_sub(self.tick).min(MAX_CHANGE_AGE); let ticks_since_system = change_tick .wrapping_sub(last_change_tick) .min(MAX_CHANGE_AGE); @@ -541,32 +542,65 @@ impl ComponentTicks { ticks_since_system > ticks_since_insert } + fn check_tick(&mut self, change_tick: u32) { + let age = change_tick.wrapping_sub(self.tick); + // This comparison assumes that `age` has not overflowed `u32::MAX` before, which will be true + // so long as this check always runs before that can happen. + if age > MAX_CHANGE_AGE { + self.tick = change_tick.wrapping_sub(MAX_CHANGE_AGE); + } + } + + /// Manually sets the change tick. + /// + /// This is normally done automatically via the [`DerefMut`](std::ops::DerefMut) implementation + /// on [`Mut`](crate::change_detection::Mut), [`ResMut`](crate::change_detection::ResMut), etc. + /// However, components and resources that make use of interior mutability might require manual updates. + /// + /// # Example + /// ```rust,no_run + /// # use bevy_ecs::{world::World, component::ComponentTicks}; + /// let world: World = unimplemented!(); + /// let component_ticks: ComponentTicks = unimplemented!(); + /// + /// component_ticks.set_changed(world.read_change_tick()); + /// ``` + #[inline] + pub fn set_changed(&mut self, change_tick: u32) { + self.tick = change_tick; + } +} + +/// Records when a component was added and when it was last mutably dereferenced (or added). +#[derive(Copy, Clone, Debug)] +pub struct ComponentTicks { + pub(crate) added: Tick, + pub(crate) changed: Tick, +} + +impl ComponentTicks { + #[inline] + /// Returns `true` if the component was added after the system last ran. + pub fn is_added(&self, last_change_tick: u32, change_tick: u32) -> bool { + self.changed.is_changed(last_change_tick, change_tick) + } + #[inline] /// Returns `true` if the component was added or mutably dereferenced after the system last ran. pub fn is_changed(&self, last_change_tick: u32, change_tick: u32) -> bool { - // This works even with wraparound because the world tick (`change_tick`) is always "newer" than - // `last_change_tick` and `self.changed`, and we scan periodically to clamp `ComponentTicks` values - // so they never get older than `u32::MAX` (the difference would overflow). - // - // The clamp here ensures determinism (since scans could differ between app runs). - let ticks_since_change = change_tick.wrapping_sub(self.changed).min(MAX_CHANGE_AGE); - let ticks_since_system = change_tick - .wrapping_sub(last_change_tick) - .min(MAX_CHANGE_AGE); - - ticks_since_system > ticks_since_change + self.changed.is_changed(last_change_tick, change_tick) } pub(crate) fn new(change_tick: u32) -> Self { Self { - added: change_tick, - changed: change_tick, + added: Tick::new(change_tick), + changed: Tick::new(change_tick), } } pub(crate) fn check_ticks(&mut self, change_tick: u32) { - check_tick(&mut self.added, change_tick); - check_tick(&mut self.changed, change_tick); + self.added.check_tick(change_tick); + self.changed.check_tick(change_tick); } /// Manually sets the change tick. @@ -585,15 +619,6 @@ impl ComponentTicks { /// ``` #[inline] pub fn set_changed(&mut self, change_tick: u32) { - self.changed = change_tick; - } -} - -fn check_tick(last_change_tick: &mut u32, change_tick: u32) { - let age = change_tick.wrapping_sub(*last_change_tick); - // This comparison assumes that `age` has not overflowed `u32::MAX` before, which will be true - // so long as this check always runs before that can happen. - if age > MAX_CHANGE_AGE { - *last_change_tick = change_tick.wrapping_sub(MAX_CHANGE_AGE); + self.changed.set_changed(change_tick); } } From 3560c200020df84a41ccf03600dfbd5147b9b0d6 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 10 Nov 2022 12:02:22 -0800 Subject: [PATCH 02/17] Introduce split ticks into Column --- crates/bevy_ecs/src/component.rs | 7 +-- crates/bevy_ecs/src/storage/table.rs | 73 +++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 40c2cb4af47e0..77981bdfe8831 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -542,7 +542,7 @@ impl Tick { ticks_since_system > ticks_since_insert } - fn check_tick(&mut self, change_tick: u32) { + pub(crate) fn check_tick(&mut self, change_tick: u32) { let age = change_tick.wrapping_sub(self.tick); // This comparison assumes that `age` has not overflowed `u32::MAX` before, which will be true // so long as this check always runs before that can happen. @@ -598,11 +598,6 @@ impl ComponentTicks { } } - pub(crate) fn check_ticks(&mut self, change_tick: u32) { - self.added.check_tick(change_tick); - self.changed.check_tick(change_tick); - } - /// Manually sets the change tick. /// /// This is normally done automatically via the [`DerefMut`](std::ops::DerefMut) implementation diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index adb684469a6d6..fcf2a392ba03a 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -1,5 +1,5 @@ use crate::{ - component::{ComponentId, ComponentInfo, ComponentTicks, Components}, + component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick}, entity::Entity, query::DebugCheckedUnwrap, storage::{blob_vec::BlobVec, SparseSet}, @@ -35,6 +35,8 @@ impl TableId { #[derive(Debug)] pub struct Column { data: BlobVec, + added_ticks: Vec>, + changed_ticks: Vec>, ticks: Vec>, } @@ -44,6 +46,8 @@ impl Column { Column { // SAFETY: component_info.drop() is valid for the types that will be inserted. data: unsafe { BlobVec::new(component_info.layout(), component_info.drop(), capacity) }, + added_ticks: Vec::with_capacity(capacity), + changed_ticks: Vec::with_capacity(capacity), ticks: Vec::with_capacity(capacity), } } @@ -68,6 +72,8 @@ impl Column { ) { debug_assert!(row < self.len()); self.data.initialize_unchecked(row, data); + *self.added_ticks.get_unchecked_mut(row).get_mut() = ticks.added; + *self.changed_ticks.get_unchecked_mut(row).get_mut() = ticks.changed; *self.ticks.get_unchecked_mut(row).get_mut() = ticks; } @@ -80,6 +86,10 @@ impl Column { pub(crate) unsafe fn replace(&mut self, row: usize, data: OwningPtr<'_>, change_tick: u32) { debug_assert!(row < self.len()); self.data.replace_unchecked(row, data); + self.changed_ticks + .get_unchecked_mut(row) + .get_mut() + .set_changed(change_tick); self.ticks .get_unchecked_mut(row) .get_mut() @@ -113,6 +123,8 @@ impl Column { #[inline] pub(crate) unsafe fn swap_remove_unchecked(&mut self, row: usize) { self.data.swap_remove_and_drop_unchecked(row); + self.added_ticks.swap_remove(row); + self.changed_ticks.swap_remove(row); self.ticks.swap_remove(row); } @@ -125,8 +137,9 @@ impl Column { (row < self.data.len()).then(|| { // SAFETY: The row was length checked before this. let data = unsafe { self.data.swap_remove_and_forget_unchecked(row) }; - let ticks = self.ticks.swap_remove(row).into_inner(); - (data, ticks) + let added = self.added_ticks.swap_remove(row).into_inner(); + let changed = self.added_ticks.swap_remove(row).into_inner(); + (data, ComponentTicks { added, changed }) }) } @@ -139,8 +152,9 @@ impl Column { row: usize, ) -> (OwningPtr<'_>, ComponentTicks) { let data = self.data.swap_remove_and_forget_unchecked(row); - let ticks = self.ticks.swap_remove(row).into_inner(); - (data, ticks) + let added = self.added_ticks.swap_remove(row).into_inner(); + let changed = self.added_ticks.swap_remove(row).into_inner(); + (data, ComponentTicks { added, changed }) } /// Removes the element from `other` at `src_row` and inserts it @@ -164,6 +178,8 @@ impl Column { debug_assert!(self.data.layout() == other.data.layout()); let ptr = self.data.get_unchecked_mut(dst_row); other.data.swap_remove_unchecked(src_row, ptr); + *self.added_ticks.get_unchecked_mut(dst_row) = other.added_ticks.swap_remove(src_row); + *self.changed_ticks.get_unchecked_mut(dst_row) = other.changed_ticks.swap_remove(src_row); *self.ticks.get_unchecked_mut(dst_row) = other.ticks.swap_remove(src_row); } @@ -171,12 +187,16 @@ impl Column { // - ptr must point to valid data of this column's component type pub(crate) unsafe fn push(&mut self, ptr: OwningPtr<'_>, ticks: ComponentTicks) { self.data.push(ptr); + self.added_ticks.push(UnsafeCell::new(ticks.added)); + self.changed_ticks.push(UnsafeCell::new(ticks.changed)); self.ticks.push(UnsafeCell::new(ticks)); } #[inline] pub(crate) fn reserve_exact(&mut self, additional: usize) { self.data.reserve_exact(additional); + self.added_ticks.reserve_exact(additional); + self.changed_ticks.reserve_exact(additional); self.ticks.reserve_exact(additional); } @@ -191,6 +211,16 @@ impl Column { self.data.get_slice() } + #[inline] + pub fn get_added_ticks_slice(&self) -> &[UnsafeCell] { + &self.added_ticks + } + + #[inline] + pub fn get_changed_ticks_slice(&self) -> &[UnsafeCell] { + &self.changed_ticks + } + #[inline] pub fn get_ticks_slice(&self) -> &[UnsafeCell] { &self.ticks @@ -236,11 +266,37 @@ impl Column { self.data.get_unchecked_mut(row) } + #[inline] + pub fn get_added_ticks(&self, row: usize) -> Option<&UnsafeCell> { + self.added_ticks.get(row) + } + + #[inline] + pub fn get_changed_ticks(&self, row: usize) -> Option<&UnsafeCell> { + self.changed_ticks.get(row) + } + #[inline] pub fn get_ticks(&self, row: usize) -> Option<&UnsafeCell> { self.ticks.get(row) } + /// # Safety + /// index must be in-bounds + #[inline] + pub unsafe fn get_added_ticks_unchecked(&self, row: usize) -> &UnsafeCell { + debug_assert!(row < self.added_ticks.len()); + self.added_ticks.get_unchecked(row) + } + + /// # Safety + /// index must be in-bounds + #[inline] + pub unsafe fn get_changed_ticks_unchecked(&self, row: usize) -> &UnsafeCell { + debug_assert!(row < self.changed_ticks.len()); + self.changed_ticks.get_unchecked(row) + } + /// # Safety /// index must be in-bounds #[inline] @@ -256,8 +312,11 @@ impl Column { #[inline] pub(crate) fn check_change_ticks(&mut self, change_tick: u32) { - for component_ticks in &mut self.ticks { - component_ticks.get_mut().check_ticks(change_tick); + for component_ticks in &mut self.added_ticks { + component_ticks.get_mut().check_tick(change_tick); + } + for component_ticks in &mut self.changed_ticks { + component_ticks.get_mut().check_tick(change_tick); } } } From e21916ac6a7ce823c0b01ae1df7b9ff9090ce070 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 10 Nov 2022 12:20:55 -0800 Subject: [PATCH 03/17] Split ticks in SparseSet --- crates/bevy_ecs/src/storage/sparse_set.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index fdb9a21176a5f..f6d6badb40368 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -1,5 +1,5 @@ use crate::{ - component::{ComponentId, ComponentInfo, ComponentTicks}, + component::{ComponentId, ComponentInfo, ComponentTicks, Tick}, entity::Entity, storage::Column, }; @@ -178,6 +178,24 @@ impl ComponentSparseSet { } } + #[inline] + pub fn get_added_ticks(&self, entity: Entity) -> Option<&UnsafeCell> { + let dense_index = *self.sparse.get(entity.index())? as usize; + #[cfg(debug_assertions)] + assert_eq!(entity, self.entities[dense_index]); + // SAFETY: if the sparse index points to something in the dense vec, it exists + unsafe { Some(self.dense.get_added_ticks_unchecked(dense_index)) } + } + + #[inline] + pub fn get_changed_ticks(&self, entity: Entity) -> Option<&UnsafeCell> { + let dense_index = *self.sparse.get(entity.index())? as usize; + #[cfg(debug_assertions)] + assert_eq!(entity, self.entities[dense_index]); + // SAFETY: if the sparse index points to something in the dense vec, it exists + unsafe { Some(self.dense.get_changed_ticks_unchecked(dense_index)) } + } + #[inline] pub fn get_ticks(&self, entity: Entity) -> Option<&UnsafeCell> { let dense_index = *self.sparse.get(entity.index())? as usize; From 4f1e4259f726a68f214ed0dc2d5c5fe9ad9d655a Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 10 Nov 2022 23:43:29 -0800 Subject: [PATCH 04/17] Bubble up splitting --- crates/bevy_ecs/src/change_detection.rs | 21 ++++---- crates/bevy_ecs/src/query/fetch.rs | 18 ++++--- crates/bevy_ecs/src/storage/resource.rs | 4 +- crates/bevy_ecs/src/storage/sparse_set.rs | 5 +- crates/bevy_ecs/src/storage/table.rs | 10 +++- crates/bevy_ecs/src/system/system_param.rs | 60 ++++++++++++++-------- crates/bevy_ecs/src/world/entity_ref.rs | 29 ++++++----- crates/bevy_ecs/src/world/mod.rs | 17 +++--- 8 files changed, 101 insertions(+), 63 deletions(-) diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index b8d1f7c196d35..d8f305b3ab755 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -1,6 +1,6 @@ //! Types that detect when their internal data mutate. -use crate::{component::ComponentTicks, ptr::PtrMut, system::Resource}; +use crate::{component::Tick, ptr::PtrMut, system::Resource}; use std::ops::{Deref, DerefMut}; /// The (arbitrarily chosen) minimum number of world tick increments between `check_tick` scans. @@ -95,21 +95,21 @@ macro_rules! change_detection_impl { #[inline] fn is_added(&self) -> bool { self.ticks - .component_ticks - .is_added(self.ticks.last_change_tick, self.ticks.change_tick) + .added + .is_changed(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] fn is_changed(&self) -> bool { self.ticks - .component_ticks + .changed .is_changed(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] fn set_changed(&mut self) { self.ticks - .component_ticks + .changed .set_changed(self.ticks.change_tick); } @@ -224,7 +224,8 @@ macro_rules! impl_debug { } pub(crate) struct Ticks<'a> { - pub(crate) component_ticks: &'a mut ComponentTicks, + pub(crate) added: &'a mut Tick, + pub(crate) changed: &'a mut Tick, pub(crate) last_change_tick: u32, pub(crate) change_tick: u32, } @@ -381,21 +382,21 @@ impl<'a> DetectChanges for MutUntyped<'a> { #[inline] fn is_added(&self) -> bool { self.ticks - .component_ticks - .is_added(self.ticks.last_change_tick, self.ticks.change_tick) + .added + .is_changed(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] fn is_changed(&self) -> bool { self.ticks - .component_ticks + .changed .is_changed(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] fn set_changed(&mut self) { self.ticks - .component_ticks + .changed .set_changed(self.ticks.change_tick); } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index ad6598bcc6e0d..e013f9d3798d6 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1,7 +1,7 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, change_detection::Ticks, - component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType}, + component::{Component, ComponentId, ComponentStorage, Tick, ComponentTicks, StorageType}, entity::Entity, query::{Access, DebugCheckedUnwrap, FilteredAccess}, storage::{ComponentSparseSet, Table}, @@ -654,7 +654,8 @@ pub struct WriteFetch<'w, T> { // T::Storage = TableStorage table_data: Option<( ThinSlicePtr<'w, UnsafeCell>, - ThinSlicePtr<'w, UnsafeCell>, + ThinSlicePtr<'w, UnsafeCell>, + ThinSlicePtr<'w, UnsafeCell>, )>, // T::Storage = SparseStorage sparse_set: Option<&'w ComponentSparseSet>, @@ -733,7 +734,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { let column = table.get_column(component_id).debug_checked_unwrap(); fetch.table_data = Some(( column.get_data_slice().into(), - column.get_ticks_slice().into(), + column.get_added_ticks_slice().into(), + column.get_changed_ticks_slice().into(), )); } @@ -745,18 +747,19 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { ) -> Self::Item<'w> { match T::Storage::STORAGE_TYPE { StorageType::Table => { - let (table_components, table_ticks) = fetch.table_data.debug_checked_unwrap(); + let (table_components, added_ticks, changed_ticks) = fetch.table_data.debug_checked_unwrap(); Mut { value: table_components.get(table_row).deref_mut(), ticks: Ticks { - component_ticks: table_ticks.get(table_row).deref_mut(), + added: added_ticks.get(table_row).deref_mut(), + changed: changed_ticks.get(table_row).deref_mut(), change_tick: fetch.change_tick, last_change_tick: fetch.last_change_tick, }, } } StorageType::SparseSet => { - let (component, component_ticks) = fetch + let (component, added, changed) = fetch .sparse_set .debug_checked_unwrap() .get_with_ticks(entity) @@ -764,7 +767,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { Mut { value: component.assert_unique().deref_mut(), ticks: Ticks { - component_ticks: component_ticks.deref_mut(), + added: added.deref_mut(), + changed: changed.deref_mut(), change_tick: fetch.change_tick, last_change_tick: fetch.last_change_tick, }, diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index edfa51e12857a..72a0fe6d4bab4 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -1,5 +1,5 @@ use crate::archetype::ArchetypeComponentId; -use crate::component::{ComponentId, ComponentTicks, Components}; +use crate::component::{ComponentId, ComponentTicks, Tick, Components}; use crate::storage::{Column, SparseSet}; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; use std::cell::UnsafeCell; @@ -44,7 +44,7 @@ impl ResourceData { } #[inline] - pub(crate) fn get_with_ticks(&self) -> Option<(Ptr<'_>, &UnsafeCell)> { + pub(crate) fn get_with_ticks(&self) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { self.column.get(0) } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index f6d6badb40368..27c1c6db07c8b 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -165,7 +165,7 @@ impl ComponentSparseSet { } #[inline] - pub fn get_with_ticks(&self, entity: Entity) -> Option<(Ptr<'_>, &UnsafeCell)> { + pub fn get_with_ticks(&self, entity: Entity) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { let dense_index = *self.sparse.get(entity.index())? as usize; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index]); @@ -173,7 +173,8 @@ impl ComponentSparseSet { unsafe { Some(( self.dense.get_data_unchecked(dense_index), - self.dense.get_ticks_unchecked(dense_index), + self.dense.get_added_ticks_unchecked(dense_index), + self.dense.get_changed_ticks_unchecked(dense_index), )) } } diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index fcf2a392ba03a..97c7ed4da9289 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -227,11 +227,17 @@ impl Column { } #[inline] - pub fn get(&self, row: usize) -> Option<(Ptr<'_>, &UnsafeCell)> { + pub fn get(&self, row: usize) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { (row < self.data.len()) // SAFETY: The row is length checked before fetching the pointer. This is being // accessed through a read-only reference to the column. - .then(|| unsafe { (self.data.get_unchecked(row), self.ticks.get_unchecked(row)) }) + .then(|| unsafe { + ( + self.data.get_unchecked(row), + self.added_ticks.get_unchecked(row), + self.changed_ticks.get_unchecked(row), + ) + }) } #[inline] diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index fa5f5bf517785..db94d51f3d202 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -3,7 +3,7 @@ use crate::{ archetype::{Archetype, Archetypes}, bundle::Bundles, change_detection::Ticks, - component::{Component, ComponentId, ComponentTicks, Components}, + component::{Component, ComponentId, ComponentTicks, Tick, Components}, entity::{Entities, Entity}, query::{ Access, FilteredAccess, FilteredAccessSet, QueryState, ReadOnlyWorldQuery, WorldQuery, @@ -273,7 +273,8 @@ pub trait Resource: Send + Sync + 'static {} /// Use `Option>` instead if the resource might not always exist. pub struct Res<'w, T: Resource> { value: &'w T, - ticks: &'w ComponentTicks, + added: &'w Tick, + changed: &'w Tick, last_change_tick: u32, change_tick: u32, } @@ -296,7 +297,8 @@ impl<'w, T: Resource> Res<'w, T> { pub fn clone(this: &Self) -> Self { Self { value: this.value, - ticks: this.ticks, + added: this.added, + changed: this.changed, last_change_tick: this.last_change_tick, change_tick: this.change_tick, } @@ -304,12 +306,12 @@ impl<'w, T: Resource> Res<'w, T> { /// Returns `true` if the resource was added after the system last ran. pub fn is_added(&self) -> bool { - self.ticks.is_added(self.last_change_tick, self.change_tick) + self.added.is_changed(self.last_change_tick, self.change_tick) } /// Returns `true` if the resource was added or mutably dereferenced after the system last ran. pub fn is_changed(&self) -> bool { - self.ticks + self.changed .is_changed(self.last_change_tick, self.change_tick) } @@ -337,7 +339,8 @@ impl<'w, T: Resource> From> for Res<'w, T> { fn from(res: ResMut<'w, T>) -> Self { Self { value: res.value, - ticks: res.ticks.component_ticks, + added: res.ticks.added, + changed: res.ticks.changed, change_tick: res.ticks.change_tick, last_change_tick: res.ticks.last_change_tick, } @@ -404,7 +407,7 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResState { world: &'w World, change_tick: u32, ) -> Self::Item { - let (ptr, ticks) = world + let (ptr, added, changed) = world .get_resource_with_ticks(state.component_id) .unwrap_or_else(|| { panic!( @@ -415,7 +418,8 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResState { }); Res { value: ptr.deref(), - ticks: ticks.deref(), + added: added.deref(), + changed: changed.deref(), last_change_tick: system_meta.last_change_tick, change_tick, } @@ -454,9 +458,10 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for OptionResState { ) -> Self::Item { world .get_resource_with_ticks(state.0.component_id) - .map(|(ptr, ticks)| Res { + .map(|(ptr, added, changed)| Res { value: ptr.deref(), - ticks: ticks.deref(), + added: added.deref(), + changed: changed.deref(), last_change_tick: system_meta.last_change_tick, change_tick, }) @@ -526,7 +531,8 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResMutState { ResMut { value: value.value, ticks: Ticks { - component_ticks: value.ticks.component_ticks, + added: value.ticks.added, + changed: value.ticks.changed, last_change_tick: system_meta.last_change_tick, change_tick, }, @@ -566,7 +572,8 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for OptionResMutState { .map(|value| ResMut { value: value.value, ticks: Ticks { - component_ticks: value.ticks.component_ticks, + added: value.ticks.added, + changed: value.ticks.changed, last_change_tick: system_meta.last_change_tick, change_tick, }, @@ -938,7 +945,10 @@ impl<'a, T> From> for NonSend<'a, T> { fn from(nsm: NonSendMut<'a, T>) -> Self { Self { value: nsm.value, - ticks: nsm.ticks.component_ticks.to_owned(), + ticks: ComponentTicks { + added: nsm.ticks.added.to_owned(), + changed: nsm.ticks.changed.to_owned(), + }, change_tick: nsm.ticks.change_tick, last_change_tick: nsm.ticks.last_change_tick, } @@ -996,7 +1006,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendState { change_tick: u32, ) -> Self::Item { world.validate_non_send_access::(); - let (ptr, ticks) = world + let (ptr, added, changed) = world .get_resource_with_ticks(state.component_id) .unwrap_or_else(|| { panic!( @@ -1008,7 +1018,10 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendState { NonSend { value: ptr.deref(), - ticks: ticks.read(), + ticks: ComponentTicks { + added: added.read(), + changed: changed.read(), + }, last_change_tick: system_meta.last_change_tick, change_tick, } @@ -1048,9 +1061,12 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for OptionNonSendState { world.validate_non_send_access::(); world .get_resource_with_ticks(state.0.component_id) - .map(|(ptr, ticks)| NonSend { + .map(|(ptr, added, changed)| NonSend { value: ptr.deref(), - ticks: ticks.read(), + ticks: ComponentTicks { + added: added.read(), + changed: changed.read(), + }, last_change_tick: system_meta.last_change_tick, change_tick, }) @@ -1111,7 +1127,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendMutState { change_tick: u32, ) -> Self::Item { world.validate_non_send_access::(); - let (ptr, ticks) = world + let (ptr, added, changed) = world .get_resource_with_ticks(state.component_id) .unwrap_or_else(|| { panic!( @@ -1123,7 +1139,8 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendMutState { NonSendMut { value: ptr.assert_unique().deref_mut(), ticks: Ticks { - component_ticks: ticks.deref_mut(), + added: added.deref_mut(), + changed: changed.deref_mut(), last_change_tick: system_meta.last_change_tick, change_tick, }, @@ -1161,10 +1178,11 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for OptionNonSendMutState { world.validate_non_send_access::(); world .get_resource_with_ticks(state.0.component_id) - .map(|(ptr, ticks)| NonSendMut { + .map(|(ptr, added, changed)| NonSendMut { value: ptr.assert_unique().deref_mut(), ticks: Ticks { - component_ticks: ticks.deref_mut(), + added: added.deref_mut(), + changed: changed.deref_mut(), last_change_tick: system_meta.last_change_tick, change_tick, }, diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index c9f9321d5a7b3..5d550c191c84a 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2,7 +2,7 @@ use crate::{ archetype::{Archetype, ArchetypeId, Archetypes}, bundle::{Bundle, BundleInfo}, change_detection::{MutUntyped, Ticks}, - component::{Component, ComponentId, ComponentTicks, Components, StorageType}, + component::{Component, ComponentId, Tick, ComponentTicks, Components, StorageType}, entity::{Entities, Entity, EntityLocation}, storage::{SparseSet, Storages}, world::{Mut, World}, @@ -102,10 +102,11 @@ impl<'w> EntityRef<'w> { change_tick: u32, ) -> Option> { get_component_and_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) - .map(|(value, ticks)| Mut { + .map(|(value, added, changed)| Mut { value: value.assert_unique().deref_mut::(), ticks: Ticks { - component_ticks: ticks.deref_mut(), + added: added.deref_mut(), + changed: changed.deref_mut(), last_change_tick, change_tick, }, @@ -229,10 +230,11 @@ impl<'w> EntityMut<'w> { #[inline] pub unsafe fn get_unchecked_mut(&self) -> Option> { get_component_and_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) - .map(|(value, ticks)| Mut { + .map(|(value, added, changed)| Mut { value: value.assert_unique().deref_mut::(), ticks: Ticks { - component_ticks: ticks.deref_mut(), + added: added.deref_mut(), + changed: changed.deref_mut(), last_change_tick: self.world.last_change_tick(), change_tick: self.world.read_change_tick(), }, @@ -635,7 +637,7 @@ unsafe fn get_component_and_ticks( component_id: ComponentId, entity: Entity, location: EntityLocation, -) -> Option<(Ptr<'_>, &UnsafeCell)> { +) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { let archetype = &world.archetypes[location.archetype_id]; let component_info = world.components.get_info_unchecked(component_id); match component_info.storage_type() { @@ -646,7 +648,8 @@ unsafe fn get_component_and_ticks( // SAFETY: archetypes only store valid table_rows and the stored component type is T Some(( components.get_data_unchecked(table_row), - components.get_ticks_unchecked(table_row), + components.get_added_ticks_unchecked(table_row), + components.get_changed_ticks_unchecked(table_row), )) } StorageType::SparseSet => world @@ -747,7 +750,7 @@ pub(crate) unsafe fn get_component_and_ticks_with_type( type_id: TypeId, entity: Entity, location: EntityLocation, -) -> Option<(Ptr<'_>, &UnsafeCell)> { +) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { let component_id = world.components.get_id(type_id)?; get_component_and_ticks(world, component_id, entity, location) } @@ -909,10 +912,11 @@ pub(crate) unsafe fn get_mut( let change_tick = world.change_tick(); let last_change_tick = world.last_change_tick(); get_component_and_ticks_with_type(world, TypeId::of::(), entity, location).map( - |(value, ticks)| Mut { + |(value, added, changed)| Mut { value: value.assert_unique().deref_mut::(), ticks: Ticks { - component_ticks: ticks.deref_mut(), + added: added.deref_mut(), + changed: changed.deref_mut(), last_change_tick, change_tick, }, @@ -929,11 +933,12 @@ pub(crate) unsafe fn get_mut_by_id( component_id: ComponentId, ) -> Option { // SAFETY: world access is unique, entity location and component_id required to be valid - get_component_and_ticks(world, component_id, entity, location).map(|(value, ticks)| { + get_component_and_ticks(world, component_id, entity, location).map(|(value, added, changed)| { MutUntyped { value: value.assert_unique(), ticks: Ticks { - component_ticks: ticks.deref_mut(), + added: added.deref_mut(), + changed: changed.deref_mut(), last_change_tick: world.last_change_tick(), change_tick: world.read_change_tick(), }, diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 1aab56a9f4ae7..5f2dbd2d0110a 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -12,7 +12,7 @@ use crate::{ bundle::{Bundle, BundleInserter, BundleSpawner, Bundles}, change_detection::{MutUntyped, Ticks}, component::{ - Component, ComponentDescriptor, ComponentId, ComponentInfo, ComponentTicks, Components, + Component, ComponentDescriptor, ComponentId, ComponentInfo, Tick, Components, }, entity::{AllocAtWithoutReplacement, Entities, Entity}, query::{QueryState, ReadOnlyWorldQuery, WorldQuery}, @@ -1001,7 +1001,7 @@ impl World { pub(crate) fn get_resource_with_ticks( &self, component_id: ComponentId, - ) -> Option<(Ptr<'_>, &UnsafeCell)> { + ) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { self.storages.resources.get(component_id)?.get_with_ticks() } @@ -1194,7 +1194,8 @@ impl World { let value_mut = Mut { value: &mut value, ticks: Ticks { - component_ticks: &mut ticks, + added: &mut ticks.added, + changed: &mut ticks.changed, last_change_tick, change_tick, }, @@ -1270,11 +1271,12 @@ impl World { &self, component_id: ComponentId, ) -> Option> { - let (ptr, ticks) = self.get_resource_with_ticks(component_id)?; + let (ptr, added, changed) = self.get_resource_with_ticks(component_id)?; Some(Mut { value: ptr.assert_unique().deref_mut(), ticks: Ticks { - component_ticks: ticks.deref_mut(), + added: added.deref_mut(), + changed: changed.deref_mut(), last_change_tick: self.last_change_tick(), change_tick: self.read_change_tick(), }, @@ -1455,14 +1457,15 @@ impl World { self.validate_non_send_access_untyped(info.name()); } - let (ptr, ticks) = self.get_resource_with_ticks(component_id)?; + let (ptr, added, changed) = self.get_resource_with_ticks(component_id)?; // SAFE: This function has exclusive access to the world so nothing aliases `ticks`. let ticks = Ticks { // SAFETY: // - index is in-bounds because the column is initialized and non-empty // - no other reference to the ticks of the same row can exist at the same time - component_ticks: unsafe { ticks.deref_mut() }, + added: unsafe { added.deref_mut() }, + changed: unsafe { changed.deref_mut() }, last_change_tick: self.last_change_tick(), change_tick: self.read_change_tick(), }; From b356548be601318aea5ffe67afeef1d024ead6f4 Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 11 Nov 2022 01:00:36 -0800 Subject: [PATCH 05/17] Remove Column::ticks --- crates/bevy_ecs/src/change_detection.rs | 4 +- crates/bevy_ecs/src/component.rs | 2 +- crates/bevy_ecs/src/query/fetch.rs | 36 +++++++++------- crates/bevy_ecs/src/query/filter.rs | 48 +++++++++++---------- crates/bevy_ecs/src/storage/resource.rs | 15 +++---- crates/bevy_ecs/src/storage/sparse_set.rs | 7 +++- crates/bevy_ecs/src/storage/table.rs | 49 ++++++++++------------ crates/bevy_ecs/src/system/system_param.rs | 5 ++- crates/bevy_ecs/src/world/entity_ref.rs | 20 ++++----- crates/bevy_ecs/src/world/mod.rs | 4 +- 10 files changed, 91 insertions(+), 99 deletions(-) diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index d8f305b3ab755..27dd17c8528e4 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -395,9 +395,7 @@ impl<'a> DetectChanges for MutUntyped<'a> { #[inline] fn set_changed(&mut self) { - self.ticks - .changed - .set_changed(self.ticks.change_tick); + self.ticks.changed.set_changed(self.ticks.change_tick); } #[inline] diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 77981bdfe8831..e7b7c5351009b 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -523,7 +523,7 @@ pub struct Tick { } impl Tick { - const fn new(tick: u32) -> Self { + pub const fn new(tick: u32) -> Self { Self { tick } } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index e013f9d3798d6..4b06ef2f76505 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1,7 +1,7 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, change_detection::Ticks, - component::{Component, ComponentId, ComponentStorage, Tick, ComponentTicks, StorageType}, + component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType, Tick}, entity::Entity, query::{Access, DebugCheckedUnwrap, FilteredAccess}, storage::{ComponentSparseSet, Table}, @@ -747,7 +747,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { ) -> Self::Item<'w> { match T::Storage::STORAGE_TYPE { StorageType::Table => { - let (table_components, added_ticks, changed_ticks) = fetch.table_data.debug_checked_unwrap(); + let (table_components, added_ticks, changed_ticks) = + fetch.table_data.debug_checked_unwrap(); Mut { value: table_components.get(table_row).deref_mut(), ticks: Ticks { @@ -767,7 +768,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { Mut { value: component.assert_unique().deref_mut(), ticks: Ticks { - added: added.deref_mut(), + added: added.deref_mut(), changed: changed.deref_mut(), change_tick: fetch.change_tick, last_change_tick: fetch.last_change_tick, @@ -996,7 +997,10 @@ impl ChangeTrackers { #[doc(hidden)] pub struct ChangeTrackersFetch<'w, T> { // T::Storage = TableStorage - table_ticks: Option>>, + table_ticks: Option<( + ThinSlicePtr<'w, UnsafeCell>, // Added + ThinSlicePtr<'w, UnsafeCell>, // Changed + )>, // T::Storage = SparseStorage sparse_set: Option<&'w ComponentSparseSet>, @@ -1074,13 +1078,11 @@ unsafe impl WorldQuery for ChangeTrackers { &id: &ComponentId, table: &'w Table, ) { - fetch.table_ticks = Some( - table - .get_column(id) - .debug_checked_unwrap() - .get_ticks_slice() - .into(), - ); + let column = table.get_column(id).debug_checked_unwrap(); + fetch.table_ticks = Some(( + column.get_added_ticks_slice().into(), + column.get_changed_ticks_slice().into(), + )); } #[inline(always)] @@ -1092,20 +1094,22 @@ unsafe impl WorldQuery for ChangeTrackers { match T::Storage::STORAGE_TYPE { StorageType::Table => ChangeTrackers { component_ticks: { - let table_ticks = fetch.table_ticks.debug_checked_unwrap(); - table_ticks.get(table_row).read() + let (added, changed) = fetch.table_ticks.debug_checked_unwrap(); + ComponentTicks { + added: added.get(table_row).read(), + changed: changed.get(table_row).read(), + } }, marker: PhantomData, last_change_tick: fetch.last_change_tick, change_tick: fetch.change_tick, }, StorageType::SparseSet => ChangeTrackers { - component_ticks: *fetch + component_ticks: fetch .sparse_set .debug_checked_unwrap() .get_ticks(entity) - .debug_checked_unwrap() - .get(), + .debug_checked_unwrap(), marker: PhantomData, last_change_tick: fetch.last_change_tick, change_tick: fetch.change_tick, diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 2cf2fc3e1c10b..91184e0b01b21 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -1,6 +1,6 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, - component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType}, + component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType, Tick}, entity::Entity, query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery}, storage::{ComponentSparseSet, Table}, @@ -413,7 +413,10 @@ macro_rules! impl_tick_filter { #[doc(hidden)] $(#[$fetch_meta])* pub struct $fetch_name<'w, T> { - table_ticks: Option>>, + table_ticks: Option<( + ThinSlicePtr<'w, UnsafeCell>, + ThinSlicePtr<'w, UnsafeCell> + )>, marker: PhantomData, sparse_set: Option<&'w ComponentSparseSet>, last_change_tick: u32, @@ -474,12 +477,13 @@ macro_rules! impl_tick_filter { &component_id: &ComponentId, table: &'w Table ) { - fetch.table_ticks = Some( - table.get_column(component_id) - .debug_checked_unwrap() - .get_ticks_slice() - .into() - ); + let column = table + .get_column(component_id) + .debug_checked_unwrap(); + fetch.table_ticks = Some(( + column.get_added_ticks_slice().into(), + column.get_changed_ticks_slice().into() + )); } #[inline] @@ -502,24 +506,24 @@ macro_rules! impl_tick_filter { ) -> Self::Item<'w> { match T::Storage::STORAGE_TYPE { StorageType::Table => { - $is_detected(&*( - fetch.table_ticks - .debug_checked_unwrap() - .get(table_row)) - .deref(), + let (added, changed) = fetch.table_ticks.debug_checked_unwrap(); + let ticks = ComponentTicks { + added: added.get(table_row).read(), + changed: changed.get(table_row).read() + }; + $is_detected(&ticks, fetch.last_change_tick, fetch.change_tick) + } + StorageType::SparseSet => { + $is_detected( + &fetch + .sparse_set + .debug_checked_unwrap() + .get_ticks(entity) + .debug_checked_unwrap(), fetch.last_change_tick, fetch.change_tick ) } - StorageType::SparseSet => { - let ticks = &*fetch - .sparse_set - .debug_checked_unwrap() - .get_ticks(entity) - .debug_checked_unwrap() - .get(); - $is_detected(ticks, fetch.last_change_tick, fetch.change_tick) - } } } diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 72a0fe6d4bab4..ce10dba1cc9ce 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -1,5 +1,5 @@ use crate::archetype::ArchetypeComponentId; -use crate::component::{ComponentId, ComponentTicks, Tick, Components}; +use crate::component::{ComponentId, ComponentTicks, Components, Tick}; use crate::storage::{Column, SparseSet}; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; use std::cell::UnsafeCell; @@ -33,14 +33,8 @@ impl ResourceData { /// Gets a read-only reference to the change ticks of the underlying resource, if available. #[inline] - pub fn get_ticks(&self) -> Option<&ComponentTicks> { - self.column - .get_ticks(0) - // SAFETY: - // - This borrow's lifetime is bounded by the lifetime on self. - // - A read-only borrow on self can only exist while a mutable borrow doesn't - // exist. - .map(|ticks| unsafe { ticks.deref() }) + pub fn get_ticks(&self) -> Option { + self.column.get_ticks(0) } #[inline] @@ -85,7 +79,8 @@ impl ResourceData { ) { if self.is_present() { self.column.replace_untracked(0, value); - *self.column.get_ticks_unchecked(0).deref_mut() = change_ticks; + *self.column.get_added_ticks_unchecked(0).deref_mut() = change_ticks.added; + *self.column.get_changed_ticks_unchecked(0).deref_mut() = change_ticks.changed; } else { self.column.push(value, change_ticks); } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 27c1c6db07c8b..1ea120fa11fc6 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -165,7 +165,10 @@ impl ComponentSparseSet { } #[inline] - pub fn get_with_ticks(&self, entity: Entity) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { + pub fn get_with_ticks( + &self, + entity: Entity, + ) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { let dense_index = *self.sparse.get(entity.index())? as usize; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index]); @@ -198,7 +201,7 @@ impl ComponentSparseSet { } #[inline] - pub fn get_ticks(&self, entity: Entity) -> Option<&UnsafeCell> { + pub fn get_ticks(&self, entity: Entity) -> Option { let dense_index = *self.sparse.get(entity.index())? as usize; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index]); diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 97c7ed4da9289..078fa8020c850 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -4,7 +4,7 @@ use crate::{ query::DebugCheckedUnwrap, storage::{blob_vec::BlobVec, SparseSet}, }; -use bevy_ptr::{OwningPtr, Ptr, PtrMut}; +use bevy_ptr::{OwningPtr, Ptr, PtrMut, UnsafeCellDeref}; use bevy_utils::HashMap; use std::alloc::Layout; use std::{ @@ -37,7 +37,6 @@ pub struct Column { data: BlobVec, added_ticks: Vec>, changed_ticks: Vec>, - ticks: Vec>, } impl Column { @@ -48,7 +47,6 @@ impl Column { data: unsafe { BlobVec::new(component_info.layout(), component_info.drop(), capacity) }, added_ticks: Vec::with_capacity(capacity), changed_ticks: Vec::with_capacity(capacity), - ticks: Vec::with_capacity(capacity), } } @@ -74,7 +72,6 @@ impl Column { self.data.initialize_unchecked(row, data); *self.added_ticks.get_unchecked_mut(row).get_mut() = ticks.added; *self.changed_ticks.get_unchecked_mut(row).get_mut() = ticks.changed; - *self.ticks.get_unchecked_mut(row).get_mut() = ticks; } /// Writes component data to the column at given row. @@ -90,10 +87,6 @@ impl Column { .get_unchecked_mut(row) .get_mut() .set_changed(change_tick); - self.ticks - .get_unchecked_mut(row) - .get_mut() - .set_changed(change_tick); } /// Writes component data to the column at given row. @@ -125,7 +118,6 @@ impl Column { self.data.swap_remove_and_drop_unchecked(row); self.added_ticks.swap_remove(row); self.changed_ticks.swap_remove(row); - self.ticks.swap_remove(row); } #[inline] @@ -180,7 +172,6 @@ impl Column { other.data.swap_remove_unchecked(src_row, ptr); *self.added_ticks.get_unchecked_mut(dst_row) = other.added_ticks.swap_remove(src_row); *self.changed_ticks.get_unchecked_mut(dst_row) = other.changed_ticks.swap_remove(src_row); - *self.ticks.get_unchecked_mut(dst_row) = other.ticks.swap_remove(src_row); } // # Safety @@ -189,7 +180,6 @@ impl Column { self.data.push(ptr); self.added_ticks.push(UnsafeCell::new(ticks.added)); self.changed_ticks.push(UnsafeCell::new(ticks.changed)); - self.ticks.push(UnsafeCell::new(ticks)); } #[inline] @@ -197,7 +187,6 @@ impl Column { self.data.reserve_exact(additional); self.added_ticks.reserve_exact(additional); self.changed_ticks.reserve_exact(additional); - self.ticks.reserve_exact(additional); } #[inline] @@ -221,22 +210,17 @@ impl Column { &self.changed_ticks } - #[inline] - pub fn get_ticks_slice(&self) -> &[UnsafeCell] { - &self.ticks - } - #[inline] pub fn get(&self, row: usize) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { (row < self.data.len()) // SAFETY: The row is length checked before fetching the pointer. This is being // accessed through a read-only reference to the column. - .then(|| unsafe { + .then(|| unsafe { ( - self.data.get_unchecked(row), + self.data.get_unchecked(row), self.added_ticks.get_unchecked(row), self.changed_ticks.get_unchecked(row), - ) + ) }) } @@ -283,8 +267,13 @@ impl Column { } #[inline] - pub fn get_ticks(&self, row: usize) -> Option<&UnsafeCell> { - self.ticks.get(row) + pub fn get_ticks(&self, row: usize) -> Option { + if row < self.data.len() { + None + } else { + // SAFETY: The size of the column has already been checked. + Some(unsafe { self.get_ticks_unchecked(row) }) + } } /// # Safety @@ -306,14 +295,19 @@ impl Column { /// # Safety /// index must be in-bounds #[inline] - pub unsafe fn get_ticks_unchecked(&self, row: usize) -> &UnsafeCell { - debug_assert!(row < self.ticks.len()); - self.ticks.get_unchecked(row) + pub unsafe fn get_ticks_unchecked(&self, row: usize) -> ComponentTicks { + debug_assert!(row < self.added_ticks.len()); + debug_assert!(row < self.changed_ticks.len()); + ComponentTicks { + added: self.added_ticks.get_unchecked(row).read(), + changed: self.changed_ticks.get_unchecked(row).read(), + } } pub fn clear(&mut self) { self.data.clear(); - self.ticks.clear(); + self.added_ticks.clear(); + self.changed_ticks.clear(); } #[inline] @@ -502,7 +496,8 @@ impl Table { self.entities.push(entity); for column in self.columns.values_mut() { column.data.set_len(self.entities.len()); - column.ticks.push(UnsafeCell::new(ComponentTicks::new(0))); + column.added_ticks.push(UnsafeCell::new(Tick::new(0))); + column.changed_ticks.push(UnsafeCell::new(Tick::new(0))); } index } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index db94d51f3d202..7bf68ee625d34 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -3,7 +3,7 @@ use crate::{ archetype::{Archetype, Archetypes}, bundle::Bundles, change_detection::Ticks, - component::{Component, ComponentId, ComponentTicks, Tick, Components}, + component::{Component, ComponentId, ComponentTicks, Components, Tick}, entity::{Entities, Entity}, query::{ Access, FilteredAccess, FilteredAccessSet, QueryState, ReadOnlyWorldQuery, WorldQuery, @@ -306,7 +306,8 @@ impl<'w, T: Resource> Res<'w, T> { /// Returns `true` if the resource was added after the system last ran. pub fn is_added(&self) -> bool { - self.added.is_changed(self.last_change_tick, self.change_tick) + self.added + .is_changed(self.last_change_tick, self.change_tick) } /// Returns `true` if the resource was added or mutably dereferenced after the system last ran. diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 5d550c191c84a..6e11b8f566169 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2,7 +2,7 @@ use crate::{ archetype::{Archetype, ArchetypeId, Archetypes}, bundle::{Bundle, BundleInfo}, change_detection::{MutUntyped, Ticks}, - component::{Component, ComponentId, Tick, ComponentTicks, Components, StorageType}, + component::{Component, ComponentId, ComponentTicks, Components, StorageType, Tick}, entity::{Entities, Entity, EntityLocation}, storage::{SparseSet, Storages}, world::{Mut, World}, @@ -77,12 +77,9 @@ impl<'w> EntityRef<'w> { /// Retrieves the change ticks for the given component. This can be useful for implementing change /// detection in custom runtimes. #[inline] - pub fn get_change_ticks(&self) -> Option<&'w ComponentTicks> { + pub fn get_change_ticks(&self) -> Option { // SAFETY: entity location is valid - unsafe { - get_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) - .map(|ticks| ticks.deref()) - } + unsafe { get_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) } } /// Gets a mutable reference to the component of type `T` associated with @@ -209,12 +206,9 @@ impl<'w> EntityMut<'w> { /// Retrieves the change ticks for the given component. This can be useful for implementing change /// detection in custom runtimes. #[inline] - pub fn get_change_ticks(&self) -> Option<&ComponentTicks> { + pub fn get_change_ticks(&self) -> Option { // SAFETY: entity location is valid - unsafe { - get_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) - .map(|ticks| ticks.deref()) - } + unsafe { get_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) } } /// Gets a mutable reference to the component of type `T` associated with @@ -666,7 +660,7 @@ unsafe fn get_ticks( component_id: ComponentId, entity: Entity, location: EntityLocation, -) -> Option<&UnsafeCell> { +) -> Option { let archetype = &world.archetypes[location.archetype_id]; let component_info = world.components.get_info_unchecked(component_id); match component_info.storage_type() { @@ -762,7 +756,7 @@ pub(crate) unsafe fn get_ticks_with_type( type_id: TypeId, entity: Entity, location: EntityLocation, -) -> Option<&UnsafeCell> { +) -> Option { let component_id = world.components.get_id(type_id)?; get_ticks(world, component_id, entity, location) } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 5f2dbd2d0110a..d72f4c4c9b4ac 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -11,9 +11,7 @@ use crate::{ archetype::{ArchetypeComponentId, ArchetypeId, Archetypes}, bundle::{Bundle, BundleInserter, BundleSpawner, Bundles}, change_detection::{MutUntyped, Ticks}, - component::{ - Component, ComponentDescriptor, ComponentId, ComponentInfo, Tick, Components, - }, + component::{Component, ComponentDescriptor, ComponentId, ComponentInfo, Components, Tick}, entity::{AllocAtWithoutReplacement, Entities, Entity}, query::{QueryState, ReadOnlyWorldQuery, WorldQuery}, storage::{ResourceData, SparseSet, Storages}, From 1c8d2426882dd58f1ca82915c513c1853d4b94d4 Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 11 Nov 2022 01:33:06 -0800 Subject: [PATCH 06/17] Fix CI --- crates/bevy_ecs/src/change_detection.rs | 45 +++++++++++++------------ crates/bevy_ecs/src/component.rs | 2 +- crates/bevy_ecs/src/storage/table.rs | 8 ++--- crates/bevy_ecs/src/world/mod.rs | 19 ++++++----- 4 files changed, 38 insertions(+), 36 deletions(-) diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index 27dd17c8528e4..924d8cb2089d3 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -428,10 +428,8 @@ mod tests { use crate::{ self as bevy_ecs, - change_detection::{ - ComponentTicks, Mut, NonSendMut, ResMut, Ticks, CHECK_TICK_THRESHOLD, MAX_CHANGE_AGE, - }, - component::Component, + change_detection::{Mut, NonSendMut, ResMut, Ticks, CHECK_TICK_THRESHOLD, MAX_CHANGE_AGE}, + component::{Component, ComponentTicks, Tick}, query::ChangeTrackers, system::{IntoSystem, Query, System}, world::World, @@ -513,8 +511,8 @@ mod tests { let mut query = world.query::>(); for tracker in query.iter(&world) { - let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added); - let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed); + let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added.tick); + let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed.tick); assert!(ticks_since_insert > MAX_CHANGE_AGE); assert!(ticks_since_change > MAX_CHANGE_AGE); } @@ -523,8 +521,8 @@ mod tests { world.check_change_ticks(); for tracker in query.iter(&world) { - let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added); - let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed); + let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added.tick); + let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed.tick); assert!(ticks_since_insert == MAX_CHANGE_AGE); assert!(ticks_since_change == MAX_CHANGE_AGE); } @@ -533,11 +531,12 @@ mod tests { #[test] fn mut_from_res_mut() { let mut component_ticks = ComponentTicks { - added: 1, - changed: 2, + added: Tick::new(1), + changed: Tick::new(2), }; let ticks = Ticks { - component_ticks: &mut component_ticks, + added: &mut component_ticks.added, + changed: &mut component_ticks.changed, last_change_tick: 3, change_tick: 4, }; @@ -548,8 +547,8 @@ mod tests { }; let into_mut: Mut = res_mut.into(); - assert_eq!(1, into_mut.ticks.component_ticks.added); - assert_eq!(2, into_mut.ticks.component_ticks.changed); + assert_eq!(1, into_mut.ticks.added.tick); + assert_eq!(2, into_mut.ticks.changed.tick); assert_eq!(3, into_mut.ticks.last_change_tick); assert_eq!(4, into_mut.ticks.change_tick); } @@ -557,11 +556,12 @@ mod tests { #[test] fn mut_from_non_send_mut() { let mut component_ticks = ComponentTicks { - added: 1, - changed: 2, + added: Tick::new(1), + changed: Tick::new(2), }; let ticks = Ticks { - component_ticks: &mut component_ticks, + added: &mut component_ticks.added, + changed: &mut component_ticks.changed, last_change_tick: 3, change_tick: 4, }; @@ -572,8 +572,8 @@ mod tests { }; let into_mut: Mut = non_send_mut.into(); - assert_eq!(1, into_mut.ticks.component_ticks.added); - assert_eq!(2, into_mut.ticks.component_ticks.changed); + assert_eq!(1, into_mut.ticks.added.tick); + assert_eq!(2, into_mut.ticks.changed.tick); assert_eq!(3, into_mut.ticks.last_change_tick); assert_eq!(4, into_mut.ticks.change_tick); } @@ -583,13 +583,14 @@ mod tests { use super::*; struct Outer(i64); + let (last_change_tick, change_tick) = (2, 3); let mut component_ticks = ComponentTicks { - added: 1, - changed: 2, + added: Tick::new(1), + changed: Tick::new(2), }; - let (last_change_tick, change_tick) = (2, 3); let ticks = Ticks { - component_ticks: &mut component_ticks, + added: &mut component_ticks.added, + changed: &mut component_ticks.changed, last_change_tick, change_tick, }; diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index e7b7c5351009b..3515712790ca5 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -582,7 +582,7 @@ impl ComponentTicks { #[inline] /// Returns `true` if the component was added after the system last ran. pub fn is_added(&self, last_change_tick: u32, change_tick: u32) -> bool { - self.changed.is_changed(last_change_tick, change_tick) + self.added.is_changed(last_change_tick, change_tick) } #[inline] diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 078fa8020c850..a391ce924b4d7 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -130,7 +130,7 @@ impl Column { // SAFETY: The row was length checked before this. let data = unsafe { self.data.swap_remove_and_forget_unchecked(row) }; let added = self.added_ticks.swap_remove(row).into_inner(); - let changed = self.added_ticks.swap_remove(row).into_inner(); + let changed = self.changed_ticks.swap_remove(row).into_inner(); (data, ComponentTicks { added, changed }) }) } @@ -145,7 +145,7 @@ impl Column { ) -> (OwningPtr<'_>, ComponentTicks) { let data = self.data.swap_remove_and_forget_unchecked(row); let added = self.added_ticks.swap_remove(row).into_inner(); - let changed = self.added_ticks.swap_remove(row).into_inner(); + let changed = self.changed_ticks.swap_remove(row).into_inner(); (data, ComponentTicks { added, changed }) } @@ -269,10 +269,10 @@ impl Column { #[inline] pub fn get_ticks(&self, row: usize) -> Option { if row < self.data.len() { - None - } else { // SAFETY: The size of the column has already been checked. Some(unsafe { self.get_ticks_unchecked(row) }) + } else { + None } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d72f4c4c9b4ac..da0dd34f3a091 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1457,15 +1457,16 @@ impl World { let (ptr, added, changed) = self.get_resource_with_ticks(component_id)?; - // SAFE: This function has exclusive access to the world so nothing aliases `ticks`. - let ticks = Ticks { - // SAFETY: - // - index is in-bounds because the column is initialized and non-empty - // - no other reference to the ticks of the same row can exist at the same time - added: unsafe { added.deref_mut() }, - changed: unsafe { changed.deref_mut() }, - last_change_tick: self.last_change_tick(), - change_tick: self.read_change_tick(), + // SAFETY: This function has exclusive access to the world so nothing aliases `ticks`. + // - index is in-bounds because the column is initialized and non-empty + // - no other reference to the ticks of the same row can exist at the same time + let ticks = unsafe { + Ticks { + added: added.deref_mut(), + changed: changed.deref_mut(), + last_change_tick: self.last_change_tick(), + change_tick: self.read_change_tick(), + } }; Some(MutUntyped { From 62baac25b546c966be7fd2fd3724f17d31f7c840 Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 11 Nov 2022 01:45:40 -0800 Subject: [PATCH 07/17] Only fetch relevant ticks for filters --- crates/bevy_ecs/src/query/filter.rs | 53 ++++++++++++++--------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 91184e0b01b21..03dfe1279f9b1 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -1,9 +1,9 @@ use crate::{ archetype::{Archetype, ArchetypeComponentId}, - component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType, Tick}, + component::{Component, ComponentId, ComponentStorage, StorageType, Tick}, entity::Entity, query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery}, - storage::{ComponentSparseSet, Table}, + storage::{Column, ComponentSparseSet, Table}, world::World, }; use bevy_ecs_macros::all_tuples; @@ -405,7 +405,8 @@ macro_rules! impl_tick_filter { $name: ident, $(#[$fetch_meta:meta])* $fetch_name: ident, - $is_detected: expr + $get_slice: expr, + $get_sparse_set: expr ) => { $(#[$meta])* pub struct $name(PhantomData); @@ -413,10 +414,7 @@ macro_rules! impl_tick_filter { #[doc(hidden)] $(#[$fetch_meta])* pub struct $fetch_name<'w, T> { - table_ticks: Option<( - ThinSlicePtr<'w, UnsafeCell>, - ThinSlicePtr<'w, UnsafeCell> - )>, + table_ticks: Option< ThinSlicePtr<'w, UnsafeCell>>, marker: PhantomData, sparse_set: Option<&'w ComponentSparseSet>, last_change_tick: u32, @@ -480,10 +478,9 @@ macro_rules! impl_tick_filter { let column = table .get_column(component_id) .debug_checked_unwrap(); - fetch.table_ticks = Some(( - column.get_added_ticks_slice().into(), - column.get_changed_ticks_slice().into() - )); + fetch.table_ticks = Some( + $get_slice(&column).into(), + ); } #[inline] @@ -506,23 +503,21 @@ macro_rules! impl_tick_filter { ) -> Self::Item<'w> { match T::Storage::STORAGE_TYPE { StorageType::Table => { - let (added, changed) = fetch.table_ticks.debug_checked_unwrap(); - let ticks = ComponentTicks { - added: added.get(table_row).read(), - changed: changed.get(table_row).read() - }; - $is_detected(&ticks, fetch.last_change_tick, fetch.change_tick) + fetch + .table_ticks + .debug_checked_unwrap() + .get(table_row) + .deref() + .is_changed(fetch.last_change_tick, fetch.change_tick) } StorageType::SparseSet => { - $is_detected( - &fetch - .sparse_set - .debug_checked_unwrap() - .get_ticks(entity) - .debug_checked_unwrap(), - fetch.last_change_tick, - fetch.change_tick - ) + let sparse_set = &fetch + .sparse_set + .debug_checked_unwrap(); + $get_sparse_set(sparse_set, entity) + .debug_checked_unwrap() + .deref() + .is_changed(fetch.last_change_tick, fetch.change_tick) } } } @@ -599,7 +594,8 @@ impl_tick_filter!( /// ``` Added, AddedFetch, - ComponentTicks::is_added + Column::get_added_ticks_slice, + ComponentSparseSet::get_added_ticks ); impl_tick_filter!( @@ -636,7 +632,8 @@ impl_tick_filter!( /// ``` Changed, ChangedFetch, - ComponentTicks::is_changed + Column::get_changed_ticks_slice, + ComponentSparseSet::get_changed_ticks ); /// A marker trait to indicate that the filter works at an archetype level. From e380e0d85a93149b0a90d55974a1bd65857b0aac Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 11 Nov 2022 03:13:16 -0800 Subject: [PATCH 08/17] Slim down initialize --- crates/bevy_ecs/src/bundle.rs | 4 ++-- crates/bevy_ecs/src/storage/table.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 8dd5b5225d5c3..9ae38040a1f03 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -9,7 +9,7 @@ use crate::{ Archetype, ArchetypeId, Archetypes, BundleComponentStatus, ComponentStatus, SpawnBundleStatus, }, - component::{Component, ComponentId, ComponentTicks, Components, StorageType}, + component::{Component, ComponentId, Tick, Components, StorageType}, entity::{Entities, Entity, EntityLocation}, storage::{SparseSetIndex, SparseSets, Storages, Table}, }; @@ -397,7 +397,7 @@ impl BundleInfo { column.initialize( table_row, component_ptr, - ComponentTicks::new(change_tick), + Tick::new(change_tick), ); } ComponentStatus::Mutated => { diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index a391ce924b4d7..d69559cf8b289 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -66,12 +66,12 @@ impl Column { &mut self, row: usize, data: OwningPtr<'_>, - ticks: ComponentTicks, + tick: Tick, ) { debug_assert!(row < self.len()); self.data.initialize_unchecked(row, data); - *self.added_ticks.get_unchecked_mut(row).get_mut() = ticks.added; - *self.changed_ticks.get_unchecked_mut(row).get_mut() = ticks.changed; + *self.added_ticks.get_unchecked_mut(row).get_mut() = tick; + *self.changed_ticks.get_unchecked_mut(row).get_mut() = tick; } /// Writes component data to the column at given row. From 51d10f0931b1b25d7440ee2cd7be6a7b358be258 Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 11 Nov 2022 04:54:11 -0800 Subject: [PATCH 09/17] Formatting --- crates/bevy_ecs/src/bundle.rs | 8 ++------ crates/bevy_ecs/src/storage/table.rs | 7 +------ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 9ae38040a1f03..6a7a250c502ef 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -9,7 +9,7 @@ use crate::{ Archetype, ArchetypeId, Archetypes, BundleComponentStatus, ComponentStatus, SpawnBundleStatus, }, - component::{Component, ComponentId, Tick, Components, StorageType}, + component::{Component, ComponentId, Components, StorageType, Tick}, entity::{Entities, Entity, EntityLocation}, storage::{SparseSetIndex, SparseSets, Storages, Table}, }; @@ -394,11 +394,7 @@ impl BundleInfo { // SAFETY: bundle_component is a valid index for this bundle match bundle_component_status.get_status(bundle_component) { ComponentStatus::Added => { - column.initialize( - table_row, - component_ptr, - Tick::new(change_tick), - ); + column.initialize(table_row, component_ptr, Tick::new(change_tick)); } ComponentStatus::Mutated => { column.replace(table_row, component_ptr, change_tick); diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index d69559cf8b289..119d063bcaf47 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -62,12 +62,7 @@ impl Column { /// # Safety /// Assumes data has already been allocated for the given row. #[inline] - pub(crate) unsafe fn initialize( - &mut self, - row: usize, - data: OwningPtr<'_>, - tick: Tick, - ) { + pub(crate) unsafe fn initialize(&mut self, row: usize, data: OwningPtr<'_>, tick: Tick) { debug_assert!(row < self.len()); self.data.initialize_unchecked(row, data); *self.added_ticks.get_unchecked_mut(row).get_mut() = tick; From 956f2269d16763d2949c10629e0883c8862d5ac8 Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 11 Nov 2022 06:19:46 -0800 Subject: [PATCH 10/17] Fix CI --- crates/bevy_ecs/src/storage/table.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 119d063bcaf47..16a0a5b2b3c59 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -654,7 +654,7 @@ mod tests { use crate::ptr::OwningPtr; use crate::storage::Storages; use crate::{ - component::{ComponentTicks, Components}, + component::{Components, Tick}, entity::Entity, storage::Table, }; @@ -679,7 +679,7 @@ mod tests { table.get_column_mut(component_id).unwrap().initialize( row, value_ptr, - ComponentTicks::new(0), + Tick::new(0), ); }); }; From ea6065d9cdc4c6cbb4d6294bea54f58e9cb10d7d Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 12 Nov 2022 05:05:03 -0800 Subject: [PATCH 11/17] is_changed -> is_older_than --- crates/bevy_ecs/src/change_detection.rs | 8 ++++---- crates/bevy_ecs/src/component.rs | 9 +++++---- crates/bevy_ecs/src/query/filter.rs | 4 ++-- crates/bevy_ecs/src/system/system_param.rs | 4 ++-- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index 924d8cb2089d3..8a10edd5d10f4 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -96,14 +96,14 @@ macro_rules! change_detection_impl { fn is_added(&self) -> bool { self.ticks .added - .is_changed(self.ticks.last_change_tick, self.ticks.change_tick) + .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] fn is_changed(&self) -> bool { self.ticks .changed - .is_changed(self.ticks.last_change_tick, self.ticks.change_tick) + .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] @@ -383,14 +383,14 @@ impl<'a> DetectChanges for MutUntyped<'a> { fn is_added(&self) -> bool { self.ticks .added - .is_changed(self.ticks.last_change_tick, self.ticks.change_tick) + .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] fn is_changed(&self) -> bool { self.ticks .changed - .is_changed(self.ticks.last_change_tick, self.ticks.change_tick) + .is_older_than(self.ticks.last_change_tick, self.ticks.change_tick) } #[inline] diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 3515712790ca5..815272d928f9f 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -528,9 +528,10 @@ impl Tick { } #[inline] - pub fn is_changed(&self, last_change_tick: u32, change_tick: u32) -> bool { + /// Returns `true` if the tick is older than the system last's run. + pub fn is_older_than(&self, last_change_tick: u32, change_tick: u32) -> bool { // This works even with wraparound because the world tick (`change_tick`) is always "newer" than - // `last_change_tick` and `self.added`, and we scan periodically to clamp `ComponentTicks` values + // `last_change_tick` and `self.tick`, and we scan periodically to clamp `ComponentTicks` values // so they never get older than `u32::MAX` (the difference would overflow). // // The clamp here ensures determinism (since scans could differ between app runs). @@ -582,13 +583,13 @@ impl ComponentTicks { #[inline] /// Returns `true` if the component was added after the system last ran. pub fn is_added(&self, last_change_tick: u32, change_tick: u32) -> bool { - self.added.is_changed(last_change_tick, change_tick) + self.added.is_older_than(last_change_tick, change_tick) } #[inline] /// Returns `true` if the component was added or mutably dereferenced after the system last ran. pub fn is_changed(&self, last_change_tick: u32, change_tick: u32) -> bool { - self.changed.is_changed(last_change_tick, change_tick) + self.changed.is_older_than(last_change_tick, change_tick) } pub(crate) fn new(change_tick: u32) -> Self { diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 03dfe1279f9b1..3a48a34e6e340 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -508,7 +508,7 @@ macro_rules! impl_tick_filter { .debug_checked_unwrap() .get(table_row) .deref() - .is_changed(fetch.last_change_tick, fetch.change_tick) + .is_older_than(fetch.last_change_tick, fetch.change_tick) } StorageType::SparseSet => { let sparse_set = &fetch @@ -517,7 +517,7 @@ macro_rules! impl_tick_filter { $get_sparse_set(sparse_set, entity) .debug_checked_unwrap() .deref() - .is_changed(fetch.last_change_tick, fetch.change_tick) + .is_older_than(fetch.last_change_tick, fetch.change_tick) } } } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 7bf68ee625d34..2bf9a37b54770 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -307,13 +307,13 @@ impl<'w, T: Resource> Res<'w, T> { /// Returns `true` if the resource was added after the system last ran. pub fn is_added(&self) -> bool { self.added - .is_changed(self.last_change_tick, self.change_tick) + .is_older_than(self.last_change_tick, self.change_tick) } /// Returns `true` if the resource was added or mutably dereferenced after the system last ran. pub fn is_changed(&self) -> bool { self.changed - .is_changed(self.last_change_tick, self.change_tick) + .is_older_than(self.last_change_tick, self.change_tick) } pub fn into_inner(self) -> &'w T { From 04849240bdab1b986ec77bfa03d9ec1790749457 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 12 Nov 2022 05:09:58 -0800 Subject: [PATCH 12/17] Split ChangeTrackers table ticks --- crates/bevy_ecs/src/query/fetch.rs | 31 +++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 4b06ef2f76505..934d125f8edec 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -997,10 +997,8 @@ impl ChangeTrackers { #[doc(hidden)] pub struct ChangeTrackersFetch<'w, T> { // T::Storage = TableStorage - table_ticks: Option<( - ThinSlicePtr<'w, UnsafeCell>, // Added - ThinSlicePtr<'w, UnsafeCell>, // Changed - )>, + table_added: Option>>, + table_changed: Option>>, // T::Storage = SparseStorage sparse_set: Option<&'w ComponentSparseSet>, @@ -1036,7 +1034,8 @@ unsafe impl WorldQuery for ChangeTrackers { change_tick: u32, ) -> ChangeTrackersFetch<'w, T> { ChangeTrackersFetch { - table_ticks: None, + table_added: None, + table_changed: None, sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| { world .storages() @@ -1052,7 +1051,8 @@ unsafe impl WorldQuery for ChangeTrackers { unsafe fn clone_fetch<'w>(fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> { ChangeTrackersFetch { - table_ticks: fetch.table_ticks, + table_added: fetch.table_added, + table_changed: fetch.table_changed, sparse_set: fetch.sparse_set, marker: fetch.marker, last_change_tick: fetch.last_change_tick, @@ -1079,10 +1079,8 @@ unsafe impl WorldQuery for ChangeTrackers { table: &'w Table, ) { let column = table.get_column(id).debug_checked_unwrap(); - fetch.table_ticks = Some(( - column.get_added_ticks_slice().into(), - column.get_changed_ticks_slice().into(), - )); + fetch.table_added = Some(column.get_added_ticks_slice().into()); + fetch.table_changed = Some(column.get_changed_ticks_slice().into()); } #[inline(always)] @@ -1094,10 +1092,17 @@ unsafe impl WorldQuery for ChangeTrackers { match T::Storage::STORAGE_TYPE { StorageType::Table => ChangeTrackers { component_ticks: { - let (added, changed) = fetch.table_ticks.debug_checked_unwrap(); ComponentTicks { - added: added.get(table_row).read(), - changed: changed.get(table_row).read(), + added: fetch + .table_added + .debug_checked_unwrap() + .get(table_row) + .read(), + changed: fetch + .table_changed + .debug_checked_unwrap() + .get(table_row) + .read(), } }, marker: PhantomData, From 79056daec4347dc40418d8aa7042bc26a01166f0 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 12 Nov 2022 05:25:19 -0800 Subject: [PATCH 13/17] Tuples to TickCells --- crates/bevy_ecs/src/component.rs | 8 +++++ crates/bevy_ecs/src/query/fetch.rs | 6 ++-- crates/bevy_ecs/src/storage/resource.rs | 5 ++- crates/bevy_ecs/src/storage/sparse_set.rs | 10 +++--- crates/bevy_ecs/src/storage/table.rs | 10 +++--- crates/bevy_ecs/src/system/system_param.rs | 36 ++++++++++---------- crates/bevy_ecs/src/world/entity_ref.rs | 38 ++++++++++++---------- crates/bevy_ecs/src/world/mod.rs | 17 +++++----- 8 files changed, 71 insertions(+), 59 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 815272d928f9f..25de63d140e69 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -5,6 +5,7 @@ use crate::{ storage::{SparseSetIndex, Storages}, system::Resource, }; +use std::cell::UnsafeCell; pub use bevy_ecs_macros::Component; use bevy_ptr::OwningPtr; use std::{ @@ -572,6 +573,13 @@ impl Tick { } } +/// Wrapper around [`Tick`]s for a single component +#[derive(Copy, Clone, Debug)] +pub struct TickCells<'a> { + pub added: &'a UnsafeCell, + pub changed: &'a UnsafeCell, +} + /// Records when a component was added and when it was last mutably dereferenced (or added). #[derive(Copy, Clone, Debug)] pub struct ComponentTicks { diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 934d125f8edec..7bf6ae0518350 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -760,7 +760,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { } } StorageType::SparseSet => { - let (component, added, changed) = fetch + let (component, ticks) = fetch .sparse_set .debug_checked_unwrap() .get_with_ticks(entity) @@ -768,8 +768,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { Mut { value: component.assert_unique().deref_mut(), ticks: Ticks { - added: added.deref_mut(), - changed: changed.deref_mut(), + added: ticks.added.deref_mut(), + changed: ticks.changed.deref_mut(), change_tick: fetch.change_tick, last_change_tick: fetch.last_change_tick, }, diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index ce10dba1cc9ce..3f9c4ad09a8ca 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -1,8 +1,7 @@ use crate::archetype::ArchetypeComponentId; -use crate::component::{ComponentId, ComponentTicks, Components, Tick}; +use crate::component::{ComponentId, ComponentTicks, Components, TickCells}; use crate::storage::{Column, SparseSet}; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; -use std::cell::UnsafeCell; /// The type-erased backing storage and metadata for a single resource within a [`World`]. /// @@ -38,7 +37,7 @@ impl ResourceData { } #[inline] - pub(crate) fn get_with_ticks(&self) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { + pub(crate) fn get_with_ticks(&self) -> Option<(Ptr<'_>, TickCells<'_>)> { self.column.get(0) } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 1ea120fa11fc6..b2cebc54a70d1 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -1,5 +1,5 @@ use crate::{ - component::{ComponentId, ComponentInfo, ComponentTicks, Tick}, + component::{ComponentId, ComponentInfo, ComponentTicks, Tick, TickCells}, entity::Entity, storage::Column, }; @@ -168,7 +168,7 @@ impl ComponentSparseSet { pub fn get_with_ticks( &self, entity: Entity, - ) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { + ) -> Option<(Ptr<'_>, TickCells<'_>)> { let dense_index = *self.sparse.get(entity.index())? as usize; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index]); @@ -176,8 +176,10 @@ impl ComponentSparseSet { unsafe { Some(( self.dense.get_data_unchecked(dense_index), - self.dense.get_added_ticks_unchecked(dense_index), - self.dense.get_changed_ticks_unchecked(dense_index), + TickCells { + added: self.dense.get_added_ticks_unchecked(dense_index), + changed: self.dense.get_changed_ticks_unchecked(dense_index), + } )) } } diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 16a0a5b2b3c59..ade22b06a32e3 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -1,5 +1,5 @@ use crate::{ - component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick}, + component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick, TickCells}, entity::Entity, query::DebugCheckedUnwrap, storage::{blob_vec::BlobVec, SparseSet}, @@ -206,15 +206,17 @@ impl Column { } #[inline] - pub fn get(&self, row: usize) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { + pub fn get(&self, row: usize) -> Option<(Ptr<'_>, TickCells<'_>)> { (row < self.data.len()) // SAFETY: The row is length checked before fetching the pointer. This is being // accessed through a read-only reference to the column. .then(|| unsafe { ( self.data.get_unchecked(row), - self.added_ticks.get_unchecked(row), - self.changed_ticks.get_unchecked(row), + TickCells { + added: self.added_ticks.get_unchecked(row), + changed: self.changed_ticks.get_unchecked(row), + } ) }) } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 2bf9a37b54770..184f3cbf80c80 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -408,7 +408,7 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResState { world: &'w World, change_tick: u32, ) -> Self::Item { - let (ptr, added, changed) = world + let (ptr, ticks) = world .get_resource_with_ticks(state.component_id) .unwrap_or_else(|| { panic!( @@ -419,8 +419,8 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResState { }); Res { value: ptr.deref(), - added: added.deref(), - changed: changed.deref(), + added: ticks.added.deref(), + changed: ticks.changed.deref(), last_change_tick: system_meta.last_change_tick, change_tick, } @@ -459,10 +459,10 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for OptionResState { ) -> Self::Item { world .get_resource_with_ticks(state.0.component_id) - .map(|(ptr, added, changed)| Res { + .map(|(ptr, ticks)| Res { value: ptr.deref(), - added: added.deref(), - changed: changed.deref(), + added: ticks.added.deref(), + changed: ticks.changed.deref(), last_change_tick: system_meta.last_change_tick, change_tick, }) @@ -1007,7 +1007,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendState { change_tick: u32, ) -> Self::Item { world.validate_non_send_access::(); - let (ptr, added, changed) = world + let (ptr, ticks) = world .get_resource_with_ticks(state.component_id) .unwrap_or_else(|| { panic!( @@ -1020,8 +1020,8 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendState { NonSend { value: ptr.deref(), ticks: ComponentTicks { - added: added.read(), - changed: changed.read(), + added: ticks.added.read(), + changed: ticks.changed.read(), }, last_change_tick: system_meta.last_change_tick, change_tick, @@ -1062,11 +1062,11 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for OptionNonSendState { world.validate_non_send_access::(); world .get_resource_with_ticks(state.0.component_id) - .map(|(ptr, added, changed)| NonSend { + .map(|(ptr, ticks)| NonSend { value: ptr.deref(), ticks: ComponentTicks { - added: added.read(), - changed: changed.read(), + added: ticks.added.read(), + changed: ticks.changed.read(), }, last_change_tick: system_meta.last_change_tick, change_tick, @@ -1128,7 +1128,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendMutState { change_tick: u32, ) -> Self::Item { world.validate_non_send_access::(); - let (ptr, added, changed) = world + let (ptr, ticks) = world .get_resource_with_ticks(state.component_id) .unwrap_or_else(|| { panic!( @@ -1140,8 +1140,8 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendMutState { NonSendMut { value: ptr.assert_unique().deref_mut(), ticks: Ticks { - added: added.deref_mut(), - changed: changed.deref_mut(), + added: ticks.added.deref_mut(), + changed: ticks.changed.deref_mut(), last_change_tick: system_meta.last_change_tick, change_tick, }, @@ -1179,11 +1179,11 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for OptionNonSendMutState { world.validate_non_send_access::(); world .get_resource_with_ticks(state.0.component_id) - .map(|(ptr, added, changed)| NonSendMut { + .map(|(ptr, ticks)| NonSendMut { value: ptr.assert_unique().deref_mut(), ticks: Ticks { - added: added.deref_mut(), - changed: changed.deref_mut(), + added: ticks.added.deref_mut(), + changed: ticks.changed.deref_mut(), last_change_tick: system_meta.last_change_tick, change_tick, }, diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 6e11b8f566169..b46db9a56a195 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2,14 +2,14 @@ use crate::{ archetype::{Archetype, ArchetypeId, Archetypes}, bundle::{Bundle, BundleInfo}, change_detection::{MutUntyped, Ticks}, - component::{Component, ComponentId, ComponentTicks, Components, StorageType, Tick}, + component::{Component, ComponentId, ComponentTicks, Components, StorageType, TickCells}, entity::{Entities, Entity, EntityLocation}, storage::{SparseSet, Storages}, world::{Mut, World}, }; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; use bevy_utils::tracing::debug; -use std::{any::TypeId, cell::UnsafeCell}; +use std::any::TypeId; /// A read-only reference to a particular [`Entity`] and all of its components #[derive(Copy, Clone)] @@ -99,11 +99,11 @@ impl<'w> EntityRef<'w> { change_tick: u32, ) -> Option> { get_component_and_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) - .map(|(value, added, changed)| Mut { + .map(|(value, ticks)| Mut { value: value.assert_unique().deref_mut::(), ticks: Ticks { - added: added.deref_mut(), - changed: changed.deref_mut(), + added: ticks.added.deref_mut(), + changed: ticks.changed.deref_mut(), last_change_tick, change_tick, }, @@ -224,11 +224,11 @@ impl<'w> EntityMut<'w> { #[inline] pub unsafe fn get_unchecked_mut(&self) -> Option> { get_component_and_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) - .map(|(value, added, changed)| Mut { + .map(|(value, ticks)| Mut { value: value.assert_unique().deref_mut::(), ticks: Ticks { - added: added.deref_mut(), - changed: changed.deref_mut(), + added: ticks.added.deref_mut(), + changed: ticks.changed.deref_mut(), last_change_tick: self.world.last_change_tick(), change_tick: self.world.read_change_tick(), }, @@ -631,7 +631,7 @@ unsafe fn get_component_and_ticks( component_id: ComponentId, entity: Entity, location: EntityLocation, -) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { +) -> Option<(Ptr<'_>, TickCells<'_>)> { let archetype = &world.archetypes[location.archetype_id]; let component_info = world.components.get_info_unchecked(component_id); match component_info.storage_type() { @@ -642,8 +642,10 @@ unsafe fn get_component_and_ticks( // SAFETY: archetypes only store valid table_rows and the stored component type is T Some(( components.get_data_unchecked(table_row), - components.get_added_ticks_unchecked(table_row), - components.get_changed_ticks_unchecked(table_row), + TickCells { + added: components.get_added_ticks_unchecked(table_row), + changed: components.get_changed_ticks_unchecked(table_row), + } )) } StorageType::SparseSet => world @@ -744,7 +746,7 @@ pub(crate) unsafe fn get_component_and_ticks_with_type( type_id: TypeId, entity: Entity, location: EntityLocation, -) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { +) -> Option<(Ptr<'_>, TickCells<'_>)> { let component_id = world.components.get_id(type_id)?; get_component_and_ticks(world, component_id, entity, location) } @@ -906,11 +908,11 @@ pub(crate) unsafe fn get_mut( let change_tick = world.change_tick(); let last_change_tick = world.last_change_tick(); get_component_and_ticks_with_type(world, TypeId::of::(), entity, location).map( - |(value, added, changed)| Mut { + |(value, ticks)| Mut { value: value.assert_unique().deref_mut::(), ticks: Ticks { - added: added.deref_mut(), - changed: changed.deref_mut(), + added: ticks.added.deref_mut(), + changed: ticks.changed.deref_mut(), last_change_tick, change_tick, }, @@ -927,12 +929,12 @@ pub(crate) unsafe fn get_mut_by_id( component_id: ComponentId, ) -> Option { // SAFETY: world access is unique, entity location and component_id required to be valid - get_component_and_ticks(world, component_id, entity, location).map(|(value, added, changed)| { + get_component_and_ticks(world, component_id, entity, location).map(|(value, ticks)| { MutUntyped { value: value.assert_unique(), ticks: Ticks { - added: added.deref_mut(), - changed: changed.deref_mut(), + added: ticks.added.deref_mut(), + changed: ticks.changed.deref_mut(), last_change_tick: world.last_change_tick(), change_tick: world.read_change_tick(), }, diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index da0dd34f3a091..ac4642b70c837 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -11,7 +11,7 @@ use crate::{ archetype::{ArchetypeComponentId, ArchetypeId, Archetypes}, bundle::{Bundle, BundleInserter, BundleSpawner, Bundles}, change_detection::{MutUntyped, Ticks}, - component::{Component, ComponentDescriptor, ComponentId, ComponentInfo, Components, Tick}, + component::{Component, ComponentDescriptor, ComponentId, ComponentInfo, Components, TickCells}, entity::{AllocAtWithoutReplacement, Entities, Entity}, query::{QueryState, ReadOnlyWorldQuery, WorldQuery}, storage::{ResourceData, SparseSet, Storages}, @@ -21,7 +21,6 @@ use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; use bevy_utils::tracing::warn; use std::{ any::TypeId, - cell::UnsafeCell, fmt, sync::atomic::{AtomicU32, Ordering}, }; @@ -999,7 +998,7 @@ impl World { pub(crate) fn get_resource_with_ticks( &self, component_id: ComponentId, - ) -> Option<(Ptr<'_>, &UnsafeCell, &UnsafeCell)> { + ) -> Option<(Ptr<'_>, TickCells<'_>)> { self.storages.resources.get(component_id)?.get_with_ticks() } @@ -1269,12 +1268,12 @@ impl World { &self, component_id: ComponentId, ) -> Option> { - let (ptr, added, changed) = self.get_resource_with_ticks(component_id)?; + let (ptr, ticks) = self.get_resource_with_ticks(component_id)?; Some(Mut { value: ptr.assert_unique().deref_mut(), ticks: Ticks { - added: added.deref_mut(), - changed: changed.deref_mut(), + added: ticks.added.deref_mut(), + changed: ticks.changed.deref_mut(), last_change_tick: self.last_change_tick(), change_tick: self.read_change_tick(), }, @@ -1455,15 +1454,15 @@ impl World { self.validate_non_send_access_untyped(info.name()); } - let (ptr, added, changed) = self.get_resource_with_ticks(component_id)?; + let (ptr, ticks) = self.get_resource_with_ticks(component_id)?; // SAFETY: This function has exclusive access to the world so nothing aliases `ticks`. // - index is in-bounds because the column is initialized and non-empty // - no other reference to the ticks of the same row can exist at the same time let ticks = unsafe { Ticks { - added: added.deref_mut(), - changed: changed.deref_mut(), + added: ticks.added.deref_mut(), + changed: ticks.changed.deref_mut(), last_change_tick: self.last_change_tick(), change_tick: self.read_change_tick(), } From e82ba14fc8ab38b82617dc3eefd8ebe46f61e044 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 12 Nov 2022 05:38:15 -0800 Subject: [PATCH 14/17] Add and use Ticks::from_tick_cells --- crates/bevy_ecs/src/change_detection.rs | 25 +++++++++++++- crates/bevy_ecs/src/component.rs | 2 +- crates/bevy_ecs/src/query/fetch.rs | 7 +--- crates/bevy_ecs/src/storage/sparse_set.rs | 7 ++-- crates/bevy_ecs/src/storage/table.rs | 2 +- crates/bevy_ecs/src/system/system_param.rs | 14 ++------ crates/bevy_ecs/src/world/entity_ref.rs | 40 ++++++++-------------- crates/bevy_ecs/src/world/mod.rs | 20 ++++------- 8 files changed, 51 insertions(+), 66 deletions(-) diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index 8a10edd5d10f4..0831f316ef4f8 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -1,6 +1,11 @@ //! Types that detect when their internal data mutate. -use crate::{component::Tick, ptr::PtrMut, system::Resource}; +use crate::{ + component::{Tick, TickCells}, + ptr::PtrMut, + system::Resource, +}; +use bevy_ptr::UnsafeCellDeref; use std::ops::{Deref, DerefMut}; /// The (arbitrarily chosen) minimum number of world tick increments between `check_tick` scans. @@ -230,6 +235,24 @@ pub(crate) struct Ticks<'a> { pub(crate) change_tick: u32, } +impl<'a> Ticks<'a> { + /// # Safety + /// This should never alias the underlying ticks. All access must be unique. + #[inline] + pub(crate) unsafe fn from_tick_cells( + cells: TickCells<'a>, + last_change_tick: u32, + change_tick: u32, + ) -> Self { + Self { + added: cells.added.deref_mut(), + changed: cells.changed.deref_mut(), + last_change_tick, + change_tick, + } + } +} + /// Unique mutable borrow of a [`Resource`]. /// /// See the [`Resource`] documentation for usage. diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 25de63d140e69..1e6cc8012f5ee 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -5,9 +5,9 @@ use crate::{ storage::{SparseSetIndex, Storages}, system::Resource, }; -use std::cell::UnsafeCell; pub use bevy_ecs_macros::Component; use bevy_ptr::OwningPtr; +use std::cell::UnsafeCell; use std::{ alloc::Layout, any::{Any, TypeId}, diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 7bf6ae0518350..9e62594e3c44e 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -767,12 +767,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { .debug_checked_unwrap(); Mut { value: component.assert_unique().deref_mut(), - ticks: Ticks { - added: ticks.added.deref_mut(), - changed: ticks.changed.deref_mut(), - change_tick: fetch.change_tick, - last_change_tick: fetch.last_change_tick, - }, + ticks: Ticks::from_tick_cells(ticks, fetch.change_tick, fetch.last_change_tick), } } } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index b2cebc54a70d1..688833f1b5536 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -165,10 +165,7 @@ impl ComponentSparseSet { } #[inline] - pub fn get_with_ticks( - &self, - entity: Entity, - ) -> Option<(Ptr<'_>, TickCells<'_>)> { + pub fn get_with_ticks(&self, entity: Entity) -> Option<(Ptr<'_>, TickCells<'_>)> { let dense_index = *self.sparse.get(entity.index())? as usize; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index]); @@ -179,7 +176,7 @@ impl ComponentSparseSet { TickCells { added: self.dense.get_added_ticks_unchecked(dense_index), changed: self.dense.get_changed_ticks_unchecked(dense_index), - } + }, )) } } diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index ade22b06a32e3..5d17d7e6d3dd0 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -216,7 +216,7 @@ impl Column { TickCells { added: self.added_ticks.get_unchecked(row), changed: self.changed_ticks.get_unchecked(row), - } + }, ) }) } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 184f3cbf80c80..3096202c7a226 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1139,12 +1139,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendMutState { }); NonSendMut { value: ptr.assert_unique().deref_mut(), - ticks: Ticks { - added: ticks.added.deref_mut(), - changed: ticks.changed.deref_mut(), - last_change_tick: system_meta.last_change_tick, - change_tick, - }, + ticks: Ticks::from_tick_cells(ticks, system_meta.last_change_tick, change_tick), } } } @@ -1181,12 +1176,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for OptionNonSendMutState { .get_resource_with_ticks(state.0.component_id) .map(|(ptr, ticks)| NonSendMut { value: ptr.assert_unique().deref_mut(), - ticks: Ticks { - added: ticks.added.deref_mut(), - changed: ticks.changed.deref_mut(), - last_change_tick: system_meta.last_change_tick, - change_tick, - }, + ticks: Ticks::from_tick_cells(ticks, system_meta.last_change_tick, change_tick), }) } } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index b46db9a56a195..c67373840dfe5 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -7,7 +7,7 @@ use crate::{ storage::{SparseSet, Storages}, world::{Mut, World}, }; -use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; +use bevy_ptr::{OwningPtr, Ptr}; use bevy_utils::tracing::debug; use std::any::TypeId; @@ -101,12 +101,7 @@ impl<'w> EntityRef<'w> { get_component_and_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) .map(|(value, ticks)| Mut { value: value.assert_unique().deref_mut::(), - ticks: Ticks { - added: ticks.added.deref_mut(), - changed: ticks.changed.deref_mut(), - last_change_tick, - change_tick, - }, + ticks: Ticks::from_tick_cells(ticks, last_change_tick, change_tick), }) } } @@ -226,12 +221,11 @@ impl<'w> EntityMut<'w> { get_component_and_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) .map(|(value, ticks)| Mut { value: value.assert_unique().deref_mut::(), - ticks: Ticks { - added: ticks.added.deref_mut(), - changed: ticks.changed.deref_mut(), - last_change_tick: self.world.last_change_tick(), - change_tick: self.world.read_change_tick(), - }, + ticks: Ticks::from_tick_cells( + ticks, + self.world.last_change_tick(), + self.world.read_change_tick(), + ), }) } @@ -645,7 +639,7 @@ unsafe fn get_component_and_ticks( TickCells { added: components.get_added_ticks_unchecked(table_row), changed: components.get_changed_ticks_unchecked(table_row), - } + }, )) } StorageType::SparseSet => world @@ -910,12 +904,7 @@ pub(crate) unsafe fn get_mut( get_component_and_ticks_with_type(world, TypeId::of::(), entity, location).map( |(value, ticks)| Mut { value: value.assert_unique().deref_mut::(), - ticks: Ticks { - added: ticks.added.deref_mut(), - changed: ticks.changed.deref_mut(), - last_change_tick, - change_tick, - }, + ticks: Ticks::from_tick_cells(ticks, last_change_tick, change_tick), }, ) } @@ -932,12 +921,11 @@ pub(crate) unsafe fn get_mut_by_id( get_component_and_ticks(world, component_id, entity, location).map(|(value, ticks)| { MutUntyped { value: value.assert_unique(), - ticks: Ticks { - added: ticks.added.deref_mut(), - changed: ticks.changed.deref_mut(), - last_change_tick: world.last_change_tick(), - change_tick: world.read_change_tick(), - }, + ticks: Ticks::from_tick_cells( + ticks, + world.last_change_tick(), + world.read_change_tick(), + ), } }) } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index ac4642b70c837..d42ae7744e127 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -11,13 +11,15 @@ use crate::{ archetype::{ArchetypeComponentId, ArchetypeId, Archetypes}, bundle::{Bundle, BundleInserter, BundleSpawner, Bundles}, change_detection::{MutUntyped, Ticks}, - component::{Component, ComponentDescriptor, ComponentId, ComponentInfo, Components, TickCells}, + component::{ + Component, ComponentDescriptor, ComponentId, ComponentInfo, Components, TickCells, + }, entity::{AllocAtWithoutReplacement, Entities, Entity}, query::{QueryState, ReadOnlyWorldQuery, WorldQuery}, storage::{ResourceData, SparseSet, Storages}, system::Resource, }; -use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; +use bevy_ptr::{OwningPtr, Ptr}; use bevy_utils::tracing::warn; use std::{ any::TypeId, @@ -1271,12 +1273,7 @@ impl World { let (ptr, ticks) = self.get_resource_with_ticks(component_id)?; Some(Mut { value: ptr.assert_unique().deref_mut(), - ticks: Ticks { - added: ticks.added.deref_mut(), - changed: ticks.changed.deref_mut(), - last_change_tick: self.last_change_tick(), - change_tick: self.read_change_tick(), - }, + ticks: Ticks::from_tick_cells(ticks, self.last_change_tick(), self.read_change_tick()), }) } @@ -1460,12 +1457,7 @@ impl World { // - index is in-bounds because the column is initialized and non-empty // - no other reference to the ticks of the same row can exist at the same time let ticks = unsafe { - Ticks { - added: ticks.added.deref_mut(), - changed: ticks.changed.deref_mut(), - last_change_tick: self.last_change_tick(), - change_tick: self.read_change_tick(), - } + Ticks::from_tick_cells(ticks, self.last_change_tick(), self.read_change_tick()) }; Some(MutUntyped { From 7ec3d5e16052ca93c639bc0321dc41a990d27ad3 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 12 Nov 2022 05:44:38 -0800 Subject: [PATCH 15/17] Add TickCells::read --- crates/bevy_ecs/src/component.rs | 14 +++++++++++++- crates/bevy_ecs/src/system/system_param.rs | 10 ++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 1e6cc8012f5ee..836c800e1cac6 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -6,7 +6,7 @@ use crate::{ system::Resource, }; pub use bevy_ecs_macros::Component; -use bevy_ptr::OwningPtr; +use bevy_ptr::{OwningPtr, UnsafeCellDeref}; use std::cell::UnsafeCell; use std::{ alloc::Layout, @@ -580,6 +580,18 @@ pub struct TickCells<'a> { pub changed: &'a UnsafeCell, } +impl<'a> TickCells<'a> { + /// # Safety + /// All cells contained within must uphold the safety invariants of [`UnsafeCellDeref::read`]. + #[inline] + pub(crate) unsafe fn read(&self) -> ComponentTicks { + ComponentTicks { + added: self.added.read(), + changed: self.changed.read(), + } + } +} + /// Records when a component was added and when it was last mutably dereferenced (or added). #[derive(Copy, Clone, Debug)] pub struct ComponentTicks { diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 3096202c7a226..0aacdf4ffbe58 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1019,10 +1019,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for NonSendState { NonSend { value: ptr.deref(), - ticks: ComponentTicks { - added: ticks.added.read(), - changed: ticks.changed.read(), - }, + ticks: ticks.read(), last_change_tick: system_meta.last_change_tick, change_tick, } @@ -1064,10 +1061,7 @@ impl<'w, 's, T: 'static> SystemParamFetch<'w, 's> for OptionNonSendState { .get_resource_with_ticks(state.0.component_id) .map(|(ptr, ticks)| NonSend { value: ptr.deref(), - ticks: ComponentTicks { - added: ticks.added.read(), - changed: ticks.changed.read(), - }, + ticks: ticks.read(), last_change_tick: system_meta.last_change_tick, change_tick, }) From 8ec9a1a16c6b994d27df16c1437f349231bb7b2f Mon Sep 17 00:00:00 2001 From: James Liu Date: Sun, 13 Nov 2022 20:00:35 -0800 Subject: [PATCH 16/17] Document Ticks Co-authored-by: PROMETHIA-27 <42193387+PROMETHIA-27@users.noreply.github.com> --- crates/bevy_ecs/src/component.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 836c800e1cac6..442fc60ae03a7 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -518,6 +518,7 @@ impl Components { } } +/// Used to track changes in state between system runs, e.g. components being added or accessed mutably. #[derive(Copy, Clone, Debug)] pub struct Tick { pub(crate) tick: u32, From e63ca26cff254290c5054a096144791cc45ffe91 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 13 Nov 2022 21:58:40 -0800 Subject: [PATCH 17/17] Cleanup filters --- crates/bevy_ecs/src/query/filter.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 3a48a34e6e340..a84d74f8075e3 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -475,11 +475,12 @@ macro_rules! impl_tick_filter { &component_id: &ComponentId, table: &'w Table ) { - let column = table - .get_column(component_id) - .debug_checked_unwrap(); fetch.table_ticks = Some( - $get_slice(&column).into(), + $get_slice( + &table + .get_column(component_id) + .debug_checked_unwrap() + ).into(), ); }