From a5cbbd4482b9263a9b0ddf46e4d443d91d38e105 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Fri, 26 Jul 2024 21:48:49 -0400 Subject: [PATCH 1/7] feat: create `change_detection_source` feature Please bikeshed this name! --- crates/bevy_ecs/Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index 87f23a574b7cd..07bb1fc0e7ad4 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -11,11 +11,12 @@ categories = ["game-engines", "data-structures"] rust-version = "1.77.0" [features] +default = ["bevy_reflect"] trace = [] multi_threaded = ["bevy_tasks/multi_threaded", "arrayvec"] bevy_debug_stepping = [] -default = ["bevy_reflect"] serialize = ["dep:serde"] +change_detection_source = [] [dependencies] bevy_ptr = { path = "../bevy_ptr", version = "0.14.0-dev" } From 7219b9cdcbe1e4c48df899f91b1e97189439f316 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Fri, 26 Jul 2024 21:53:22 -0400 Subject: [PATCH 2/7] refactor: use imports instead of full qualified path --- crates/bevy_ecs/src/bundle.rs | 15 ++---- crates/bevy_ecs/src/change_detection.rs | 50 ++++++++++--------- crates/bevy_ecs/src/query/fetch.rs | 6 +-- crates/bevy_ecs/src/storage/resource.rs | 19 ++----- crates/bevy_ecs/src/storage/sparse_set.rs | 10 ++-- crates/bevy_ecs/src/storage/table.rs | 36 +++++-------- crates/bevy_ecs/src/system/system_param.rs | 5 +- crates/bevy_ecs/src/world/mod.rs | 21 ++------ crates/bevy_ecs/src/world/spawn_batch.rs | 6 +-- .../bevy_ecs/src/world/unsafe_world_cell.rs | 20 ++------ 10 files changed, 69 insertions(+), 119 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 28b43fc8c1b32..510243432f7b3 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -19,8 +19,7 @@ use crate::{ }; use bevy_ptr::{ConstNonNull, OwningPtr}; use bevy_utils::all_tuples; -use std::any::TypeId; -use std::ptr::NonNull; +use std::{any::TypeId, panic::Location, ptr::NonNull}; /// The `Bundle` trait enables insertion and removal of [`Component`]s from an entity. /// @@ -387,7 +386,7 @@ impl BundleInfo { table_row: TableRow, change_tick: Tick, bundle: T, - caller: core::panic::Location<'static>, + caller: Location<'static>, ) { // NOTE: get_components calls this closure on each component in "bundle order". // bundle_info.component_ids are also in "bundle order" @@ -648,7 +647,7 @@ impl<'w> BundleInserter<'w> { let add_bundle = self.add_bundle.as_ref(); let table = self.table.as_mut(); let archetype = self.archetype.as_mut(); - let caller = *core::panic::Location::caller(); + let caller = *Location::caller(); let (new_archetype, new_location) = match &mut self.result { InsertBundleResult::SameArchetype => { @@ -887,7 +886,7 @@ impl<'w> BundleSpawner<'w> { &mut self, entity: Entity, bundle: T, - caller: core::panic::Location<'static>, + caller: Location<'static>, ) -> EntityLocation { let table = self.table.as_mut(); let archetype = self.archetype.as_mut(); @@ -935,11 +934,7 @@ impl<'w> BundleSpawner<'w> { /// # Safety /// `T` must match this [`BundleInfo`]'s type #[inline] - pub unsafe fn spawn( - &mut self, - bundle: T, - caller: core::panic::Location<'static>, - ) -> Entity { + pub unsafe fn spawn(&mut self, bundle: T, caller: Location<'static>) -> Entity { let entity = self.entities().alloc(); // SAFETY: entity is allocated (but non-existent), `T` matches this BundleInfo's type unsafe { diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index 74d3e8608b795..22ef97cd5e5f8 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -8,6 +8,7 @@ use crate::{ use bevy_ptr::{Ptr, UnsafeCellDeref}; use std::mem; use std::ops::{Deref, DerefMut}; +use std::panic::Location; /// The (arbitrarily chosen) minimum number of world tick increments between `check_tick` scans. /// @@ -65,7 +66,7 @@ pub trait DetectChanges { fn last_changed(&self) -> Tick; /// The location that last caused this to change. - fn changed_by(&self) -> core::panic::Location<'static>; + fn changed_by(&self) -> Location<'static>; } /// Types that implement reliable change detection. @@ -286,7 +287,7 @@ macro_rules! change_detection_impl { } #[inline] - fn changed_by(&self) -> core::panic::Location<'static> { + fn changed_by(&self) -> Location<'static> { *self.caller } } @@ -318,14 +319,14 @@ macro_rules! change_detection_mut_impl { #[track_caller] fn set_changed(&mut self) { *self.ticks.changed = self.ticks.this_run; - *self.caller = *core::panic::Location::caller(); + *self.caller = *Location::caller(); } #[inline] #[track_caller] fn set_last_changed(&mut self, last_changed: Tick) { *self.ticks.changed = last_changed; - *self.caller = *core::panic::Location::caller(); + *self.caller = *Location::caller(); } #[inline] @@ -339,7 +340,7 @@ macro_rules! change_detection_mut_impl { #[track_caller] fn deref_mut(&mut self) -> &mut Self::Target { self.set_changed(); - *self.caller = *core::panic::Location::caller(); + *self.caller = *Location::caller(); self.value } } @@ -518,7 +519,7 @@ impl<'w> From> for Ticks<'w> { pub struct Res<'w, T: ?Sized + Resource> { pub(crate) value: &'w T, pub(crate) ticks: Ticks<'w>, - pub(crate) caller: &'w core::panic::Location<'static>, + pub(crate) caller: &'w Location<'static>, } impl<'w, T: Resource> Res<'w, T> { @@ -581,7 +582,7 @@ impl_debug!(Res<'w, T>, Resource); pub struct ResMut<'w, T: ?Sized + Resource> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, - pub(crate) caller: &'w mut core::panic::Location<'static>, + pub(crate) caller: &'w mut Location<'static>, } impl<'w, 'a, T: Resource> IntoIterator for &'a ResMut<'w, T> @@ -641,7 +642,7 @@ impl<'w, T: Resource> From> for Mut<'w, T> { pub struct NonSendMut<'w, T: ?Sized + 'static> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, - pub(crate) caller: &'w mut core::panic::Location<'static>, + pub(crate) caller: &'w mut Location<'static>, } change_detection_impl!(NonSendMut<'w, T>, T,); @@ -688,7 +689,7 @@ impl<'w, T: 'static> From> for Mut<'w, T> { pub struct Ref<'w, T: ?Sized> { pub(crate) value: &'w T, pub(crate) ticks: Ticks<'w>, - pub(crate) caller: &'w core::panic::Location<'static>, + pub(crate) caller: &'w Location<'static>, } impl<'w, T: ?Sized> Ref<'w, T> { @@ -726,7 +727,7 @@ impl<'w, T: ?Sized> Ref<'w, T> { changed: &'w Tick, last_run: Tick, this_run: Tick, - caller: &'w core::panic::Location<'static>, + caller: &'w Location<'static>, ) -> Ref<'w, T> { Ref { value, @@ -818,7 +819,7 @@ impl_debug!(Ref<'w, T>,); pub struct Mut<'w, T: ?Sized> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, - pub(crate) caller: &'w mut core::panic::Location<'static>, + pub(crate) caller: &'w mut Location<'static>, } impl<'w, T: ?Sized> Mut<'w, T> { @@ -843,7 +844,7 @@ impl<'w, T: ?Sized> Mut<'w, T> { last_changed: &'w mut Tick, last_run: Tick, this_run: Tick, - caller: &'w mut core::panic::Location<'static>, + caller: &'w mut Location<'static>, ) -> Self { Self { value, @@ -909,7 +910,7 @@ impl_debug!(Mut<'w, T>,); pub struct MutUntyped<'w> { pub(crate) value: PtrMut<'w>, pub(crate) ticks: TicksMut<'w>, - pub(crate) caller: &'w mut core::panic::Location<'static>, + pub(crate) caller: &'w mut Location<'static>, } impl<'w> MutUntyped<'w> { @@ -1024,7 +1025,7 @@ impl<'w> DetectChanges for MutUntyped<'w> { } #[inline] - fn changed_by(&self) -> core::panic::Location<'static> { + fn changed_by(&self) -> Location<'static> { *self.caller } } @@ -1036,14 +1037,14 @@ impl<'w> DetectChangesMut for MutUntyped<'w> { #[track_caller] fn set_changed(&mut self) { *self.ticks.changed = self.ticks.this_run; - *self.caller = *core::panic::Location::caller(); + *self.caller = *Location::caller(); } #[inline] #[track_caller] fn set_last_changed(&mut self, last_changed: Tick) { *self.ticks.changed = last_changed; - *self.caller = *core::panic::Location::caller(); + *self.caller = *Location::caller(); } #[inline] @@ -1075,7 +1076,10 @@ mod tests { use bevy_ecs_macros::Resource; use bevy_ptr::PtrMut; use bevy_reflect::{FromType, ReflectFromPtr}; - use std::ops::{Deref, DerefMut}; + use std::{ + ops::{Deref, DerefMut}, + panic::Location, + }; use crate::{ self as bevy_ecs, @@ -1206,7 +1210,7 @@ mod tests { this_run: Tick::new(4), }; let mut res = R {}; - let mut caller = *core::panic::Location::caller(); + let mut caller = *Location::caller(); let res_mut = ResMut { value: &mut res, ticks, @@ -1227,7 +1231,7 @@ mod tests { changed: Tick::new(3), }; let mut res = R {}; - let mut caller = *core::panic::Location::caller(); + let mut caller = *Location::caller(); let val = Mut::new( &mut res, @@ -1255,7 +1259,7 @@ mod tests { this_run: Tick::new(4), }; let mut res = R {}; - let mut caller = *core::panic::Location::caller(); + let mut caller = *Location::caller(); let non_send_mut = NonSendMut { value: &mut res, ticks, @@ -1288,7 +1292,7 @@ mod tests { }; let mut outer = Outer(0); - let mut caller = *core::panic::Location::caller(); + let mut caller = *Location::caller(); let ptr = Mut { value: &mut outer, ticks, @@ -1375,7 +1379,7 @@ mod tests { }; let mut value: i32 = 5; - let mut caller = *core::panic::Location::caller(); + let mut caller = *Location::caller(); let value = MutUntyped { value: PtrMut::from(&mut value), ticks, @@ -1410,7 +1414,7 @@ mod tests { this_run: Tick::new(4), }; let mut c = C {}; - let mut caller = *core::panic::Location::caller(); + let mut caller = *Location::caller(); let mut_typed = Mut { value: &mut c, ticks, diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 998644579ce5e..1d508ae670310 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -12,7 +12,7 @@ use crate::{ }; use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref}; use bevy_utils::all_tuples; -use std::{cell::UnsafeCell, marker::PhantomData}; +use std::{cell::UnsafeCell, marker::PhantomData, panic::Location}; /// Types that can be fetched from a [`World`] using a [`Query`]. /// @@ -1026,7 +1026,7 @@ pub struct RefFetch<'w, T> { ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, - ThinSlicePtr<'w, UnsafeCell>>, + ThinSlicePtr<'w, UnsafeCell>>, )>, // T::STORAGE_TYPE = StorageType::SparseSet sparse_set: Option<&'w ComponentSparseSet>, @@ -1215,7 +1215,7 @@ pub struct WriteFetch<'w, T> { ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, - ThinSlicePtr<'w, UnsafeCell>>, + ThinSlicePtr<'w, UnsafeCell>>, )>, // T::STORAGE_TYPE = StorageType::SparseSet sparse_set: Option<&'w ComponentSparseSet>, diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index de76f8c94df05..d22eb13954d17 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -3,6 +3,7 @@ use crate::change_detection::{MutUntyped, TicksMut}; use crate::component::{ComponentId, ComponentTicks, Components, Tick, TickCells}; use crate::storage::{blob_vec::BlobVec, SparseSet}; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; +use std::panic::Location; use std::{cell::UnsafeCell, mem::ManuallyDrop, thread::ThreadId}; /// The type-erased backing storage and metadata for a single resource within a [`World`]. @@ -17,7 +18,7 @@ pub struct ResourceData { type_name: String, id: ArchetypeComponentId, origin_thread_id: Option, - caller: UnsafeCell>, + caller: UnsafeCell>, } impl Drop for ResourceData { @@ -112,11 +113,7 @@ impl ResourceData { #[inline] pub(crate) fn get_with_ticks( &self, - ) -> Option<( - Ptr<'_>, - TickCells<'_>, - &UnsafeCell>, - )> { + ) -> Option<(Ptr<'_>, TickCells<'_>, &UnsafeCell>)> { self.is_present().then(|| { self.validate_access(); ( @@ -217,13 +214,7 @@ impl ResourceData { /// original thread it was inserted from. #[inline] #[must_use = "The returned pointer to the removed component should be used or dropped"] - pub(crate) fn remove( - &mut self, - ) -> Option<( - OwningPtr<'_>, - ComponentTicks, - core::panic::Location<'static>, - )> { + pub(crate) fn remove(&mut self) -> Option<(OwningPtr<'_>, ComponentTicks, Location<'static>)> { if !self.is_present() { return None; } @@ -354,7 +345,7 @@ impl Resources { type_name: String::from(component_info.name()), id: f(), origin_thread_id: None, - caller: UnsafeCell::new(*core::panic::Location::caller()) + caller: UnsafeCell::new(*Location::caller()) } }) } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index d9733043688b7..d3a438a481e28 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -5,7 +5,7 @@ use crate::{ }; use bevy_ptr::{OwningPtr, Ptr}; use nonmax::NonMaxUsize; -use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData}; +use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData, panic::Location}; type EntityIndex = u32; @@ -169,7 +169,7 @@ impl ComponentSparseSet { entity: Entity, value: OwningPtr<'_>, change_tick: Tick, - caller: core::panic::Location<'static>, + caller: Location<'static>, ) { if let Some(&dense_index) = self.sparse.get(entity.index()) { #[cfg(debug_assertions)] @@ -227,11 +227,7 @@ impl ComponentSparseSet { pub fn get_with_ticks( &self, entity: Entity, - ) -> Option<( - Ptr<'_>, - TickCells<'_>, - &UnsafeCell>, - )> { + ) -> Option<(Ptr<'_>, TickCells<'_>, &UnsafeCell>)> { let dense_index = *self.sparse.get(entity.index())?; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 686a322deb095..cf6da7db84a11 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -6,7 +6,7 @@ use crate::{ }; use bevy_ptr::{OwningPtr, Ptr, PtrMut, UnsafeCellDeref}; use bevy_utils::HashMap; -use std::alloc::Layout; +use std::{alloc::Layout, panic::Location}; use std::{ cell::UnsafeCell, ops::{Index, IndexMut}, @@ -152,7 +152,7 @@ pub struct Column { data: BlobVec, added_ticks: Vec>, changed_ticks: Vec>, - callers: Vec>>, + callers: Vec>>, } impl Column { @@ -186,7 +186,7 @@ impl Column { row: TableRow, data: OwningPtr<'_>, tick: Tick, - caller: core::panic::Location<'static>, + caller: Location<'static>, ) { debug_assert!(row.as_usize() < self.len()); self.data.initialize_unchecked(row.as_usize(), data); @@ -209,7 +209,7 @@ impl Column { row: TableRow, data: OwningPtr<'_>, change_tick: Tick, - caller: core::panic::Location<'static>, + caller: Location<'static>, ) { debug_assert!(row.as_usize() < self.len()); self.data.replace_unchecked(row.as_usize(), data); @@ -266,11 +266,7 @@ impl Column { pub(crate) unsafe fn swap_remove_and_forget_unchecked( &mut self, row: TableRow, - ) -> ( - OwningPtr<'_>, - ComponentTicks, - core::panic::Location<'static>, - ) { + ) -> (OwningPtr<'_>, ComponentTicks, Location<'static>) { let data = self.data.swap_remove_and_forget_unchecked(row.as_usize()); let added = self.added_ticks.swap_remove(row.as_usize()).into_inner(); let changed = self.changed_ticks.swap_remove(row.as_usize()).into_inner(); @@ -315,7 +311,7 @@ impl Column { &mut self, ptr: OwningPtr<'_>, ticks: ComponentTicks, - caller: core::panic::Location<'static>, + caller: Location<'static>, ) { self.data.push(ptr); self.added_ticks.push(UnsafeCell::new(ticks.added)); @@ -379,7 +375,7 @@ impl Column { /// Users of this API must ensure that accesses to each individual element /// adhere to the safety invariants of [`UnsafeCell`]. #[inline] - pub fn get_callers_slice(&self) -> &[UnsafeCell>] { + pub fn get_callers_slice(&self) -> &[UnsafeCell>] { &self.callers } @@ -390,11 +386,7 @@ impl Column { pub fn get( &self, row: TableRow, - ) -> Option<( - Ptr<'_>, - TickCells<'_>, - &UnsafeCell>, - )> { + ) -> Option<(Ptr<'_>, TickCells<'_>, &UnsafeCell>)> { (row.as_usize() < 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. @@ -523,10 +515,7 @@ impl Column { /// # Safety /// `row` must be within the range `[0, self.len())`. #[inline] - pub unsafe fn get_caller_unchecked( - &self, - row: TableRow, - ) -> &UnsafeCell> { + pub unsafe fn get_caller_unchecked(&self, row: TableRow) -> &UnsafeCell> { debug_assert!(row.as_usize() < self.callers.len()); self.callers.get_unchecked(row.as_usize()) } @@ -804,9 +793,7 @@ impl Table { column.data.set_len(self.entities.len()); column.added_ticks.push(UnsafeCell::new(Tick::new(0))); column.changed_ticks.push(UnsafeCell::new(Tick::new(0))); - column - .callers - .push(UnsafeCell::new(*core::panic::Location::caller())); + column.callers.push(UnsafeCell::new(*Location::caller())); } TableRow::from_usize(index) } @@ -991,6 +978,7 @@ mod tests { entity::Entity, storage::{TableBuilder, TableRow}, }; + use std::panic::Location; #[derive(Component)] struct W(T); @@ -1014,7 +1002,7 @@ mod tests { row, value_ptr, Tick::new(0), - *core::panic::Location::caller(), + *Location::caller(), ); }); }; diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 72ac4cb6959a3..790471e409ce3 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -22,6 +22,7 @@ use std::{ fmt::Debug, marker::PhantomData, ops::{Deref, DerefMut}, + panic::Location, }; /// A parameter that can be used in a [`System`](super::System). @@ -1044,7 +1045,7 @@ pub struct NonSend<'w, T: 'static> { ticks: ComponentTicks, last_run: Tick, this_run: Tick, - caller: &'w core::panic::Location<'static>, + caller: &'w Location<'static>, } // SAFETY: Only reads a single World non-send resource @@ -1071,7 +1072,7 @@ impl<'w, T: 'static> NonSend<'w, T> { } /// The location that last caused this to change. - pub fn changed_by(&self) -> core::panic::Location<'static> { + pub fn changed_by(&self) -> Location<'static> { *self.caller } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 7503be653aca5..933934b2691a4 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -41,6 +41,7 @@ use std::{ any::TypeId, fmt, mem::MaybeUninit, + panic::Location, sync::atomic::{AtomicU32, Ordering}, }; mod identifier; @@ -959,9 +960,7 @@ impl World { let entity_location = { let mut bundle_spawner = BundleSpawner::new::(self, change_tick); // SAFETY: bundle's type matches `bundle_info`, entity is allocated but non-existent - unsafe { - bundle_spawner.spawn_non_existent(entity, bundle, *core::panic::Location::caller()) - } + unsafe { bundle_spawner.spawn_non_existent(entity, bundle, *Location::caller()) } }; // SAFETY: entity and location are valid, as they were just created above @@ -1797,25 +1796,13 @@ impl World { AllocAtWithoutReplacement::DidNotExist => { if let SpawnOrInsert::Spawn(ref mut spawner) = spawn_or_insert { // SAFETY: `entity` is allocated (but non existent), bundle matches inserter - unsafe { - spawner.spawn_non_existent( - entity, - bundle, - *core::panic::Location::caller(), - ) - }; + unsafe { spawner.spawn_non_existent(entity, bundle, *Location::caller()) }; } else { // SAFETY: we initialized this bundle_id in `init_info` let mut spawner = unsafe { BundleSpawner::new_with_id(self, bundle_id, change_tick) }; // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter - unsafe { - spawner.spawn_non_existent( - entity, - bundle, - *core::panic::Location::caller(), - ) - }; + unsafe { spawner.spawn_non_existent(entity, bundle, *Location::caller()) }; spawn_or_insert = SpawnOrInsert::Spawn(spawner); } } diff --git a/crates/bevy_ecs/src/world/spawn_batch.rs b/crates/bevy_ecs/src/world/spawn_batch.rs index bb575f67a410a..105fd972ce867 100644 --- a/crates/bevy_ecs/src/world/spawn_batch.rs +++ b/crates/bevy_ecs/src/world/spawn_batch.rs @@ -3,7 +3,7 @@ use crate::{ entity::Entity, world::World, }; -use std::iter::FusedIterator; +use std::{iter::FusedIterator, panic::Location}; /// An iterator that spawns a series of entities and returns the [ID](Entity) of /// each spawned entity. @@ -16,7 +16,7 @@ where { inner: I, spawner: BundleSpawner<'w>, - caller: core::panic::Location<'static>, + caller: Location<'static>, } impl<'w, I> SpawnBatchIter<'w, I> @@ -43,7 +43,7 @@ where Self { inner: iter, spawner, - caller: *core::panic::Location::caller(), + caller: *Location::caller(), } } } diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index fad87f17be0b1..5334842f31a32 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -16,7 +16,7 @@ use crate::{ world::RawCommandQueue, }; use bevy_ptr::{Ptr, UnsafeCellDeref}; -use std::{any::TypeId, cell::UnsafeCell, fmt::Debug, marker::PhantomData, ptr}; +use std::{any::TypeId, cell::UnsafeCell, fmt::Debug, marker::PhantomData, panic::Location, ptr}; /// Variant of the [`World`] where resource and component accesses take `&self`, and the responsibility to avoid /// aliasing violations are given to the caller instead of being checked at compile-time by rust's unique XOR shared rule. @@ -538,11 +538,7 @@ impl<'w> UnsafeWorldCell<'w> { pub(crate) unsafe fn get_resource_with_ticks( self, component_id: ComponentId, - ) -> Option<( - Ptr<'w>, - TickCells<'w>, - &'w UnsafeCell>, - )> { + ) -> Option<(Ptr<'w>, TickCells<'w>, &'w UnsafeCell>)> { // SAFETY: // - caller ensures there is no `&mut World` // - caller ensures there are no mutable borrows of this resource @@ -566,11 +562,7 @@ impl<'w> UnsafeWorldCell<'w> { pub(crate) unsafe fn get_non_send_with_ticks( self, component_id: ComponentId, - ) -> Option<( - Ptr<'w>, - TickCells<'w>, - &'w UnsafeCell>, - )> { + ) -> Option<(Ptr<'w>, TickCells<'w>, &'w UnsafeCell>)> { // SAFETY: // - caller ensures there is no `&mut World` // - caller ensures there are no mutable borrows of this resource @@ -980,11 +972,7 @@ unsafe fn get_component_and_ticks( storage_type: StorageType, entity: Entity, location: EntityLocation, -) -> Option<( - Ptr<'_>, - TickCells<'_>, - &UnsafeCell>, -)> { +) -> Option<(Ptr<'_>, TickCells<'_>, &UnsafeCell>)> { match storage_type { StorageType::Table => { let components = world.fetch_table(location, component_id)?; From 60497cb8359fa86b7b67996f33bfec6f4de0d10e Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Fri, 26 Jul 2024 22:29:07 -0400 Subject: [PATCH 3/7] feat: store `Location` behind a `&'static` --- crates/bevy_ecs/src/bundle.rs | 12 ++-- crates/bevy_ecs/src/change_detection.rs | 60 +++++++++---------- crates/bevy_ecs/src/query/fetch.rs | 4 +- crates/bevy_ecs/src/storage/resource.rs | 14 +++-- crates/bevy_ecs/src/storage/sparse_set.rs | 8 ++- crates/bevy_ecs/src/storage/table.rs | 28 +++++---- crates/bevy_ecs/src/system/system_param.rs | 6 +- crates/bevy_ecs/src/world/mod.rs | 6 +- crates/bevy_ecs/src/world/spawn_batch.rs | 4 +- .../bevy_ecs/src/world/unsafe_world_cell.rs | 18 +++++- 10 files changed, 97 insertions(+), 63 deletions(-) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 510243432f7b3..2134218fcef55 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -386,7 +386,7 @@ impl BundleInfo { table_row: TableRow, change_tick: Tick, bundle: T, - caller: Location<'static>, + caller: &'static Location<'static>, ) { // NOTE: get_components calls this closure on each component in "bundle order". // bundle_info.component_ids are also in "bundle order" @@ -647,7 +647,7 @@ impl<'w> BundleInserter<'w> { let add_bundle = self.add_bundle.as_ref(); let table = self.table.as_mut(); let archetype = self.archetype.as_mut(); - let caller = *Location::caller(); + let caller = Location::caller(); let (new_archetype, new_location) = match &mut self.result { InsertBundleResult::SameArchetype => { @@ -886,7 +886,7 @@ impl<'w> BundleSpawner<'w> { &mut self, entity: Entity, bundle: T, - caller: Location<'static>, + caller: &'static Location<'static>, ) -> EntityLocation { let table = self.table.as_mut(); let archetype = self.archetype.as_mut(); @@ -934,7 +934,11 @@ impl<'w> BundleSpawner<'w> { /// # Safety /// `T` must match this [`BundleInfo`]'s type #[inline] - pub unsafe fn spawn(&mut self, bundle: T, caller: Location<'static>) -> Entity { + pub unsafe fn spawn( + &mut self, + bundle: T, + caller: &'static Location<'static>, + ) -> Entity { let entity = self.entities().alloc(); // SAFETY: entity is allocated (but non-existent), `T` matches this BundleInfo's type unsafe { diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index 22ef97cd5e5f8..d7f98c3b244e0 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -66,7 +66,7 @@ pub trait DetectChanges { fn last_changed(&self) -> Tick; /// The location that last caused this to change. - fn changed_by(&self) -> Location<'static>; + fn changed_by(&self) -> &'static Location<'static>; } /// Types that implement reliable change detection. @@ -287,8 +287,8 @@ macro_rules! change_detection_impl { } #[inline] - fn changed_by(&self) -> Location<'static> { - *self.caller + fn changed_by(&self) -> &'static Location<'static> { + self.caller } } @@ -319,14 +319,14 @@ macro_rules! change_detection_mut_impl { #[track_caller] fn set_changed(&mut self) { *self.ticks.changed = self.ticks.this_run; - *self.caller = *Location::caller(); + self.caller = Location::caller(); } #[inline] #[track_caller] fn set_last_changed(&mut self, last_changed: Tick) { *self.ticks.changed = last_changed; - *self.caller = *Location::caller(); + self.caller = Location::caller(); } #[inline] @@ -340,7 +340,7 @@ macro_rules! change_detection_mut_impl { #[track_caller] fn deref_mut(&mut self) -> &mut Self::Target { self.set_changed(); - *self.caller = *Location::caller(); + self.caller = Location::caller(); self.value } } @@ -519,7 +519,7 @@ impl<'w> From> for Ticks<'w> { pub struct Res<'w, T: ?Sized + Resource> { pub(crate) value: &'w T, pub(crate) ticks: Ticks<'w>, - pub(crate) caller: &'w Location<'static>, + pub(crate) caller: &'static Location<'static>, } impl<'w, T: Resource> Res<'w, T> { @@ -582,7 +582,7 @@ impl_debug!(Res<'w, T>, Resource); pub struct ResMut<'w, T: ?Sized + Resource> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, - pub(crate) caller: &'w mut Location<'static>, + pub(crate) caller: &'static Location<'static>, } impl<'w, 'a, T: Resource> IntoIterator for &'a ResMut<'w, T> @@ -642,7 +642,7 @@ impl<'w, T: Resource> From> for Mut<'w, T> { pub struct NonSendMut<'w, T: ?Sized + 'static> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, - pub(crate) caller: &'w mut Location<'static>, + pub(crate) caller: &'static Location<'static>, } change_detection_impl!(NonSendMut<'w, T>, T,); @@ -689,7 +689,7 @@ impl<'w, T: 'static> From> for Mut<'w, T> { pub struct Ref<'w, T: ?Sized> { pub(crate) value: &'w T, pub(crate) ticks: Ticks<'w>, - pub(crate) caller: &'w Location<'static>, + pub(crate) caller: &'static Location<'static>, } impl<'w, T: ?Sized> Ref<'w, T> { @@ -727,7 +727,7 @@ impl<'w, T: ?Sized> Ref<'w, T> { changed: &'w Tick, last_run: Tick, this_run: Tick, - caller: &'w Location<'static>, + caller: &'static Location<'static>, ) -> Ref<'w, T> { Ref { value, @@ -819,7 +819,7 @@ impl_debug!(Ref<'w, T>,); pub struct Mut<'w, T: ?Sized> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, - pub(crate) caller: &'w mut Location<'static>, + pub(crate) caller: &'static Location<'static>, } impl<'w, T: ?Sized> Mut<'w, T> { @@ -844,7 +844,7 @@ impl<'w, T: ?Sized> Mut<'w, T> { last_changed: &'w mut Tick, last_run: Tick, this_run: Tick, - caller: &'w mut Location<'static>, + caller: &'static Location<'static>, ) -> Self { Self { value, @@ -910,7 +910,7 @@ impl_debug!(Mut<'w, T>,); pub struct MutUntyped<'w> { pub(crate) value: PtrMut<'w>, pub(crate) ticks: TicksMut<'w>, - pub(crate) caller: &'w mut Location<'static>, + pub(crate) caller: &'static Location<'static>, } impl<'w> MutUntyped<'w> { @@ -1025,8 +1025,8 @@ impl<'w> DetectChanges for MutUntyped<'w> { } #[inline] - fn changed_by(&self) -> Location<'static> { - *self.caller + fn changed_by(&self) -> &'static Location<'static> { + self.caller } } @@ -1037,14 +1037,14 @@ impl<'w> DetectChangesMut for MutUntyped<'w> { #[track_caller] fn set_changed(&mut self) { *self.ticks.changed = self.ticks.this_run; - *self.caller = *Location::caller(); + self.caller = Location::caller(); } #[inline] #[track_caller] fn set_last_changed(&mut self, last_changed: Tick) { *self.ticks.changed = last_changed; - *self.caller = *Location::caller(); + self.caller = Location::caller(); } #[inline] @@ -1210,11 +1210,11 @@ mod tests { this_run: Tick::new(4), }; let mut res = R {}; - let mut caller = *Location::caller(); + let caller = Location::caller(); let res_mut = ResMut { value: &mut res, ticks, - caller: &mut caller, + caller, }; let into_mut: Mut = res_mut.into(); @@ -1231,7 +1231,7 @@ mod tests { changed: Tick::new(3), }; let mut res = R {}; - let mut caller = *Location::caller(); + let caller = Location::caller(); let val = Mut::new( &mut res, @@ -1239,7 +1239,7 @@ mod tests { &mut component_ticks.changed, Tick::new(2), // last_run Tick::new(4), // this_run - &mut caller, + caller, ); assert!(!val.is_added()); @@ -1259,11 +1259,11 @@ mod tests { this_run: Tick::new(4), }; let mut res = R {}; - let mut caller = *Location::caller(); + let caller = Location::caller(); let non_send_mut = NonSendMut { value: &mut res, ticks, - caller: &mut caller, + caller, }; let into_mut: Mut = non_send_mut.into(); @@ -1292,11 +1292,11 @@ mod tests { }; let mut outer = Outer(0); - let mut caller = *Location::caller(); + let caller = Location::caller(); let ptr = Mut { value: &mut outer, ticks, - caller: &mut caller, + caller, }; assert!(!ptr.is_changed()); @@ -1379,11 +1379,11 @@ mod tests { }; let mut value: i32 = 5; - let mut caller = *Location::caller(); + let caller = Location::caller(); let value = MutUntyped { value: PtrMut::from(&mut value), ticks, - caller: &mut caller, + caller, }; let reflect_from_ptr = >::from_type(); @@ -1414,11 +1414,11 @@ mod tests { this_run: Tick::new(4), }; let mut c = C {}; - let mut caller = *Location::caller(); + let caller = Location::caller(); let mut_typed = Mut { value: &mut c, ticks, - caller: &mut caller, + caller, }; let into_mut: MutUntyped = mut_typed.into(); diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 1d508ae670310..c6ed7899cf006 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1026,7 +1026,7 @@ pub struct RefFetch<'w, T> { ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, - ThinSlicePtr<'w, UnsafeCell>>, + ThinSlicePtr<'w, UnsafeCell<&'static Location<'static>>>, )>, // T::STORAGE_TYPE = StorageType::SparseSet sparse_set: Option<&'w ComponentSparseSet>, @@ -1215,7 +1215,7 @@ pub struct WriteFetch<'w, T> { ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, - ThinSlicePtr<'w, UnsafeCell>>, + ThinSlicePtr<'w, UnsafeCell<&'static Location<'static>>>, )>, // T::STORAGE_TYPE = StorageType::SparseSet sparse_set: Option<&'w ComponentSparseSet>, diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index d22eb13954d17..655538ac694c6 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -18,7 +18,7 @@ pub struct ResourceData { type_name: String, id: ArchetypeComponentId, origin_thread_id: Option, - caller: UnsafeCell>, + caller: UnsafeCell<&'static Location<'static>>, } impl Drop for ResourceData { @@ -113,7 +113,11 @@ impl ResourceData { #[inline] pub(crate) fn get_with_ticks( &self, - ) -> Option<(Ptr<'_>, TickCells<'_>, &UnsafeCell>)> { + ) -> Option<( + Ptr<'_>, + TickCells<'_>, + &UnsafeCell<&'static Location<'static>>, + )> { self.is_present().then(|| { self.validate_access(); ( @@ -214,7 +218,9 @@ impl ResourceData { /// original thread it was inserted from. #[inline] #[must_use = "The returned pointer to the removed component should be used or dropped"] - pub(crate) fn remove(&mut self) -> Option<(OwningPtr<'_>, ComponentTicks, Location<'static>)> { + pub(crate) fn remove( + &mut self, + ) -> Option<(OwningPtr<'_>, ComponentTicks, &'static Location<'static>)> { if !self.is_present() { return None; } @@ -345,7 +351,7 @@ impl Resources { type_name: String::from(component_info.name()), id: f(), origin_thread_id: None, - caller: UnsafeCell::new(*Location::caller()) + caller: UnsafeCell::new(Location::caller()) } }) } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index d3a438a481e28..1d5d4a1b4af7f 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -169,7 +169,7 @@ impl ComponentSparseSet { entity: Entity, value: OwningPtr<'_>, change_tick: Tick, - caller: Location<'static>, + caller: &'static Location<'static>, ) { if let Some(&dense_index) = self.sparse.get(entity.index()) { #[cfg(debug_assertions)] @@ -227,7 +227,11 @@ impl ComponentSparseSet { pub fn get_with_ticks( &self, entity: Entity, - ) -> Option<(Ptr<'_>, TickCells<'_>, &UnsafeCell>)> { + ) -> Option<( + Ptr<'_>, + TickCells<'_>, + &UnsafeCell<&'static Location<'static>>, + )> { let dense_index = *self.sparse.get(entity.index())?; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index cf6da7db84a11..4dcf1bbf9c979 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -152,7 +152,7 @@ pub struct Column { data: BlobVec, added_ticks: Vec>, changed_ticks: Vec>, - callers: Vec>>, + callers: Vec>>, } impl Column { @@ -186,7 +186,7 @@ impl Column { row: TableRow, data: OwningPtr<'_>, tick: Tick, - caller: Location<'static>, + caller: &'static Location<'static>, ) { debug_assert!(row.as_usize() < self.len()); self.data.initialize_unchecked(row.as_usize(), data); @@ -209,7 +209,7 @@ impl Column { row: TableRow, data: OwningPtr<'_>, change_tick: Tick, - caller: Location<'static>, + caller: &'static Location<'static>, ) { debug_assert!(row.as_usize() < self.len()); self.data.replace_unchecked(row.as_usize(), data); @@ -266,7 +266,7 @@ impl Column { pub(crate) unsafe fn swap_remove_and_forget_unchecked( &mut self, row: TableRow, - ) -> (OwningPtr<'_>, ComponentTicks, Location<'static>) { + ) -> (OwningPtr<'_>, ComponentTicks, &'static Location<'static>) { let data = self.data.swap_remove_and_forget_unchecked(row.as_usize()); let added = self.added_ticks.swap_remove(row.as_usize()).into_inner(); let changed = self.changed_ticks.swap_remove(row.as_usize()).into_inner(); @@ -311,7 +311,7 @@ impl Column { &mut self, ptr: OwningPtr<'_>, ticks: ComponentTicks, - caller: Location<'static>, + caller: &'static Location<'static>, ) { self.data.push(ptr); self.added_ticks.push(UnsafeCell::new(ticks.added)); @@ -375,7 +375,7 @@ impl Column { /// Users of this API must ensure that accesses to each individual element /// adhere to the safety invariants of [`UnsafeCell`]. #[inline] - pub fn get_callers_slice(&self) -> &[UnsafeCell>] { + pub fn get_callers_slice(&self) -> &[UnsafeCell<&'static Location<'static>>] { &self.callers } @@ -386,7 +386,11 @@ impl Column { pub fn get( &self, row: TableRow, - ) -> Option<(Ptr<'_>, TickCells<'_>, &UnsafeCell>)> { + ) -> Option<( + Ptr<'_>, + TickCells<'_>, + &UnsafeCell<&'static Location<'static>>, + )> { (row.as_usize() < 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. @@ -515,7 +519,10 @@ impl Column { /// # Safety /// `row` must be within the range `[0, self.len())`. #[inline] - pub unsafe fn get_caller_unchecked(&self, row: TableRow) -> &UnsafeCell> { + pub unsafe fn get_caller_unchecked( + &self, + row: TableRow, + ) -> &UnsafeCell<&'static Location<'static>> { debug_assert!(row.as_usize() < self.callers.len()); self.callers.get_unchecked(row.as_usize()) } @@ -793,7 +800,7 @@ impl Table { column.data.set_len(self.entities.len()); column.added_ticks.push(UnsafeCell::new(Tick::new(0))); column.changed_ticks.push(UnsafeCell::new(Tick::new(0))); - column.callers.push(UnsafeCell::new(*Location::caller())); + column.callers.push(UnsafeCell::new(Location::caller())); } TableRow::from_usize(index) } @@ -979,6 +986,7 @@ mod tests { storage::{TableBuilder, TableRow}, }; use std::panic::Location; + #[derive(Component)] struct W(T); @@ -1002,7 +1010,7 @@ mod tests { row, value_ptr, Tick::new(0), - *Location::caller(), + Location::caller(), ); }); }; diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 790471e409ce3..e232f9a6e0318 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1045,7 +1045,7 @@ pub struct NonSend<'w, T: 'static> { ticks: ComponentTicks, last_run: Tick, this_run: Tick, - caller: &'w Location<'static>, + caller: &'static Location<'static>, } // SAFETY: Only reads a single World non-send resource @@ -1072,8 +1072,8 @@ impl<'w, T: 'static> NonSend<'w, T> { } /// The location that last caused this to change. - pub fn changed_by(&self) -> Location<'static> { - *self.caller + pub fn changed_by(&self) -> &'static Location<'static> { + self.caller } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 933934b2691a4..1dac3e6959d82 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -960,7 +960,7 @@ impl World { let entity_location = { let mut bundle_spawner = BundleSpawner::new::(self, change_tick); // SAFETY: bundle's type matches `bundle_info`, entity is allocated but non-existent - unsafe { bundle_spawner.spawn_non_existent(entity, bundle, *Location::caller()) } + unsafe { bundle_spawner.spawn_non_existent(entity, bundle, Location::caller()) } }; // SAFETY: entity and location are valid, as they were just created above @@ -1796,13 +1796,13 @@ impl World { AllocAtWithoutReplacement::DidNotExist => { if let SpawnOrInsert::Spawn(ref mut spawner) = spawn_or_insert { // SAFETY: `entity` is allocated (but non existent), bundle matches inserter - unsafe { spawner.spawn_non_existent(entity, bundle, *Location::caller()) }; + unsafe { spawner.spawn_non_existent(entity, bundle, Location::caller()) }; } else { // SAFETY: we initialized this bundle_id in `init_info` let mut spawner = unsafe { BundleSpawner::new_with_id(self, bundle_id, change_tick) }; // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter - unsafe { spawner.spawn_non_existent(entity, bundle, *Location::caller()) }; + unsafe { spawner.spawn_non_existent(entity, bundle, Location::caller()) }; spawn_or_insert = SpawnOrInsert::Spawn(spawner); } } diff --git a/crates/bevy_ecs/src/world/spawn_batch.rs b/crates/bevy_ecs/src/world/spawn_batch.rs index 105fd972ce867..f379458eb6bf9 100644 --- a/crates/bevy_ecs/src/world/spawn_batch.rs +++ b/crates/bevy_ecs/src/world/spawn_batch.rs @@ -16,7 +16,7 @@ where { inner: I, spawner: BundleSpawner<'w>, - caller: Location<'static>, + caller: &'static Location<'static>, } impl<'w, I> SpawnBatchIter<'w, I> @@ -43,7 +43,7 @@ where Self { inner: iter, spawner, - caller: *Location::caller(), + caller: Location::caller(), } } } diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 5334842f31a32..7d1010b9965b2 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -538,7 +538,11 @@ impl<'w> UnsafeWorldCell<'w> { pub(crate) unsafe fn get_resource_with_ticks( self, component_id: ComponentId, - ) -> Option<(Ptr<'w>, TickCells<'w>, &'w UnsafeCell>)> { + ) -> Option<( + Ptr<'w>, + TickCells<'w>, + &'w UnsafeCell<&'static Location<'static>>, + )> { // SAFETY: // - caller ensures there is no `&mut World` // - caller ensures there are no mutable borrows of this resource @@ -562,7 +566,11 @@ impl<'w> UnsafeWorldCell<'w> { pub(crate) unsafe fn get_non_send_with_ticks( self, component_id: ComponentId, - ) -> Option<(Ptr<'w>, TickCells<'w>, &'w UnsafeCell>)> { + ) -> Option<( + Ptr<'w>, + TickCells<'w>, + &'w UnsafeCell<&'static Location<'static>>, + )> { // SAFETY: // - caller ensures there is no `&mut World` // - caller ensures there are no mutable borrows of this resource @@ -972,7 +980,11 @@ unsafe fn get_component_and_ticks( storage_type: StorageType, entity: Entity, location: EntityLocation, -) -> Option<(Ptr<'_>, TickCells<'_>, &UnsafeCell>)> { +) -> Option<( + Ptr<'_>, + TickCells<'_>, + &UnsafeCell<&'static Location<'static>>, +)> { match storage_type { StorageType::Table => { let components = world.fetch_table(location, component_id)?; From c0b21e1600301c3049728c261d7dfcf8e9f7c2e6 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Sat, 27 Jul 2024 00:04:12 -0400 Subject: [PATCH 4/7] feat: feature flag change detection tracking --- crates/bevy_ecs/Cargo.toml | 2 +- crates/bevy_ecs/src/bundle.rs | 46 ++++-- crates/bevy_ecs/src/change_detection.rs | 132 ++++++++++++++++-- crates/bevy_ecs/src/query/fetch.rs | 36 +++-- crates/bevy_ecs/src/reflect/component.rs | 2 + crates/bevy_ecs/src/reflect/resource.rs | 1 + crates/bevy_ecs/src/storage/resource.rs | 26 ++-- crates/bevy_ecs/src/storage/sparse_set.rs | 25 ++-- crates/bevy_ecs/src/storage/table.rs | 54 +++++-- crates/bevy_ecs/src/system/system_param.rs | 38 +++-- crates/bevy_ecs/src/world/mod.rs | 44 ++++-- crates/bevy_ecs/src/world/spawn_batch.rs | 14 +- .../bevy_ecs/src/world/unsafe_world_cell.rs | 60 ++++---- 13 files changed, 359 insertions(+), 121 deletions(-) diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index 07bb1fc0e7ad4..8e330e66a64f0 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -16,7 +16,7 @@ trace = [] multi_threaded = ["bevy_tasks/multi_threaded", "arrayvec"] bevy_debug_stepping = [] serialize = ["dep:serde"] -change_detection_source = [] +track_change_detection = [] [dependencies] bevy_ptr = { path = "../bevy_ptr", version = "0.14.0-dev" } diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 2134218fcef55..23fb6d634ec8e 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -19,7 +19,9 @@ use crate::{ }; use bevy_ptr::{ConstNonNull, OwningPtr}; use bevy_utils::all_tuples; -use std::{any::TypeId, panic::Location, ptr::NonNull}; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; +use std::{any::TypeId, ptr::NonNull}; /// The `Bundle` trait enables insertion and removal of [`Component`]s from an entity. /// @@ -386,7 +388,7 @@ impl BundleInfo { table_row: TableRow, change_tick: Tick, bundle: T, - caller: &'static Location<'static>, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) { // NOTE: get_components calls this closure on each component in "bundle order". // bundle_info.component_ids are also in "bundle order" @@ -403,10 +405,22 @@ impl BundleInfo { let status = unsafe { bundle_component_status.get_status(bundle_component) }; match status { ComponentStatus::Added => { - column.initialize(table_row, component_ptr, change_tick, caller); + column.initialize( + table_row, + component_ptr, + change_tick, + #[cfg(feature = "track_change_detection")] + caller, + ); } ComponentStatus::Mutated => { - column.replace(table_row, component_ptr, change_tick, caller); + column.replace( + table_row, + component_ptr, + change_tick, + #[cfg(feature = "track_change_detection")] + caller, + ); } } } @@ -415,7 +429,13 @@ impl BundleInfo { // SAFETY: If component_id is in self.component_ids, BundleInfo::new requires that // a sparse set exists for the component. unsafe { sparse_sets.get_mut(component_id).debug_checked_unwrap() }; - sparse_set.insert(entity, component_ptr, change_tick, caller); + sparse_set.insert( + entity, + component_ptr, + change_tick, + #[cfg(feature = "track_change_detection")] + caller, + ); } } bundle_component += 1; @@ -647,6 +667,7 @@ impl<'w> BundleInserter<'w> { let add_bundle = self.add_bundle.as_ref(); let table = self.table.as_mut(); let archetype = self.archetype.as_mut(); + #[cfg(feature = "track_change_detection")] let caller = Location::caller(); let (new_archetype, new_location) = match &mut self.result { @@ -665,6 +686,7 @@ impl<'w> BundleInserter<'w> { location.table_row, self.change_tick, bundle, + #[cfg(feature = "track_change_detection")] caller, ); @@ -704,6 +726,7 @@ impl<'w> BundleInserter<'w> { result.table_row, self.change_tick, bundle, + #[cfg(feature = "track_change_detection")] caller, ); @@ -784,6 +807,7 @@ impl<'w> BundleInserter<'w> { move_result.new_row, self.change_tick, bundle, + #[cfg(feature = "track_change_detection")] caller, ); @@ -886,7 +910,7 @@ impl<'w> BundleSpawner<'w> { &mut self, entity: Entity, bundle: T, - caller: &'static Location<'static>, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) -> EntityLocation { let table = self.table.as_mut(); let archetype = self.archetype.as_mut(); @@ -909,6 +933,7 @@ impl<'w> BundleSpawner<'w> { table_row, self.change_tick, bundle, + #[cfg(feature = "track_change_detection")] caller, ); entities.set(entity.index(), location); @@ -937,12 +962,17 @@ impl<'w> BundleSpawner<'w> { pub unsafe fn spawn( &mut self, bundle: T, - caller: &'static Location<'static>, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) -> Entity { let entity = self.entities().alloc(); // SAFETY: entity is allocated (but non-existent), `T` matches this BundleInfo's type unsafe { - self.spawn_non_existent(entity, bundle, caller); + self.spawn_non_existent( + entity, + bundle, + #[cfg(feature = "track_change_detection")] + caller, + ); } entity } diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index d7f98c3b244e0..0c3b444c9bc42 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -5,10 +5,13 @@ use crate::{ ptr::PtrMut, system::Resource, }; +#[cfg(feature = "track_change_detection")] +use bevy_ptr::ThinSlicePtr; use bevy_ptr::{Ptr, UnsafeCellDeref}; use std::mem; use std::ops::{Deref, DerefMut}; -use std::panic::Location; +#[cfg(feature = "track_change_detection")] +use std::{cell::UnsafeCell, panic::Location}; /// The (arbitrarily chosen) minimum number of world tick increments between `check_tick` scans. /// @@ -66,6 +69,7 @@ pub trait DetectChanges { fn last_changed(&self) -> Tick; /// The location that last caused this to change. + #[cfg(feature = "track_change_detection")] fn changed_by(&self) -> &'static Location<'static>; } @@ -287,6 +291,7 @@ macro_rules! change_detection_impl { } #[inline] + #[cfg(feature = "track_change_detection")] fn changed_by(&self) -> &'static Location<'static> { self.caller } @@ -319,14 +324,20 @@ macro_rules! change_detection_mut_impl { #[track_caller] fn set_changed(&mut self) { *self.ticks.changed = self.ticks.this_run; - self.caller = Location::caller(); + #[cfg(feature = "track_change_detection")] + { + self.caller = Location::caller(); + } } #[inline] #[track_caller] fn set_last_changed(&mut self, last_changed: Tick) { *self.ticks.changed = last_changed; - self.caller = Location::caller(); + #[cfg(feature = "track_change_detection")] + { + self.caller = Location::caller(); + } } #[inline] @@ -340,7 +351,10 @@ macro_rules! change_detection_mut_impl { #[track_caller] fn deref_mut(&mut self) -> &mut Self::Target { self.set_changed(); - self.caller = Location::caller(); + #[cfg(feature = "track_change_detection")] + { + self.caller = Location::caller(); + } self.value } } @@ -378,6 +392,7 @@ macro_rules! impl_methods { last_run: self.ticks.last_run, this_run: self.ticks.this_run, }, + #[cfg(feature = "track_change_detection")] caller: self.caller, } } @@ -408,6 +423,7 @@ macro_rules! impl_methods { Mut { value: f(self.value), ticks: self.ticks, + #[cfg(feature = "track_change_detection")] caller: self.caller, } } @@ -519,6 +535,7 @@ impl<'w> From> for Ticks<'w> { pub struct Res<'w, T: ?Sized + Resource> { pub(crate) value: &'w T, pub(crate) ticks: Ticks<'w>, + #[cfg(feature = "track_change_detection")] pub(crate) caller: &'static Location<'static>, } @@ -532,6 +549,7 @@ impl<'w, T: Resource> Res<'w, T> { Self { value: this.value, ticks: this.ticks.clone(), + #[cfg(feature = "track_change_detection")] caller: this.caller, } } @@ -549,6 +567,7 @@ impl<'w, T: Resource> From> for Res<'w, T> { Self { value: res.value, ticks: res.ticks.into(), + #[cfg(feature = "track_change_detection")] caller: res.caller, } } @@ -582,6 +601,7 @@ impl_debug!(Res<'w, T>, Resource); pub struct ResMut<'w, T: ?Sized + Resource> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, + #[cfg(feature = "track_change_detection")] pub(crate) caller: &'static Location<'static>, } @@ -622,6 +642,7 @@ impl<'w, T: Resource> From> for Mut<'w, T> { Mut { value: other.value, ticks: other.ticks, + #[cfg(feature = "track_change_detection")] caller: other.caller, } } @@ -642,6 +663,7 @@ impl<'w, T: Resource> From> for Mut<'w, T> { pub struct NonSendMut<'w, T: ?Sized + 'static> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, + #[cfg(feature = "track_change_detection")] pub(crate) caller: &'static Location<'static>, } @@ -657,6 +679,7 @@ impl<'w, T: 'static> From> for Mut<'w, T> { Mut { value: other.value, ticks: other.ticks, + #[cfg(feature = "track_change_detection")] caller: other.caller, } } @@ -689,6 +712,7 @@ impl<'w, T: 'static> From> for Mut<'w, T> { pub struct Ref<'w, T: ?Sized> { pub(crate) value: &'w T, pub(crate) ticks: Ticks<'w>, + #[cfg(feature = "track_change_detection")] pub(crate) caller: &'static Location<'static>, } @@ -706,6 +730,7 @@ impl<'w, T: ?Sized> Ref<'w, T> { Ref { value: f(self.value), ticks: self.ticks, + #[cfg(feature = "track_change_detection")] caller: self.caller, } } @@ -727,7 +752,7 @@ impl<'w, T: ?Sized> Ref<'w, T> { changed: &'w Tick, last_run: Tick, this_run: Tick, - caller: &'static Location<'static>, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) -> Ref<'w, T> { Ref { value, @@ -737,6 +762,7 @@ impl<'w, T: ?Sized> Ref<'w, T> { last_run, this_run, }, + #[cfg(feature = "track_change_detection")] caller, } } @@ -819,6 +845,7 @@ impl_debug!(Ref<'w, T>,); pub struct Mut<'w, T: ?Sized> { pub(crate) value: &'w mut T, pub(crate) ticks: TicksMut<'w>, + #[cfg(feature = "track_change_detection")] pub(crate) caller: &'static Location<'static>, } @@ -844,7 +871,7 @@ impl<'w, T: ?Sized> Mut<'w, T> { last_changed: &'w mut Tick, last_run: Tick, this_run: Tick, - caller: &'static Location<'static>, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) -> Self { Self { value, @@ -854,6 +881,7 @@ impl<'w, T: ?Sized> Mut<'w, T> { last_run, this_run, }, + #[cfg(feature = "track_change_detection")] caller, } } @@ -864,6 +892,7 @@ impl<'w, T: ?Sized> From> for Ref<'w, T> { Self { value: mut_ref.value, ticks: mut_ref.ticks.into(), + #[cfg(feature = "track_change_detection")] caller: mut_ref.caller, } } @@ -910,6 +939,7 @@ impl_debug!(Mut<'w, T>,); pub struct MutUntyped<'w> { pub(crate) value: PtrMut<'w>, pub(crate) ticks: TicksMut<'w>, + #[cfg(feature = "track_change_detection")] pub(crate) caller: &'static Location<'static>, } @@ -935,6 +965,7 @@ impl<'w> MutUntyped<'w> { last_run: self.ticks.last_run, this_run: self.ticks.this_run, }, + #[cfg(feature = "track_change_detection")] caller: self.caller, } } @@ -986,6 +1017,7 @@ impl<'w> MutUntyped<'w> { Mut { value: f(self.value), ticks: self.ticks, + #[cfg(feature = "track_change_detection")] caller: self.caller, } } @@ -999,6 +1031,7 @@ impl<'w> MutUntyped<'w> { // SAFETY: `value` is `Aligned` and caller ensures the pointee type is `T`. value: unsafe { self.value.deref_mut() }, ticks: self.ticks, + #[cfg(feature = "track_change_detection")] caller: self.caller, } } @@ -1025,6 +1058,7 @@ impl<'w> DetectChanges for MutUntyped<'w> { } #[inline] + #[cfg(feature = "track_change_detection")] fn changed_by(&self) -> &'static Location<'static> { self.caller } @@ -1037,14 +1071,20 @@ impl<'w> DetectChangesMut for MutUntyped<'w> { #[track_caller] fn set_changed(&mut self) { *self.ticks.changed = self.ticks.this_run; - self.caller = Location::caller(); + #[cfg(feature = "track_change_detection")] + { + self.caller = Location::caller(); + } } #[inline] #[track_caller] fn set_last_changed(&mut self, last_changed: Tick) { *self.ticks.changed = last_changed; - self.caller = Location::caller(); + #[cfg(feature = "track_change_detection")] + { + self.caller = Location::caller(); + } } #[inline] @@ -1066,20 +1106,71 @@ impl<'w, T> From> for MutUntyped<'w> { MutUntyped { value: value.value.into(), ticks: value.ticks, + #[cfg(feature = "track_change_detection")] caller: value.caller, } } } +/// A type alias to [`&'static Location<'static>`](std::panic::Location) when the `track_change_detection` feature is +/// enabled, and the unit type `()` when it is not. +/// +/// This is primarily used in places where `#[cfg(...)]` attributes are not allowed, such as +/// function return types. Because unit is a zero-sized type, it is the equivalent of not using a +/// `Location` at all. +/// +/// Please use this type sparingly: prefer normal `#[cfg(...)]` attributes when possible. +#[cfg(feature = "track_change_detection")] +pub(crate) type MaybeLocation = &'static Location<'static>; + +/// A type alias to [`&'static Location<'static>`](std::panic::Location) when the `track_change_detection` feature is +/// enabled, and the unit type `()` when it is not. +/// +/// This is primarily used in places where `#[cfg(...)]` attributes are not allowed, such as +/// function return types. Because unit is a zero-sized type, it is the equivalent of not using a +/// `Location` at all. +/// +/// Please use this type sparingly: prefer normal `#[cfg(...)]` attributes when possible. +#[cfg(not(feature = "track_change_detection"))] +pub(crate) type MaybeLocation = (); + +/// A type alias to `&UnsafeCell<&'static Location<'static>>` when the `track_change_detecion` +/// feature is enabled, and the unit type `()` when it is not. +/// +/// See [`MaybeLocation`] for further information. +#[cfg(feature = "track_change_detection")] +pub(crate) type MaybeUnsafeCellLocation<'a> = &'a UnsafeCell<&'static Location<'static>>; + +/// A type alias to `&UnsafeCell<&'static Location<'static>>` when the `track_change_detecion` +/// feature is enabled, and the unit type `()` when it is not. +/// +/// See [`MaybeLocation`] for further information. +#[cfg(not(feature = "track_change_detection"))] +pub(crate) type MaybeUnsafeCellLocation<'a> = (); + +/// A type alias to `ThinSlicePtr<'w, UnsafeCell<&'static Location<'static>>>` when the +/// `track_change_detection` feature is enabled, and the unit type `()` when it is not. +/// +/// See [`MaybeLocation`] for further information. +#[cfg(feature = "track_change_detection")] +pub(crate) type MaybeThinSlicePtrLocation<'w> = + ThinSlicePtr<'w, UnsafeCell<&'static Location<'static>>>; + +/// A type alias to `ThinSlicePtr<'w, UnsafeCell<&'static Location<'static>>>` when the +/// `track_change_detection` feature is enabled, and the unit type `()` when it is not. +/// +/// See [`MaybeLocation`] for further information. +#[cfg(not(feature = "track_change_detection"))] +pub(crate) type MaybeThinSlicePtrLocation<'w> = (); + #[cfg(test)] mod tests { use bevy_ecs_macros::Resource; use bevy_ptr::PtrMut; use bevy_reflect::{FromType, ReflectFromPtr}; - use std::{ - ops::{Deref, DerefMut}, - panic::Location, - }; + use std::ops::{Deref, DerefMut}; + #[cfg(feature = "track_change_detection")] + use std::panic::Location; use crate::{ self as bevy_ecs, @@ -1210,10 +1301,13 @@ mod tests { this_run: Tick::new(4), }; let mut res = R {}; + #[cfg(feature = "track_change_detection")] let caller = Location::caller(); + let res_mut = ResMut { value: &mut res, ticks, + #[cfg(feature = "track_change_detection")] caller, }; @@ -1231,6 +1325,7 @@ mod tests { changed: Tick::new(3), }; let mut res = R {}; + #[cfg(feature = "track_change_detection")] let caller = Location::caller(); let val = Mut::new( @@ -1239,6 +1334,7 @@ mod tests { &mut component_ticks.changed, Tick::new(2), // last_run Tick::new(4), // this_run + #[cfg(feature = "track_change_detection")] caller, ); @@ -1259,10 +1355,13 @@ mod tests { this_run: Tick::new(4), }; let mut res = R {}; + #[cfg(feature = "track_change_detection")] let caller = Location::caller(); + let non_send_mut = NonSendMut { value: &mut res, ticks, + #[cfg(feature = "track_change_detection")] caller, }; @@ -1292,10 +1391,13 @@ mod tests { }; let mut outer = Outer(0); + #[cfg(feature = "track_change_detection")] let caller = Location::caller(); + let ptr = Mut { value: &mut outer, ticks, + #[cfg(feature = "track_change_detection")] caller, }; assert!(!ptr.is_changed()); @@ -1379,10 +1481,13 @@ mod tests { }; let mut value: i32 = 5; + #[cfg(feature = "track_change_detection")] let caller = Location::caller(); + let value = MutUntyped { value: PtrMut::from(&mut value), ticks, + #[cfg(feature = "track_change_detection")] caller, }; @@ -1414,10 +1519,13 @@ mod tests { this_run: Tick::new(4), }; let mut c = C {}; + #[cfg(feature = "track_change_detection")] let caller = Location::caller(); + let mut_typed = Mut { value: &mut c, ticks, + #[cfg(feature = "track_change_detection")] caller, }; diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index c6ed7899cf006..587d6d3bd4fbd 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1,6 +1,6 @@ use crate::{ archetype::{Archetype, Archetypes}, - change_detection::{Ticks, TicksMut}, + change_detection::{MaybeThinSlicePtrLocation, Ticks, TicksMut}, component::{Component, ComponentId, Components, StorageType, Tick}, entity::{Entities, Entity, EntityLocation}, query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery}, @@ -12,7 +12,7 @@ use crate::{ }; use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref}; use bevy_utils::all_tuples; -use std::{cell::UnsafeCell, marker::PhantomData, panic::Location}; +use std::{cell::UnsafeCell, marker::PhantomData}; /// Types that can be fetched from a [`World`] using a [`Query`]. /// @@ -1026,7 +1026,7 @@ pub struct RefFetch<'w, T> { ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, - ThinSlicePtr<'w, UnsafeCell<&'static Location<'static>>>, + MaybeThinSlicePtrLocation<'w>, )>, // T::STORAGE_TYPE = StorageType::SparseSet sparse_set: Option<&'w ComponentSparseSet>, @@ -1116,7 +1116,10 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { column.get_data_slice().into(), column.get_added_ticks_slice().into(), column.get_changed_ticks_slice().into(), + #[cfg(feature = "track_change_detection")] column.get_callers_slice().into(), + #[cfg(not(feature = "track_change_detection"))] + (), )); } @@ -1129,7 +1132,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { match T::STORAGE_TYPE { StorageType::Table => { // SAFETY: STORAGE_TYPE = Table - let (table_components, added_ticks, changed_ticks, callers) = + let (table_components, added_ticks, changed_ticks, _callers) = unsafe { fetch.table_data.debug_checked_unwrap() }; // SAFETY: The caller ensures `table_row` is in range. @@ -1139,7 +1142,8 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { // SAFETY: The caller ensures `table_row` is in range. let changed = unsafe { changed_ticks.get(table_row.as_usize()) }; // SAFETY: The caller ensures `table_row` is in range. - let caller = unsafe { callers.get(table_row.as_usize()) }; + #[cfg(feature = "track_change_detection")] + let caller = unsafe { _callers.get(table_row.as_usize()) }; Ref { value: component.deref(), @@ -1149,6 +1153,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { this_run: fetch.this_run, last_run: fetch.last_run, }, + #[cfg(feature = "track_change_detection")] caller: caller.deref(), } } @@ -1157,7 +1162,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { let component_sparse_set = unsafe { fetch.sparse_set.debug_checked_unwrap() }; // SAFETY: The caller ensures `entity` is in range. - let (component, ticks, caller) = unsafe { + let (component, ticks, _caller) = unsafe { component_sparse_set .get_with_ticks(entity) .debug_checked_unwrap() @@ -1166,7 +1171,8 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { Ref { value: component.deref(), ticks: Ticks::from_tick_cells(ticks, fetch.last_run, fetch.this_run), - caller: caller.deref(), + #[cfg(feature = "track_change_detection")] + caller: _caller.deref(), } } } @@ -1215,7 +1221,7 @@ pub struct WriteFetch<'w, T> { ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, ThinSlicePtr<'w, UnsafeCell>, - ThinSlicePtr<'w, UnsafeCell<&'static Location<'static>>>, + MaybeThinSlicePtrLocation<'w>, )>, // T::STORAGE_TYPE = StorageType::SparseSet sparse_set: Option<&'w ComponentSparseSet>, @@ -1305,7 +1311,10 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { column.get_data_slice().into(), column.get_added_ticks_slice().into(), column.get_changed_ticks_slice().into(), + #[cfg(feature = "track_change_detection")] column.get_callers_slice().into(), + #[cfg(not(feature = "track_change_detection"))] + (), )); } @@ -1318,7 +1327,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { match T::STORAGE_TYPE { StorageType::Table => { // SAFETY: STORAGE_TYPE = Table - let (table_components, added_ticks, changed_ticks, callers) = + let (table_components, added_ticks, changed_ticks, _callers) = unsafe { fetch.table_data.debug_checked_unwrap() }; // SAFETY: The caller ensures `table_row` is in range. @@ -1328,7 +1337,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { // SAFETY: The caller ensures `table_row` is in range. let changed = unsafe { changed_ticks.get(table_row.as_usize()) }; // SAFETY: The caller ensures `table_row` is in range. - let caller = unsafe { callers.get(table_row.as_usize()) }; + #[cfg(feature = "track_change_detection")] + let caller = unsafe { _callers.get(table_row.as_usize()) }; Mut { value: component.deref_mut(), @@ -1338,6 +1348,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { this_run: fetch.this_run, last_run: fetch.last_run, }, + #[cfg(feature = "track_change_detection")] caller: caller.deref_mut(), } } @@ -1346,7 +1357,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { let component_sparse_set = unsafe { fetch.sparse_set.debug_checked_unwrap() }; // SAFETY: The caller ensures `entity` is in range. - let (component, ticks, caller) = unsafe { + let (component, ticks, _caller) = unsafe { component_sparse_set .get_with_ticks(entity) .debug_checked_unwrap() @@ -1355,7 +1366,8 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { Mut { value: component.assert_unique().deref_mut(), ticks: TicksMut::from_tick_cells(ticks, fetch.last_run, fetch.this_run), - caller: caller.deref_mut(), + #[cfg(feature = "track_change_detection")] + caller: _caller.deref_mut(), } } } diff --git a/crates/bevy_ecs/src/reflect/component.rs b/crates/bevy_ecs/src/reflect/component.rs index 90cbd9e2f826b..83024814eeb05 100644 --- a/crates/bevy_ecs/src/reflect/component.rs +++ b/crates/bevy_ecs/src/reflect/component.rs @@ -296,6 +296,7 @@ impl FromType for ReflectComponent { entity.into_mut::().map(|c| Mut { value: c.value as &mut dyn Reflect, ticks: c.ticks, + #[cfg(feature = "track_change_detection")] caller: c.caller, }) }, @@ -306,6 +307,7 @@ impl FromType for ReflectComponent { entity.get_mut::().map(|c| Mut { value: c.value as &mut dyn Reflect, ticks: c.ticks, + #[cfg(feature = "track_change_detection")] caller: c.caller, }) } diff --git a/crates/bevy_ecs/src/reflect/resource.rs b/crates/bevy_ecs/src/reflect/resource.rs index a7523c322b3d2..1c2c4164fb6da 100644 --- a/crates/bevy_ecs/src/reflect/resource.rs +++ b/crates/bevy_ecs/src/reflect/resource.rs @@ -207,6 +207,7 @@ impl FromType for ReflectResource { world.get_resource_mut::().map(|res| Mut { value: res.value as &mut dyn Reflect, ticks: res.ticks, + #[cfg(feature = "track_change_detection")] caller: res.caller, }) } diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 655538ac694c6..9080c6d2cfbc8 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -1,8 +1,9 @@ use crate::archetype::ArchetypeComponentId; -use crate::change_detection::{MutUntyped, TicksMut}; +use crate::change_detection::{MaybeLocation, MaybeUnsafeCellLocation, MutUntyped, TicksMut}; use crate::component::{ComponentId, ComponentTicks, Components, Tick, TickCells}; use crate::storage::{blob_vec::BlobVec, SparseSet}; use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; +#[cfg(feature = "track_change_detection")] use std::panic::Location; use std::{cell::UnsafeCell, mem::ManuallyDrop, thread::ThreadId}; @@ -18,6 +19,7 @@ pub struct ResourceData { type_name: String, id: ArchetypeComponentId, origin_thread_id: Option, + #[cfg(feature = "track_change_detection")] caller: UnsafeCell<&'static Location<'static>>, } @@ -113,11 +115,7 @@ impl ResourceData { #[inline] pub(crate) fn get_with_ticks( &self, - ) -> Option<( - Ptr<'_>, - TickCells<'_>, - &UnsafeCell<&'static Location<'static>>, - )> { + ) -> Option<(Ptr<'_>, TickCells<'_>, MaybeUnsafeCellLocation<'_>)> { self.is_present().then(|| { self.validate_access(); ( @@ -127,7 +125,10 @@ impl ResourceData { added: &self.added_ticks, changed: &self.changed_ticks, }, + #[cfg(feature = "track_change_detection")] &self.caller, + #[cfg(not(feature = "track_change_detection"))] + (), ) }) } @@ -138,14 +139,15 @@ impl ResourceData { /// If `SEND` is false, this will panic if a value is present and is not accessed from the /// original thread it was inserted in. pub(crate) fn get_mut(&mut self, last_run: Tick, this_run: Tick) -> Option> { - let (ptr, ticks, caller) = self.get_with_ticks()?; + let (ptr, ticks, _caller) = self.get_with_ticks()?; Some(MutUntyped { // SAFETY: We have exclusive access to the underlying storage. value: unsafe { ptr.assert_unique() }, // SAFETY: We have exclusive access to the underlying storage. ticks: unsafe { TicksMut::from_tick_cells(ticks, last_run, this_run) }, + #[cfg(feature = "track_change_detection")] // SAFETY: We have exclusive access to the underlying storage. - caller: unsafe { caller.deref_mut() }, + caller: unsafe { _caller.deref_mut() }, }) } @@ -218,9 +220,7 @@ impl ResourceData { /// original thread it was inserted from. #[inline] #[must_use = "The returned pointer to the removed component should be used or dropped"] - pub(crate) fn remove( - &mut self, - ) -> Option<(OwningPtr<'_>, ComponentTicks, &'static Location<'static>)> { + pub(crate) fn remove(&mut self) -> Option<(OwningPtr<'_>, ComponentTicks, MaybeLocation)> { if !self.is_present() { return None; } @@ -231,7 +231,10 @@ impl ResourceData { let res = unsafe { self.data.swap_remove_and_forget_unchecked(Self::ROW) }; // SAFETY: This function is being called through an exclusive mutable reference to Self + #[cfg(feature = "track_change_detection")] let caller = unsafe { *self.caller.deref_mut() }; + #[cfg(not(feature = "track_change_detection"))] + let caller = (); // SAFETY: This function is being called through an exclusive mutable reference to Self, which // makes it sound to read these ticks. @@ -351,6 +354,7 @@ impl Resources { type_name: String::from(component_info.name()), id: f(), origin_thread_id: None, + #[cfg(feature = "track_change_detection")] caller: UnsafeCell::new(Location::caller()) } }) diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 1d5d4a1b4af7f..3b10c69a5436c 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -1,11 +1,14 @@ use crate::{ + change_detection::MaybeUnsafeCellLocation, component::{ComponentId, ComponentInfo, ComponentTicks, Tick, TickCells}, entity::Entity, storage::{Column, TableRow}, }; use bevy_ptr::{OwningPtr, Ptr}; use nonmax::NonMaxUsize; -use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData, panic::Location}; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; +use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData}; type EntityIndex = u32; @@ -169,16 +172,21 @@ impl ComponentSparseSet { entity: Entity, value: OwningPtr<'_>, change_tick: Tick, - caller: &'static Location<'static>, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) { if let Some(&dense_index) = self.sparse.get(entity.index()) { #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); + #[cfg(feature = "track_change_detection")] self.dense.replace(dense_index, value, change_tick, caller); } else { let dense_index = self.dense.len(); - self.dense - .push(value, ComponentTicks::new(change_tick), caller); + self.dense.push( + value, + ComponentTicks::new(change_tick), + #[cfg(feature = "track_change_detection")] + caller, + ); self.sparse .insert(entity.index(), TableRow::from_usize(dense_index)); #[cfg(debug_assertions)] @@ -227,11 +235,7 @@ impl ComponentSparseSet { pub fn get_with_ticks( &self, entity: Entity, - ) -> Option<( - Ptr<'_>, - TickCells<'_>, - &UnsafeCell<&'static Location<'static>>, - )> { + ) -> Option<(Ptr<'_>, TickCells<'_>, MaybeUnsafeCellLocation<'_>)> { let dense_index = *self.sparse.get(entity.index())?; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); @@ -243,7 +247,10 @@ impl ComponentSparseSet { added: self.dense.get_added_tick_unchecked(dense_index), changed: self.dense.get_changed_tick_unchecked(dense_index), }, + #[cfg(feature = "track_change_detection")] self.dense.get_caller_unchecked(dense_index), + #[cfg(not(feature = "track_change_detection"))] + (), )) } } diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 4dcf1bbf9c979..a853a663fc434 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -1,4 +1,5 @@ use crate::{ + change_detection::{MaybeLocation, MaybeUnsafeCellLocation}, component::{ComponentId, ComponentInfo, ComponentTicks, Components, Tick, TickCells}, entity::Entity, query::DebugCheckedUnwrap, @@ -6,7 +7,9 @@ use crate::{ }; use bevy_ptr::{OwningPtr, Ptr, PtrMut, UnsafeCellDeref}; use bevy_utils::HashMap; -use std::{alloc::Layout, panic::Location}; +use std::alloc::Layout; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; use std::{ cell::UnsafeCell, ops::{Index, IndexMut}, @@ -152,6 +155,7 @@ pub struct Column { data: BlobVec, added_ticks: Vec>, changed_ticks: Vec>, + #[cfg(feature = "track_change_detection")] callers: Vec>>, } @@ -164,6 +168,7 @@ 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), + #[cfg(feature = "track_change_detection")] callers: Vec::with_capacity(capacity), } } @@ -186,7 +191,7 @@ impl Column { row: TableRow, data: OwningPtr<'_>, tick: Tick, - caller: &'static Location<'static>, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) { debug_assert!(row.as_usize() < self.len()); self.data.initialize_unchecked(row.as_usize(), data); @@ -195,7 +200,10 @@ impl Column { .changed_ticks .get_unchecked_mut(row.as_usize()) .get_mut() = tick; - *self.callers.get_unchecked_mut(row.as_usize()).get_mut() = caller; + #[cfg(feature = "track_change_detection")] + { + *self.callers.get_unchecked_mut(row.as_usize()).get_mut() = caller; + } } /// Writes component data to the column at given row. @@ -209,7 +217,7 @@ impl Column { row: TableRow, data: OwningPtr<'_>, change_tick: Tick, - caller: &'static Location<'static>, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) { debug_assert!(row.as_usize() < self.len()); self.data.replace_unchecked(row.as_usize(), data); @@ -217,7 +225,11 @@ impl Column { .changed_ticks .get_unchecked_mut(row.as_usize()) .get_mut() = change_tick; - *self.callers.get_unchecked_mut(row.as_usize()).get_mut() = caller; + + #[cfg(feature = "track_change_detection")] + { + *self.callers.get_unchecked_mut(row.as_usize()).get_mut() = caller; + } } /// Gets the current number of elements stored in the column. @@ -247,6 +259,7 @@ impl Column { self.data.swap_remove_and_drop_unchecked(row.as_usize()); self.added_ticks.swap_remove(row.as_usize()); self.changed_ticks.swap_remove(row.as_usize()); + #[cfg(feature = "track_change_detection")] self.callers.swap_remove(row.as_usize()); } @@ -266,11 +279,14 @@ impl Column { pub(crate) unsafe fn swap_remove_and_forget_unchecked( &mut self, row: TableRow, - ) -> (OwningPtr<'_>, ComponentTicks, &'static Location<'static>) { + ) -> (OwningPtr<'_>, ComponentTicks, MaybeLocation) { let data = self.data.swap_remove_and_forget_unchecked(row.as_usize()); let added = self.added_ticks.swap_remove(row.as_usize()).into_inner(); let changed = self.changed_ticks.swap_remove(row.as_usize()).into_inner(); + #[cfg(feature = "track_change_detection")] let caller = self.callers.swap_remove(row.as_usize()).into_inner(); + #[cfg(not(feature = "track_change_detection"))] + let caller = (); (data, ComponentTicks { added, changed }, caller) } @@ -299,8 +315,11 @@ impl Column { other.added_ticks.swap_remove(src_row.as_usize()); *self.changed_ticks.get_unchecked_mut(dst_row.as_usize()) = other.changed_ticks.swap_remove(src_row.as_usize()); - *self.callers.get_unchecked_mut(dst_row.as_usize()) = - other.callers.swap_remove(src_row.as_usize()); + #[cfg(feature = "track_change_detection")] + { + *self.callers.get_unchecked_mut(dst_row.as_usize()) = + other.callers.swap_remove(src_row.as_usize()); + } } /// Pushes a new value onto the end of the [`Column`]. @@ -311,11 +330,12 @@ impl Column { &mut self, ptr: OwningPtr<'_>, ticks: ComponentTicks, - caller: &'static Location<'static>, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, ) { self.data.push(ptr); self.added_ticks.push(UnsafeCell::new(ticks.added)); self.changed_ticks.push(UnsafeCell::new(ticks.changed)); + #[cfg(feature = "track_change_detection")] self.callers.push(UnsafeCell::new(caller)); } @@ -324,6 +344,7 @@ impl Column { self.data.reserve_exact(additional); self.added_ticks.reserve_exact(additional); self.changed_ticks.reserve_exact(additional); + #[cfg(feature = "track_change_detection")] self.callers.reserve_exact(additional); } @@ -375,6 +396,7 @@ impl Column { /// Users of this API must ensure that accesses to each individual element /// adhere to the safety invariants of [`UnsafeCell`]. #[inline] + #[cfg(feature = "track_change_detection")] pub fn get_callers_slice(&self) -> &[UnsafeCell<&'static Location<'static>>] { &self.callers } @@ -386,11 +408,7 @@ impl Column { pub fn get( &self, row: TableRow, - ) -> Option<( - Ptr<'_>, - TickCells<'_>, - &UnsafeCell<&'static Location<'static>>, - )> { + ) -> Option<(Ptr<'_>, TickCells<'_>, MaybeUnsafeCellLocation<'_>)> { (row.as_usize() < 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. @@ -401,7 +419,10 @@ impl Column { added: self.added_ticks.get_unchecked(row.as_usize()), changed: self.changed_ticks.get_unchecked(row.as_usize()), }, + #[cfg(feature = "track_change_detection")] self.callers.get_unchecked(row.as_usize()), + #[cfg(not(feature = "track_change_detection"))] + (), ) }) } @@ -519,6 +540,7 @@ impl Column { /// # Safety /// `row` must be within the range `[0, self.len())`. #[inline] + #[cfg(feature = "track_change_detection")] pub unsafe fn get_caller_unchecked( &self, row: TableRow, @@ -549,6 +571,7 @@ impl Column { self.data.clear(); self.added_ticks.clear(); self.changed_ticks.clear(); + #[cfg(feature = "track_change_detection")] self.callers.clear(); } @@ -800,6 +823,7 @@ impl Table { column.data.set_len(self.entities.len()); column.added_ticks.push(UnsafeCell::new(Tick::new(0))); column.changed_ticks.push(UnsafeCell::new(Tick::new(0))); + #[cfg(feature = "track_change_detection")] column.callers.push(UnsafeCell::new(Location::caller())); } TableRow::from_usize(index) @@ -985,6 +1009,7 @@ mod tests { entity::Entity, storage::{TableBuilder, TableRow}, }; + #[cfg(feature = "track_change_detection")] use std::panic::Location; #[derive(Component)] @@ -1010,6 +1035,7 @@ mod tests { row, value_ptr, Tick::new(0), + #[cfg(feature = "track_change_detection")] Location::caller(), ); }); diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index e232f9a6e0318..1e0065e48853f 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -18,11 +18,12 @@ pub use bevy_ecs_macros::Resource; pub use bevy_ecs_macros::SystemParam; use bevy_ptr::UnsafeCellDeref; use bevy_utils::{all_tuples, synccell::SyncCell}; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; use std::{ fmt::Debug, marker::PhantomData, ops::{Deref, DerefMut}, - panic::Location, }; /// A parameter that can be used in a [`System`](super::System). @@ -524,7 +525,7 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Self::Item<'w, 's> { - let (ptr, ticks, caller) = + let (ptr, ticks, _caller) = world .get_resource_with_ticks(component_id) .unwrap_or_else(|| { @@ -542,7 +543,8 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { last_run: system_meta.last_run, this_run: change_tick, }, - caller: caller.deref(), + #[cfg(feature = "track_change_detection")] + caller: _caller.deref(), } } } @@ -568,7 +570,7 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { ) -> Self::Item<'w, 's> { world .get_resource_with_ticks(component_id) - .map(|(ptr, ticks, caller)| Res { + .map(|(ptr, ticks, _caller)| Res { value: ptr.deref(), ticks: Ticks { added: ticks.added.deref(), @@ -576,7 +578,8 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { last_run: system_meta.last_run, this_run: change_tick, }, - caller: caller.deref(), + #[cfg(feature = "track_change_detection")] + caller: _caller.deref(), }) } } @@ -639,6 +642,7 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { last_run: system_meta.last_run, this_run: change_tick, }, + #[cfg(feature = "track_change_detection")] caller: value.caller, } } @@ -670,6 +674,7 @@ unsafe impl<'a, T: Resource> SystemParam for Option> { last_run: system_meta.last_run, this_run: change_tick, }, + #[cfg(feature = "track_change_detection")] caller: value.caller, }) } @@ -1045,6 +1050,7 @@ pub struct NonSend<'w, T: 'static> { ticks: ComponentTicks, last_run: Tick, this_run: Tick, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, } @@ -1072,6 +1078,7 @@ impl<'w, T: 'static> NonSend<'w, T> { } /// The location that last caused this to change. + #[cfg(feature = "track_change_detection")] pub fn changed_by(&self) -> &'static Location<'static> { self.caller } @@ -1094,6 +1101,7 @@ impl<'a, T> From> for NonSend<'a, T> { }, this_run: nsm.ticks.this_run, last_run: nsm.ticks.last_run, + #[cfg(feature = "track_change_detection")] caller: nsm.caller, } } @@ -1139,7 +1147,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Self::Item<'w, 's> { - let (ptr, ticks, caller) = + let (ptr, ticks, _caller) = world .get_non_send_with_ticks(component_id) .unwrap_or_else(|| { @@ -1155,7 +1163,8 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { ticks: ticks.read(), last_run: system_meta.last_run, this_run: change_tick, - caller: caller.deref(), + #[cfg(feature = "track_change_detection")] + caller: _caller.deref(), } } } @@ -1181,12 +1190,13 @@ unsafe impl SystemParam for Option> { ) -> Self::Item<'w, 's> { world .get_non_send_with_ticks(component_id) - .map(|(ptr, ticks, caller)| NonSend { + .map(|(ptr, ticks, _caller)| NonSend { value: ptr.deref(), ticks: ticks.read(), last_run: system_meta.last_run, this_run: change_tick, - caller: caller.deref(), + #[cfg(feature = "track_change_detection")] + caller: _caller.deref(), }) } } @@ -1234,7 +1244,7 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> Self::Item<'w, 's> { - let (ptr, ticks, caller) = + let (ptr, ticks, _caller) = world .get_non_send_with_ticks(component_id) .unwrap_or_else(|| { @@ -1247,7 +1257,8 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { NonSendMut { value: ptr.assert_unique().deref_mut(), ticks: TicksMut::from_tick_cells(ticks, system_meta.last_run, change_tick), - caller: caller.deref_mut(), + #[cfg(feature = "track_change_detection")] + caller: _caller.deref_mut(), } } } @@ -1270,10 +1281,11 @@ unsafe impl<'a, T: 'static> SystemParam for Option> { ) -> Self::Item<'w, 's> { world .get_non_send_with_ticks(component_id) - .map(|(ptr, ticks, caller)| NonSendMut { + .map(|(ptr, ticks, _caller)| NonSendMut { value: ptr.assert_unique().deref_mut(), ticks: TicksMut::from_tick_cells(ticks, system_meta.last_run, change_tick), - caller: caller.deref_mut(), + #[cfg(feature = "track_change_detection")] + caller: _caller.deref_mut(), }) } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 1dac3e6959d82..bc1c76be16701 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -35,13 +35,16 @@ use crate::{ world::command_queue::RawCommandQueue, world::error::TryRunScheduleError, }; -use bevy_ptr::{OwningPtr, Ptr, UnsafeCellDeref}; +#[cfg(feature = "track_change_detection")] +use bevy_ptr::UnsafeCellDeref; +use bevy_ptr::{OwningPtr, Ptr}; use bevy_utils::tracing::warn; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; use std::{ any::TypeId, fmt, mem::MaybeUninit, - panic::Location, sync::atomic::{AtomicU32, Ordering}, }; mod identifier; @@ -960,7 +963,14 @@ impl World { let entity_location = { let mut bundle_spawner = BundleSpawner::new::(self, change_tick); // SAFETY: bundle's type matches `bundle_info`, entity is allocated but non-existent - unsafe { bundle_spawner.spawn_non_existent(entity, bundle, Location::caller()) } + unsafe { + bundle_spawner.spawn_non_existent( + entity, + bundle, + #[cfg(feature = "track_change_detection")] + Location::caller(), + ) + } }; // SAFETY: entity and location are valid, as they were just created above @@ -1796,13 +1806,27 @@ impl World { AllocAtWithoutReplacement::DidNotExist => { if let SpawnOrInsert::Spawn(ref mut spawner) = spawn_or_insert { // SAFETY: `entity` is allocated (but non existent), bundle matches inserter - unsafe { spawner.spawn_non_existent(entity, bundle, Location::caller()) }; + unsafe { + spawner.spawn_non_existent( + entity, + bundle, + #[cfg(feature = "track_change_detection")] + Location::caller(), + ) + }; } else { // SAFETY: we initialized this bundle_id in `init_info` let mut spawner = unsafe { BundleSpawner::new_with_id(self, bundle_id, change_tick) }; // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter - unsafe { spawner.spawn_non_existent(entity, bundle, Location::caller()) }; + unsafe { + spawner.spawn_non_existent( + entity, + bundle, + #[cfg(feature = "track_change_detection")] + Location::caller(), + ) + }; spawn_or_insert = SpawnOrInsert::Spawn(spawner); } } @@ -1850,7 +1874,7 @@ impl World { .components .get_resource_id(TypeId::of::()) .unwrap_or_else(|| panic!("resource does not exist: {}", std::any::type_name::())); - let (ptr, mut ticks, mut caller) = self + let (ptr, mut ticks, mut _caller) = self .storages .resources .get_mut(component_id) @@ -1867,7 +1891,8 @@ impl World { last_run: last_change_tick, this_run: change_tick, }, - caller: &mut caller, + #[cfg(feature = "track_change_detection")] + caller: _caller, }; let result = f(self, value_mut); assert!(!self.contains_resource::(), @@ -2476,7 +2501,7 @@ impl World { .get_info(component_id) .debug_checked_unwrap() }; - let (ptr, ticks, caller) = data.get_with_ticks()?; + let (ptr, ticks, _caller) = data.get_with_ticks()?; // SAFETY: // - We have exclusive access to the world, so no other code can be aliasing the `TickCells` @@ -2495,10 +2520,11 @@ impl World { // - We iterate one resource at a time, and we let go of each `PtrMut` before getting the next one value: unsafe { ptr.assert_unique() }, ticks, + #[cfg(feature = "track_change_detection")] // SAFETY: // - We have exclusive access to the world, so no other code can be aliasing the `Ptr` // - We iterate one resource at a time, and we let go of each `PtrMut` before getting the next one - caller: unsafe { caller.deref_mut() }, + caller: unsafe { _caller.deref_mut() }, }; Some((component_info, mut_untyped)) diff --git a/crates/bevy_ecs/src/world/spawn_batch.rs b/crates/bevy_ecs/src/world/spawn_batch.rs index f379458eb6bf9..b51029711a2e4 100644 --- a/crates/bevy_ecs/src/world/spawn_batch.rs +++ b/crates/bevy_ecs/src/world/spawn_batch.rs @@ -3,7 +3,9 @@ use crate::{ entity::Entity, world::World, }; -use std::{iter::FusedIterator, panic::Location}; +use std::iter::FusedIterator; +#[cfg(feature = "track_change_detection")] +use std::panic::Location; /// An iterator that spawns a series of entities and returns the [ID](Entity) of /// each spawned entity. @@ -16,6 +18,7 @@ where { inner: I, spawner: BundleSpawner<'w>, + #[cfg(feature = "track_change_detection")] caller: &'static Location<'static>, } @@ -43,6 +46,7 @@ where Self { inner: iter, spawner, + #[cfg(feature = "track_change_detection")] caller: Location::caller(), } } @@ -72,7 +76,13 @@ where fn next(&mut self) -> Option { let bundle = self.inner.next()?; // SAFETY: bundle matches spawner type - unsafe { Some(self.spawner.spawn(bundle, self.caller)) } + unsafe { + Some(self.spawner.spawn( + bundle, + #[cfg(feature = "track_change_detection")] + self.caller, + )) + } } fn size_hint(&self) -> (usize, Option) { diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 7d1010b9965b2..30c7f939d236a 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -6,7 +6,7 @@ use super::{Mut, Ref, World, WorldId}; use crate::{ archetype::{Archetype, Archetypes}, bundle::Bundles, - change_detection::{MutUntyped, Ticks, TicksMut}, + change_detection::{MaybeUnsafeCellLocation, MutUntyped, Ticks, TicksMut}, component::{ComponentId, ComponentTicks, Components, StorageType, Tick, TickCells}, entity::{Entities, Entity, EntityLocation}, prelude::Component, @@ -15,8 +15,10 @@ use crate::{ system::{Res, Resource}, world::RawCommandQueue, }; -use bevy_ptr::{Ptr, UnsafeCellDeref}; -use std::{any::TypeId, cell::UnsafeCell, fmt::Debug, marker::PhantomData, panic::Location, ptr}; +use bevy_ptr::Ptr; +#[cfg(feature = "track_change_detection")] +use bevy_ptr::UnsafeCellDeref; +use std::{any::TypeId, cell::UnsafeCell, fmt::Debug, marker::PhantomData, ptr}; /// Variant of the [`World`] where resource and component accesses take `&self`, and the responsibility to avoid /// aliasing violations are given to the caller instead of being checked at compile-time by rust's unique XOR shared rule. @@ -323,7 +325,7 @@ impl<'w> UnsafeWorldCell<'w> { // SAFETY: caller ensures `self` has permission to access the resource // caller also ensure that no mutable reference to the resource exists - let (ptr, ticks, caller) = unsafe { self.get_resource_with_ticks(component_id)? }; + let (ptr, ticks, _caller) = unsafe { self.get_resource_with_ticks(component_id)? }; // SAFETY: `component_id` was obtained from the type ID of `R` let value = unsafe { ptr.deref::() }; @@ -333,11 +335,13 @@ impl<'w> UnsafeWorldCell<'w> { unsafe { Ticks::from_tick_cells(ticks, self.last_change_tick(), self.change_tick()) }; // SAFETY: caller ensures that no mutable reference to the resource exists - let caller = unsafe { caller.deref() }; + #[cfg(feature = "track_change_detection")] + let caller = unsafe { _caller.deref() }; Some(Res { value, ticks, + #[cfg(feature = "track_change_detection")] caller, }) } @@ -442,7 +446,7 @@ impl<'w> UnsafeWorldCell<'w> { ) -> Option> { // SAFETY: we only access data that the caller has ensured is unaliased and `self` // has permission to access. - let (ptr, ticks, caller) = unsafe { self.storages() } + let (ptr, ticks, _caller) = unsafe { self.storages() } .resources .get(component_id)? .get_with_ticks()?; @@ -460,10 +464,11 @@ impl<'w> UnsafeWorldCell<'w> { // - caller ensures that the resource is unaliased value: unsafe { ptr.assert_unique() }, ticks, + #[cfg(feature = "track_change_detection")] // SAFETY: // - caller ensures that `self` has permission to access the resource // - caller ensures that the resource is unaliased - caller: unsafe { caller.deref_mut() }, + caller: unsafe { _caller.deref_mut() }, }) } @@ -508,7 +513,7 @@ impl<'w> UnsafeWorldCell<'w> { let change_tick = self.change_tick(); // SAFETY: we only access data that the caller has ensured is unaliased and `self` // has permission to access. - let (ptr, ticks, caller) = unsafe { self.storages() } + let (ptr, ticks, _caller) = unsafe { self.storages() } .non_send_resources .get(component_id)? .get_with_ticks()?; @@ -523,8 +528,9 @@ impl<'w> UnsafeWorldCell<'w> { // SAFETY: This function has exclusive access to the world so nothing aliases `ptr`. value: unsafe { ptr.assert_unique() }, ticks, + #[cfg(feature = "track_change_detection")] // SAFETY: This function has exclusive access to the world - caller: unsafe { caller.deref_mut() }, + caller: unsafe { _caller.deref_mut() }, }) } @@ -538,11 +544,7 @@ impl<'w> UnsafeWorldCell<'w> { pub(crate) unsafe fn get_resource_with_ticks( self, component_id: ComponentId, - ) -> Option<( - Ptr<'w>, - TickCells<'w>, - &'w UnsafeCell<&'static Location<'static>>, - )> { + ) -> Option<(Ptr<'w>, TickCells<'w>, MaybeUnsafeCellLocation<'w>)> { // SAFETY: // - caller ensures there is no `&mut World` // - caller ensures there are no mutable borrows of this resource @@ -566,11 +568,7 @@ impl<'w> UnsafeWorldCell<'w> { pub(crate) unsafe fn get_non_send_with_ticks( self, component_id: ComponentId, - ) -> Option<( - Ptr<'w>, - TickCells<'w>, - &'w UnsafeCell<&'static Location<'static>>, - )> { + ) -> Option<(Ptr<'w>, TickCells<'w>, MaybeUnsafeCellLocation<'w>)> { // SAFETY: // - caller ensures there is no `&mut World` // - caller ensures there are no mutable borrows of this resource @@ -734,11 +732,12 @@ impl<'w> UnsafeEntityCell<'w> { self.entity, self.location, ) - .map(|(value, cells, caller)| Ref { + .map(|(value, cells, _caller)| Ref { // SAFETY: returned component is of type T value: value.deref::(), ticks: Ticks::from_tick_cells(cells, last_change_tick, change_tick), - caller: caller.deref(), + #[cfg(feature = "track_change_detection")] + caller: _caller.deref(), }) } } @@ -834,11 +833,12 @@ impl<'w> UnsafeEntityCell<'w> { self.entity, self.location, ) - .map(|(value, cells, caller)| Mut { + .map(|(value, cells, _caller)| Mut { // SAFETY: returned component is of type T value: value.assert_unique().deref_mut::(), ticks: TicksMut::from_tick_cells(cells, last_change_tick, change_tick), - caller: caller.deref_mut(), + #[cfg(feature = "track_change_detection")] + caller: _caller.deref_mut(), }) } } @@ -895,7 +895,7 @@ impl<'w> UnsafeEntityCell<'w> { self.entity, self.location, ) - .map(|(value, cells, caller)| MutUntyped { + .map(|(value, cells, _caller)| MutUntyped { // SAFETY: world access validated by caller and ties world lifetime to `MutUntyped` lifetime value: value.assert_unique(), ticks: TicksMut::from_tick_cells( @@ -903,7 +903,8 @@ impl<'w> UnsafeEntityCell<'w> { self.world.last_change_tick(), self.world.change_tick(), ), - caller: caller.deref_mut(), + #[cfg(feature = "track_change_detection")] + caller: _caller.deref_mut(), }) } } @@ -980,11 +981,7 @@ unsafe fn get_component_and_ticks( storage_type: StorageType, entity: Entity, location: EntityLocation, -) -> Option<( - Ptr<'_>, - TickCells<'_>, - &UnsafeCell<&'static Location<'static>>, -)> { +) -> Option<(Ptr<'_>, TickCells<'_>, MaybeUnsafeCellLocation<'_>)> { match storage_type { StorageType::Table => { let components = world.fetch_table(location, component_id)?; @@ -996,7 +993,10 @@ unsafe fn get_component_and_ticks( added: components.get_added_tick_unchecked(location.table_row), changed: components.get_changed_tick_unchecked(location.table_row), }, + #[cfg(feature = "track_change_detection")] components.get_caller_unchecked(location.table_row), + #[cfg(not(feature = "track_change_detection"))] + (), )) } StorageType::SparseSet => world.fetch_sparse_set(component_id)?.get_with_ticks(entity), From 34e5601a914f4bd6b77f563ec4b9ba7f742edeca Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Sat, 27 Jul 2024 00:22:24 -0400 Subject: [PATCH 5/7] fix: replaced components not being dropped --- crates/bevy_ecs/src/storage/sparse_set.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 3b10c69a5436c..a12a3242436d0 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -177,8 +177,13 @@ impl ComponentSparseSet { if let Some(&dense_index) = self.sparse.get(entity.index()) { #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); - #[cfg(feature = "track_change_detection")] - self.dense.replace(dense_index, value, change_tick, caller); + self.dense.replace( + dense_index, + value, + change_tick, + #[cfg(feature = "track_change_detection")] + caller, + ); } else { let dense_index = self.dense.len(); self.dense.push( From 0dba2c7dbe8a49a7fdeb2a45a54b33c7883ffa06 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Sat, 27 Jul 2024 00:27:00 -0400 Subject: [PATCH 6/7] chore: add `track_change_detection` feature to `bevy_internal` and `bevy` --- Cargo.toml | 3 +++ crates/bevy_internal/Cargo.toml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index fc1e10fc7b83d..b31eaa52fc28b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -338,6 +338,9 @@ ios_simulator = ["bevy_internal/ios_simulator"] # Enable built in global state machines bevy_state = ["bevy_internal/bevy_state"] +# Enables source location tracking for change detection, which can assist with debugging +track_change_detection = ["bevy_internal/track_change_detection"] + [dependencies] bevy_internal = { path = "crates/bevy_internal", version = "0.14.0-dev", default-features = false } diff --git a/crates/bevy_internal/Cargo.toml b/crates/bevy_internal/Cargo.toml index 64264595a2e0e..dec89efd4a454 100644 --- a/crates/bevy_internal/Cargo.toml +++ b/crates/bevy_internal/Cargo.toml @@ -186,6 +186,9 @@ ios_simulator = ["bevy_pbr?/ios_simulator", "bevy_render?/ios_simulator"] # Enable built in global state machines bevy_state = ["dep:bevy_state"] +# Enables source location tracking for change detection, which can assist with debugging +track_change_detection = ["bevy_ecs/track_change_detection"] + [dependencies] # bevy bevy_a11y = { path = "../bevy_a11y", version = "0.14.0-dev" } From 27518786f7cfd7d6a998a694eda30cefe1e72b67 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Sat, 27 Jul 2024 00:39:35 -0400 Subject: [PATCH 7/7] fix: change detection example --- examples/ecs/component_change_detection.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/examples/ecs/component_change_detection.rs b/examples/ecs/component_change_detection.rs index b558abc894f42..3135c75d6b277 100644 --- a/examples/ecs/component_change_detection.rs +++ b/examples/ecs/component_change_detection.rs @@ -40,11 +40,25 @@ fn change_component(time: Res