From b3561e218e563e89b723d0691534eabfe7f8c209 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 8 May 2025 15:43:40 -0400 Subject: [PATCH 01/10] use EntityRow in sparse_set --- crates/bevy_ecs/src/storage/sparse_set.rs | 34 +++++++++++------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 69305e8a141ce..5bf4101c0ece8 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -1,7 +1,7 @@ use crate::{ change_detection::MaybeLocation, component::{ComponentId, ComponentInfo, ComponentTicks, Tick, TickCells}, - entity::Entity, + entity::{Entity, EntityRow}, storage::{Column, TableRow}, }; use alloc::{boxed::Box, vec::Vec}; @@ -9,8 +9,6 @@ use bevy_ptr::{OwningPtr, Ptr}; use core::{cell::UnsafeCell, hash::Hash, marker::PhantomData, panic::Location}; use nonmax::NonMaxUsize; -type EntityIndex = u32; - #[derive(Debug)] pub(crate) struct SparseArray { values: Vec>, @@ -121,10 +119,10 @@ pub struct ComponentSparseSet { // stored for entities that are alive. The generation is not required, but is stored // in debug builds to validate that access is correct. #[cfg(not(debug_assertions))] - entities: Vec, + entities: Vec, #[cfg(debug_assertions)] entities: Vec, - sparse: SparseArray, + sparse: SparseArray, } impl ComponentSparseSet { @@ -170,7 +168,7 @@ impl ComponentSparseSet { change_tick: Tick, caller: MaybeLocation, ) { - if let Some(&dense_index) = self.sparse.get(entity.index()) { + if let Some(&dense_index) = self.sparse.get(entity.row()) { #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); self.dense.replace(dense_index, value, change_tick, caller); @@ -179,7 +177,7 @@ impl ComponentSparseSet { self.dense .push(value, ComponentTicks::new(change_tick), caller); self.sparse - .insert(entity.index(), TableRow::from_usize(dense_index)); + .insert(entity.row(), TableRow::from_usize(dense_index)); #[cfg(debug_assertions)] assert_eq!(self.entities.len(), dense_index); #[cfg(not(debug_assertions))] @@ -194,7 +192,7 @@ impl ComponentSparseSet { pub fn contains(&self, entity: Entity) -> bool { #[cfg(debug_assertions)] { - if let Some(&dense_index) = self.sparse.get(entity.index()) { + if let Some(&dense_index) = self.sparse.get(entity.row()) { #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); true @@ -211,7 +209,7 @@ impl ComponentSparseSet { /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] pub fn get(&self, entity: Entity) -> Option> { - self.sparse.get(entity.index()).map(|&dense_index| { + self.sparse.get(entity.row()).map(|&dense_index| { #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); // SAFETY: if the sparse index points to something in the dense vec, it exists @@ -231,7 +229,7 @@ impl ComponentSparseSet { TickCells<'_>, MaybeLocation<&UnsafeCell<&'static Location<'static>>>, )> { - let dense_index = *self.sparse.get(entity.index())?; + let dense_index = *self.sparse.get(entity.row())?; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); // SAFETY: if the sparse index points to something in the dense vec, it exists @@ -252,7 +250,7 @@ impl ComponentSparseSet { /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] pub fn get_added_tick(&self, entity: Entity) -> Option<&UnsafeCell> { - let dense_index = *self.sparse.get(entity.index())?; + let dense_index = *self.sparse.get(entity.row())?; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); // SAFETY: if the sparse index points to something in the dense vec, it exists @@ -264,7 +262,7 @@ impl ComponentSparseSet { /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] pub fn get_changed_tick(&self, entity: Entity) -> Option<&UnsafeCell> { - let dense_index = *self.sparse.get(entity.index())?; + let dense_index = *self.sparse.get(entity.row())?; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); // SAFETY: if the sparse index points to something in the dense vec, it exists @@ -276,7 +274,7 @@ impl ComponentSparseSet { /// Returns `None` if `entity` does not have a component in the sparse set. #[inline] pub fn get_ticks(&self, entity: Entity) -> Option { - let dense_index = *self.sparse.get(entity.index())?; + let dense_index = *self.sparse.get(entity.row())?; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); // SAFETY: if the sparse index points to something in the dense vec, it exists @@ -292,7 +290,7 @@ impl ComponentSparseSet { entity: Entity, ) -> MaybeLocation>>> { MaybeLocation::new_with_flattened(|| { - let dense_index = *self.sparse.get(entity.index())?; + let dense_index = *self.sparse.get(entity.row())?; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); // SAFETY: if the sparse index points to something in the dense vec, it exists @@ -311,7 +309,7 @@ impl ComponentSparseSet { /// it exists). #[must_use = "The returned pointer must be used to drop the removed component."] pub(crate) fn remove_and_forget(&mut self, entity: Entity) -> Option> { - self.sparse.remove(entity.index()).map(|dense_index| { + self.sparse.remove(entity.row()).map(|dense_index| { #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); self.entities.swap_remove(dense_index.as_usize()); @@ -323,7 +321,7 @@ impl ComponentSparseSet { #[cfg(not(debug_assertions))] let index = swapped_entity; #[cfg(debug_assertions)] - let index = swapped_entity.index(); + let index = swapped_entity.row(); *self.sparse.get_mut(index).unwrap() = dense_index; } value @@ -334,7 +332,7 @@ impl ComponentSparseSet { /// /// Returns `true` if `entity` had a component value in the sparse set. pub(crate) fn remove(&mut self, entity: Entity) -> bool { - if let Some(dense_index) = self.sparse.remove(entity.index()) { + if let Some(dense_index) = self.sparse.remove(entity.row()) { #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index.as_usize()]); self.entities.swap_remove(dense_index.as_usize()); @@ -348,7 +346,7 @@ impl ComponentSparseSet { #[cfg(not(debug_assertions))] let index = swapped_entity; #[cfg(debug_assertions)] - let index = swapped_entity.index(); + let index = swapped_entity.row(); *self.sparse.get_mut(index).unwrap() = dense_index; } true From 984b99e16022292256276116b437aacd5d8827fd Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 8 May 2025 16:35:08 -0400 Subject: [PATCH 02/10] Make TableRow nonmax --- crates/bevy_ecs/src/archetype.rs | 4 +- crates/bevy_ecs/src/batching.rs | 2 +- crates/bevy_ecs/src/query/fetch.rs | 26 +++-- crates/bevy_ecs/src/query/iter.rs | 61 +++++------ crates/bevy_ecs/src/query/par_iter.rs | 19 ++-- crates/bevy_ecs/src/query/state.rs | 14 +-- crates/bevy_ecs/src/storage/sparse_set.rs | 16 ++- crates/bevy_ecs/src/storage/table/mod.rs | 120 ++++++++++++---------- 8 files changed, 147 insertions(+), 115 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 2468c5e9a8fbf..a26ef3fb30a9f 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -606,8 +606,8 @@ impl Archetype { /// Gets the total number of entities that belong to the archetype. #[inline] - pub fn len(&self) -> usize { - self.entities.len() + pub fn len(&self) -> u32 { + self.entities.len() as u32 } /// Checks if the archetype has any entities. diff --git a/crates/bevy_ecs/src/batching.rs b/crates/bevy_ecs/src/batching.rs index cdffbfa05dd36..ab9f2d582c781 100644 --- a/crates/bevy_ecs/src/batching.rs +++ b/crates/bevy_ecs/src/batching.rs @@ -22,7 +22,7 @@ use core::ops::Range; /// [`EventReader::par_read`]: crate::event::EventReader::par_read #[derive(Clone, Debug)] pub struct BatchingStrategy { - /// The upper and lower limits for a batch of entities. + /// The upper and lower limits for a batch of items. /// /// Setting the bounds to the same value will result in a fixed /// batch size. diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 3c1ff5262cd15..3486087595aa8 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -518,7 +518,7 @@ unsafe impl ReadOnlyQueryData for EntityLocation {} /// match spawn_details.spawned_by().into_option() { /// Some(location) => println!(" by {:?}", location), /// None => println!() -/// } +/// } /// } /// } /// @@ -1617,11 +1617,15 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { ) { let column = table.get_column(component_id).debug_checked_unwrap(); let table_data = Some(( - column.get_data_slice(table.entity_count()).into(), - column.get_added_ticks_slice(table.entity_count()).into(), - column.get_changed_ticks_slice(table.entity_count()).into(), + column.get_data_slice(table.entity_count() as usize).into(), + column + .get_added_ticks_slice(table.entity_count() as usize) + .into(), + column + .get_changed_ticks_slice(table.entity_count() as usize) + .into(), column - .get_changed_by_slice(table.entity_count()) + .get_changed_by_slice(table.entity_count() as usize) .map(Into::into), )); // SAFETY: set_table is only called when T::STORAGE_TYPE = StorageType::Table @@ -1812,11 +1816,15 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { ) { let column = table.get_column(component_id).debug_checked_unwrap(); let table_data = Some(( - column.get_data_slice(table.entity_count()).into(), - column.get_added_ticks_slice(table.entity_count()).into(), - column.get_changed_ticks_slice(table.entity_count()).into(), + column.get_data_slice(table.entity_count() as usize).into(), + column + .get_added_ticks_slice(table.entity_count() as usize) + .into(), + column + .get_changed_ticks_slice(table.entity_count() as usize) + .into(), column - .get_changed_by_slice(table.entity_count()) + .get_changed_by_slice(table.entity_count() as usize) .map(Into::into), )); // SAFETY: set_table is only called when T::STORAGE_TYPE = StorageType::Table diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index fc89843493a03..6ca4f48acd858 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -19,6 +19,7 @@ use core::{ mem::MaybeUninit, ops::Range, }; +use nonmax::NonMaxU32; /// An [`Iterator`] over query results of a [`Query`](crate::system::Query). /// @@ -136,7 +137,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { mut accum: B, func: &mut Func, storage: StorageId, - range: Option>, + range: Option>, ) -> B where Func: FnMut(B, D::Item<'w>) -> B, @@ -199,7 +200,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { mut accum: B, func: &mut Func, table: &'w Table, - rows: Range, + rows: Range, ) -> B where Func: FnMut(B, D::Item<'w>) -> B, @@ -207,10 +208,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { if table.is_empty() { return accum; } - debug_assert!( - rows.end <= u32::MAX as usize, - "TableRow is only valid up to u32::MAX" - ); D::set_table(&mut self.cursor.fetch, &self.query_state.fetch_state, table); F::set_table( @@ -222,8 +219,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { let entities = table.entities(); for row in rows { // SAFETY: Caller assures `row` in range of the current archetype. - let entity = unsafe { entities.get_unchecked(row) }; - let row = TableRow::from_usize(row); + let entity = unsafe { entities.get_unchecked(row as usize) }; + // SAFETY: This is from an exclusive range, so it can't be max. + let row = unsafe { TableRow::from_u32(NonMaxU32::new_unchecked(row)) }; // SAFETY: set_table was called prior. // Caller assures `row` in range of the current archetype. @@ -254,7 +252,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { mut accum: B, func: &mut Func, archetype: &'w Archetype, - indices: Range, + indices: Range, ) -> B where Func: FnMut(B, D::Item<'w>) -> B, @@ -279,7 +277,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { let entities = archetype.entities(); for index in indices { // SAFETY: Caller assures `index` in range of the current archetype. - let archetype_entity = unsafe { entities.get_unchecked(index) }; + let archetype_entity = unsafe { entities.get_unchecked(index as usize) }; // SAFETY: set_archetype was called prior. // Caller assures `index` in range of the current archetype. @@ -323,7 +321,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { mut accum: B, func: &mut Func, archetype: &'w Archetype, - rows: Range, + rows: Range, ) -> B where Func: FnMut(B, D::Item<'w>) -> B, @@ -331,10 +329,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { if archetype.is_empty() { return accum; } - debug_assert!( - rows.end <= u32::MAX as usize, - "TableRow is only valid up to u32::MAX" - ); let table = self.tables.get(archetype.table_id()).debug_checked_unwrap(); debug_assert!( archetype.len() == table.entity_count(), @@ -356,8 +350,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { let entities = table.entities(); for row in rows { // SAFETY: Caller assures `row` in range of the current archetype. - let entity = unsafe { *entities.get_unchecked(row) }; - let row = TableRow::from_usize(row); + let entity = unsafe { *entities.get_unchecked(row as usize) }; + // SAFETY: This is from an exclusive range, so it can't be max. + let row = unsafe { TableRow::from_u32(NonMaxU32::new_unchecked(row)) }; // SAFETY: set_table was called prior. // Caller assures `row` in range of the current archetype. @@ -895,7 +890,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Iterator for QueryIter<'w, 's, D, F> let max_size = self.cursor.max_remaining(self.tables, self.archetypes); let archetype_query = F::IS_ARCHETYPAL; let min_size = if archetype_query { max_size } else { 0 }; - (min_size, Some(max_size)) + (min_size as usize, Some(max_size as usize)) } #[inline] @@ -2275,7 +2270,7 @@ impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter, const K: usize> Iterator .enumerate() .try_fold(0, |acc, (i, cursor)| { let n = cursor.max_remaining(self.tables, self.archetypes); - Some(acc + choose(n, K - i)?) + Some(acc + choose(n as usize, K - i)?) }); let archetype_query = F::IS_ARCHETYPAL; @@ -2317,9 +2312,9 @@ struct QueryIterationCursor<'w, 's, D: QueryData, F: QueryFilter> { fetch: D::Fetch<'w>, filter: F::Fetch<'w>, // length of the table or length of the archetype, depending on whether both `D`'s and `F`'s fetches are dense - current_len: usize, + current_len: u32, // either table row or archetype index, depending on whether both `D`'s and `F`'s fetches are dense - current_row: usize, + current_row: u32, } impl Clone for QueryIterationCursor<'_, '_, D, F> { @@ -2400,7 +2395,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { let index = self.current_row - 1; if self.is_dense { // SAFETY: This must have been called previously in `next` as `current_row > 0` - let entity = unsafe { self.table_entities.get_unchecked(index) }; + let entity = unsafe { self.table_entities.get_unchecked(index as usize) }; // SAFETY: // - `set_table` must have been called previously either in `next` or before it. // - `*entity` and `index` are in the current table. @@ -2408,12 +2403,14 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { Some(D::fetch( &mut self.fetch, *entity, - TableRow::from_usize(index), + // SAFETY: This is from an exclusive range, so it can't be max. + TableRow::from_u32(NonMaxU32::new_unchecked(index)), )) } } else { // SAFETY: This must have been called previously in `next` as `current_row > 0` - let archetype_entity = unsafe { self.archetype_entities.get_unchecked(index) }; + let archetype_entity = + unsafe { self.archetype_entities.get_unchecked(index as usize) }; // SAFETY: // - `set_archetype` must have been called previously either in `next` or before it. // - `archetype_entity.id()` and `archetype_entity.table_row()` are in the current archetype. @@ -2434,9 +2431,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { /// /// Note that if `F::IS_ARCHETYPAL`, the return value /// will be **the exact count of remaining values**. - fn max_remaining(&self, tables: &'w Tables, archetypes: &'w Archetypes) -> usize { + fn max_remaining(&self, tables: &'w Tables, archetypes: &'w Archetypes) -> u32 { let ids = self.storage_id_iter.clone(); - let remaining_matched: usize = if self.is_dense { + let remaining_matched: u32 = if self.is_dense { // SAFETY: The if check ensures that storage_id_iter stores TableIds unsafe { ids.map(|id| tables[id.table_id].entity_count()).sum() } } else { @@ -2483,8 +2480,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // SAFETY: set_table was called prior. // `current_row` is a table row in range of the current table, because if it was not, then the above would have been executed. - let entity = unsafe { self.table_entities.get_unchecked(self.current_row) }; - let row = TableRow::from_usize(self.current_row); + let entity = + unsafe { self.table_entities.get_unchecked(self.current_row as usize) }; + // SAFETY: The row is less than the u32 len, so it must not be max. + let row = unsafe { TableRow::from_u32(NonMaxU32::new_unchecked(self.current_row)) }; if !F::filter_fetch(&mut self.filter, *entity, row) { self.current_row += 1; continue; @@ -2532,8 +2531,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // SAFETY: set_archetype was called prior. // `current_row` is an archetype index row in range of the current archetype, because if it was not, then the if above would have been executed. - let archetype_entity = - unsafe { self.archetype_entities.get_unchecked(self.current_row) }; + let archetype_entity = unsafe { + self.archetype_entities + .get_unchecked(self.current_row as usize) + }; if !F::filter_fetch( &mut self.filter, archetype_entity.id(), diff --git a/crates/bevy_ecs/src/query/par_iter.rs b/crates/bevy_ecs/src/query/par_iter.rs index 6683238aa36ee..c685659260fe6 100644 --- a/crates/bevy_ecs/src/query/par_iter.rs +++ b/crates/bevy_ecs/src/query/par_iter.rs @@ -62,7 +62,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryParIter<'w, 's, D, F> { /// query.par_iter().for_each_init(|| queue.borrow_local_mut(),|local_queue, item| { /// **local_queue += 1; /// }); - /// + /// /// // collect value from every thread /// let entity_count: usize = queue.iter_mut().map(|v| *v).sum(); /// } @@ -130,7 +130,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryParIter<'w, 's, D, F> { } #[cfg(all(not(target_arch = "wasm32"), feature = "multi_threaded"))] - fn get_batch_size(&self, thread_count: usize) -> usize { + fn get_batch_size(&self, thread_count: usize) -> u32 { let max_items = || { let id_iter = self.state.matched_storage_ids.iter(); if self.state.is_dense { @@ -147,10 +147,11 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryParIter<'w, 's, D, F> { .map(|id| unsafe { archetypes[id.archetype_id].len() }) .max() } + .map(|v| v as usize) .unwrap_or(0) }; self.batching_strategy - .calc_batch_size(max_items, thread_count) + .calc_batch_size(max_items, thread_count) as u32 } } @@ -232,7 +233,7 @@ impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter, E: EntityEquivalent + Sync> /// query.par_iter_many(&entities).for_each_init(|| queue.borrow_local_mut(),|local_queue, item| { /// **local_queue += some_expensive_operation(item); /// }); - /// + /// /// // collect value from every thread /// let final_value: usize = queue.iter_mut().map(|v| *v).sum(); /// } @@ -301,9 +302,9 @@ impl<'w, 's, D: ReadOnlyQueryData, F: QueryFilter, E: EntityEquivalent + Sync> } #[cfg(all(not(target_arch = "wasm32"), feature = "multi_threaded"))] - fn get_batch_size(&self, thread_count: usize) -> usize { + fn get_batch_size(&self, thread_count: usize) -> u32 { self.batching_strategy - .calc_batch_size(|| self.entity_list.len(), thread_count) + .calc_batch_size(|| self.entity_list.len(), thread_count) as u32 } } @@ -387,7 +388,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, E: EntityEquivalent + Sync> /// query.par_iter_many_unique(&entities).for_each_init(|| queue.borrow_local_mut(),|local_queue, item| { /// **local_queue += some_expensive_operation(item); /// }); - /// + /// /// // collect value from every thread /// let final_value: usize = queue.iter_mut().map(|v| *v).sum(); /// } @@ -456,8 +457,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter, E: EntityEquivalent + Sync> } #[cfg(all(not(target_arch = "wasm32"), feature = "multi_threaded"))] - fn get_batch_size(&self, thread_count: usize) -> usize { + fn get_batch_size(&self, thread_count: usize) -> u32 { self.batching_strategy - .calc_batch_size(|| self.entity_list.len(), thread_count) + .calc_batch_size(|| self.entity_list.len(), thread_count) as u32 } } diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index cae8e592e757e..8c74d1a018b4f 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1502,7 +1502,7 @@ impl QueryState { &self, init_accum: INIT, world: UnsafeWorldCell<'w>, - batch_size: usize, + batch_size: u32, func: FN, last_run: Tick, this_run: Tick, @@ -1545,7 +1545,7 @@ impl QueryState { // submit single storage larger than batch_size let submit_single = |count, storage_id: StorageId| { - for offset in (0..count).step_by(batch_size) { + for offset in (0..count).step_by(batch_size as usize) { let mut func = func.clone(); let init_accum = init_accum.clone(); let len = batch_size.min(count - offset); @@ -1561,7 +1561,7 @@ impl QueryState { } }; - let storage_entity_count = |storage_id: StorageId| -> usize { + let storage_entity_count = |storage_id: StorageId| -> u32 { if self.is_dense { tables[storage_id.table_id].entity_count() } else { @@ -1617,7 +1617,7 @@ impl QueryState { init_accum: INIT, world: UnsafeWorldCell<'w>, entity_list: &UniqueEntityEquivalentSlice, - batch_size: usize, + batch_size: u32, mut func: FN, last_run: Tick, this_run: Tick, @@ -1631,7 +1631,7 @@ impl QueryState { // QueryState::par_many_fold_init_unchecked_manual, QueryState::par_many_unique_fold_init_unchecked_manual bevy_tasks::ComputeTaskPool::get().scope(|scope| { - let chunks = entity_list.chunks_exact(batch_size); + let chunks = entity_list.chunks_exact(batch_size as usize); let remainder = chunks.remainder(); for batch in chunks { @@ -1680,7 +1680,7 @@ impl QueryState { init_accum: INIT, world: UnsafeWorldCell<'w>, entity_list: &[E], - batch_size: usize, + batch_size: u32, mut func: FN, last_run: Tick, this_run: Tick, @@ -1694,7 +1694,7 @@ impl QueryState { // QueryState::par_many_fold_init_unchecked_manual, QueryState::par_many_unique_fold_init_unchecked_manual bevy_tasks::ComputeTaskPool::get().scope(|scope| { - let chunks = entity_list.chunks_exact(batch_size); + let chunks = entity_list.chunks_exact(batch_size as usize); let remainder = chunks.remainder(); for batch in chunks { diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 5bf4101c0ece8..e70a24bb3a65e 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -7,7 +7,7 @@ use crate::{ use alloc::{boxed::Box, vec::Vec}; use bevy_ptr::{OwningPtr, Ptr}; use core::{cell::UnsafeCell, hash::Hash, marker::PhantomData, panic::Location}; -use nonmax::NonMaxUsize; +use nonmax::{NonMaxU32, NonMaxUsize}; #[derive(Debug)] pub(crate) struct SparseArray { @@ -176,12 +176,18 @@ impl ComponentSparseSet { let dense_index = self.dense.len(); self.dense .push(value, ComponentTicks::new(change_tick), caller); - self.sparse - .insert(entity.row(), TableRow::from_usize(dense_index)); + + // SAFETY: This entity row does not exist here yet, so there are no duplicates, + // and the entity index can not be the max, so the length must not be max either. + // To do so would have caused a panic in the entity alloxator. + let table_row = + unsafe { TableRow::from_u32(NonMaxU32::new_unchecked(dense_index as u32)) }; + + self.sparse.insert(entity.row(), table_row); #[cfg(debug_assertions)] assert_eq!(self.entities.len(), dense_index); #[cfg(not(debug_assertions))] - self.entities.push(entity.index()); + self.entities.push(entity.row()); #[cfg(debug_assertions)] self.entities.push(entity); } @@ -201,7 +207,7 @@ impl ComponentSparseSet { } } #[cfg(not(debug_assertions))] - self.sparse.contains(entity.index()) + self.sparse.contains(entity.row()) } /// Returns a reference to the entity's component value. diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index fd8eb410e4f33..8ceeb6ea5e478 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -16,6 +16,7 @@ use core::{ ops::{Index, IndexMut}, panic::Location, }; +use nonmax::NonMaxU32; mod column; /// An opaque unique ID for a [`Table`] within a [`World`]. @@ -100,39 +101,29 @@ impl TableId { /// [`Archetype::entity_table_row`]: crate::archetype::Archetype::entity_table_row /// [`Archetype::table_id`]: crate::archetype::Archetype::table_id #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct TableRow(u32); +#[repr(transparent)] +pub struct TableRow(NonMaxU32); impl TableRow { - pub(crate) const INVALID: TableRow = TableRow(u32::MAX); + pub(crate) const INVALID: TableRow = TableRow(NonMaxU32::MAX); - /// Creates a `TableRow`. + /// Creates a [`TableRow`]. #[inline] - pub const fn from_u32(index: u32) -> Self { + pub const fn from_u32(index: NonMaxU32) -> Self { Self(index) } - /// Creates a `TableRow` from a [`usize`] index. - /// - /// # Panics - /// - /// Will panic in debug mode if the provided value does not fit within a [`u32`]. - #[inline] - pub const fn from_usize(index: usize) -> Self { - debug_assert!(index as u32 as usize == index); - Self(index as u32) - } - /// Gets the index of the row as a [`usize`]. #[inline] pub const fn as_usize(self) -> usize { // usize is at least u32 in Bevy - self.0 as usize + self.0.get() as usize } /// Gets the index of the row as a [`usize`]. #[inline] pub const fn as_u32(self) -> u32 { - self.0 + self.0.get() } } @@ -225,9 +216,9 @@ impl Table { /// # Safety /// `row` must be in-bounds (`row.as_usize()` < `self.len()`) pub(crate) unsafe fn swap_remove_unchecked(&mut self, row: TableRow) -> Option { - debug_assert!(row.as_usize() < self.entity_count()); + debug_assert!(row.as_u32() < self.entity_count()); let last_element_index = self.entity_count() - 1; - if row.as_usize() != last_element_index { + if row.as_u32() != last_element_index { // Instead of checking this condition on every `swap_remove` call, we // check it here and use `swap_remove_nonoverlapping`. for col in self.columns.values_mut() { @@ -237,22 +228,26 @@ impl Table { // - `row` != `last_element_index` // - the `len` is kept within `self.entities`, it will update accordingly. unsafe { - col.swap_remove_and_drop_unchecked_nonoverlapping(last_element_index, row); + col.swap_remove_and_drop_unchecked_nonoverlapping( + last_element_index as usize, + row, + ); }; } } else { // If `row.as_usize()` == `last_element_index` than there's no point in removing the component // at `row`, but we still need to drop it. for col in self.columns.values_mut() { - col.drop_last_component(last_element_index); + col.drop_last_component(last_element_index as usize); } } - let is_last = row.as_usize() == last_element_index; + let is_last = row.as_u32() == last_element_index; self.entities.swap_remove(row.as_usize()); if is_last { None } else { - Some(self.entities[row.as_usize()]) + // SAFETY: This was sawp removed and was not last, so it must be in bounds. + unsafe { Some(*self.entities.get_unchecked(row.as_usize())) } } } @@ -269,16 +264,21 @@ impl Table { row: TableRow, new_table: &mut Table, ) -> TableMoveResult { - debug_assert!(row.as_usize() < self.entity_count()); + debug_assert!(row.as_u32() < self.entity_count()); let last_element_index = self.entity_count() - 1; - let is_last = row.as_usize() == last_element_index; + let is_last = row.as_u32() == last_element_index; let new_row = new_table.allocate(self.entities.swap_remove(row.as_usize())); for (component_id, column) in self.columns.iter_mut() { if let Some(new_column) = new_table.get_column_mut(*component_id) { - new_column.initialize_from_unchecked(column, last_element_index, row, new_row); + new_column.initialize_from_unchecked( + column, + last_element_index as usize, + row, + new_row, + ); } else { // It's the caller's responsibility to drop these cases. - column.swap_remove_and_forget_unchecked(last_element_index, row); + column.swap_remove_and_forget_unchecked(last_element_index as usize, row); } } TableMoveResult { @@ -286,7 +286,8 @@ impl Table { swapped_entity: if is_last { None } else { - Some(self.entities[row.as_usize()]) + // SAFETY: This was sawp removed and was not last, so it must be in bounds. + unsafe { Some(*self.entities.get_unchecked(row.as_usize())) } }, } } @@ -302,15 +303,20 @@ impl Table { row: TableRow, new_table: &mut Table, ) -> TableMoveResult { - debug_assert!(row.as_usize() < self.entity_count()); + debug_assert!(row.as_u32() < self.entity_count()); let last_element_index = self.entity_count() - 1; - let is_last = row.as_usize() == last_element_index; + let is_last = row.as_u32() == last_element_index; let new_row = new_table.allocate(self.entities.swap_remove(row.as_usize())); for (component_id, column) in self.columns.iter_mut() { if let Some(new_column) = new_table.get_column_mut(*component_id) { - new_column.initialize_from_unchecked(column, last_element_index, row, new_row); + new_column.initialize_from_unchecked( + column, + last_element_index as usize, + row, + new_row, + ); } else { - column.swap_remove_and_drop_unchecked(last_element_index, row); + column.swap_remove_and_drop_unchecked(last_element_index as usize, row); } } TableMoveResult { @@ -318,7 +324,8 @@ impl Table { swapped_entity: if is_last { None } else { - Some(self.entities[row.as_usize()]) + // SAFETY: This was sawp removed and was not last, so it must be in bounds. + unsafe { Some(*self.entities.get_unchecked(row.as_usize())) } }, } } @@ -335,22 +342,23 @@ impl Table { row: TableRow, new_table: &mut Table, ) -> TableMoveResult { - debug_assert!(row.as_usize() < self.entity_count()); + debug_assert!(row.as_u32() < self.entity_count()); let last_element_index = self.entity_count() - 1; - let is_last = row.as_usize() == last_element_index; + let is_last = row.as_u32() == last_element_index; let new_row = new_table.allocate(self.entities.swap_remove(row.as_usize())); for (component_id, column) in self.columns.iter_mut() { new_table .get_column_mut(*component_id) .debug_checked_unwrap() - .initialize_from_unchecked(column, last_element_index, row, new_row); + .initialize_from_unchecked(column, last_element_index as usize, row, new_row); } TableMoveResult { new_row, swapped_entity: if is_last { None } else { - Some(self.entities[row.as_usize()]) + // SAFETY: This was sawp removed and was not last, so it must be in bounds. + unsafe { Some(*self.entities.get_unchecked(row.as_usize())) } }, } } @@ -365,7 +373,7 @@ impl Table { component_id: ComponentId, ) -> Option<&[UnsafeCell]> { self.get_column(component_id) - .map(|col| col.get_data_slice(self.entity_count())) + .map(|col| col.get_data_slice(self.entity_count() as usize)) } /// Get the added ticks of the column matching `component_id` as a slice. @@ -375,7 +383,7 @@ impl Table { ) -> Option<&[UnsafeCell]> { self.get_column(component_id) // SAFETY: `self.len()` is guaranteed to be the len of the ticks array - .map(|col| unsafe { col.get_added_ticks_slice(self.entity_count()) }) + .map(|col| unsafe { col.get_added_ticks_slice(self.entity_count() as usize) }) } /// Get the changed ticks of the column matching `component_id` as a slice. @@ -385,7 +393,7 @@ impl Table { ) -> Option<&[UnsafeCell]> { self.get_column(component_id) // SAFETY: `self.len()` is guaranteed to be the len of the ticks array - .map(|col| unsafe { col.get_changed_ticks_slice(self.entity_count()) }) + .map(|col| unsafe { col.get_changed_ticks_slice(self.entity_count() as usize) }) } /// Fetches the calling locations that last changed the each component @@ -396,7 +404,7 @@ impl Table { MaybeLocation::new_with_flattened(|| { self.get_column(component_id) // SAFETY: `self.len()` is guaranteed to be the len of the locations array - .map(|col| unsafe { col.get_changed_by_slice(self.entity_count()) }) + .map(|col| unsafe { col.get_changed_by_slice(self.entity_count() as usize) }) }) } @@ -406,7 +414,7 @@ impl Table { component_id: ComponentId, row: TableRow, ) -> Option<&UnsafeCell> { - (row.as_usize() < self.entity_count()).then_some( + (row.as_u32() < self.entity_count()).then_some( // SAFETY: `row.as_usize()` < `len` unsafe { self.get_column(component_id)? @@ -422,7 +430,7 @@ impl Table { component_id: ComponentId, row: TableRow, ) -> Option<&UnsafeCell> { - (row.as_usize() < self.entity_count()).then_some( + (row.as_u32() < self.entity_count()).then_some( // SAFETY: `row.as_usize()` < `len` unsafe { self.get_column(component_id)? @@ -439,7 +447,7 @@ impl Table { row: TableRow, ) -> MaybeLocation>>> { MaybeLocation::new_with_flattened(|| { - (row.as_usize() < self.entity_count()).then_some( + (row.as_u32() < self.entity_count()).then_some( // SAFETY: `row.as_usize()` < `len` unsafe { self.get_column(component_id)? @@ -499,7 +507,7 @@ impl Table { /// Reserves `additional` elements worth of capacity within the table. pub(crate) fn reserve(&mut self, additional: usize) { - if self.capacity() - self.entity_count() < additional { + if (self.capacity() - self.entity_count() as usize) < additional { let column_cap = self.capacity(); self.entities.reserve(additional); @@ -563,10 +571,15 @@ impl Table { /// Allocates space for a new entity /// /// # Safety - /// the allocated row must be written to immediately with valid values in each column + /// + /// The allocated row must be written to immediately with valid values in each column pub(crate) unsafe fn allocate(&mut self, entity: Entity) -> TableRow { self.reserve(1); let len = self.entity_count(); + // SAFETY: No entity row may be in more than one table row at once, so there are no duplicates, + // and there can not be an entity row of u32::MAX. Therefore, this can not be max either. + let row = unsafe { TableRow::from_u32(NonMaxU32::new_unchecked(len)) }; + let len = len as usize; self.entities.push(entity); for col in self.columns.values_mut() { col.added_ticks @@ -580,13 +593,16 @@ impl Table { changed_by.initialize_unchecked(len, UnsafeCell::new(caller)); }); } - TableRow::from_usize(len) + + row } /// Gets the number of entities currently being stored in the table. #[inline] - pub fn entity_count(&self) -> usize { - self.entities.len() + pub fn entity_count(&self) -> u32 { + // No entity may have more than one table row, so there are no duplicates, + // and there may only ever be u32::MAX entities, so the length never exceeds u32's cappacity. + self.entities.len() as u32 } /// Get the drop function for some component that is stored in this table. @@ -618,7 +634,7 @@ impl Table { /// Call [`Tick::check_tick`] on all of the ticks in the [`Table`] pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) { - let len = self.entity_count(); + let len = self.entity_count() as usize; for col in self.columns.values_mut() { // SAFETY: `len` is the actual length of the column unsafe { col.check_change_ticks(len, change_tick) }; @@ -632,7 +648,7 @@ impl Table { /// Clears all of the stored components in the [`Table`]. pub(crate) fn clear(&mut self) { - let len = self.entity_count(); + let len = self.entity_count() as usize; // We must clear the entities first, because in the drop function causes a panic, it will result in a double free of the columns. self.entities.clear(); for column in self.columns.values_mut() { @@ -806,7 +822,7 @@ impl IndexMut for Tables { impl Drop for Table { fn drop(&mut self) { - let len = self.entity_count(); + let len = self.entity_count() as usize; let cap = self.capacity(); self.entities.clear(); for col in self.columns.values_mut() { From f2bdd9d7e812ecfe8bdcc7a12c1ee7b5ce25a71b Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 8 May 2025 16:45:37 -0400 Subject: [PATCH 03/10] make archetype row non max --- crates/bevy_ecs/src/archetype.rs | 37 ++++++++++++++++++++++++++------ crates/bevy_ecs/src/world/mod.rs | 30 +++++--------------------- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index a26ef3fb30a9f..8eae8e2a9014e 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -32,6 +32,7 @@ use core::{ hash::Hash, ops::{Index, IndexMut, RangeFrom}, }; +use nonmax::NonMaxU32; /// An opaque location within a [`Archetype`]. /// @@ -44,23 +45,23 @@ use core::{ #[derive(Debug, Copy, Clone, Eq, PartialEq)] // SAFETY: Must be repr(transparent) due to the safety requirements on EntityLocation #[repr(transparent)] -pub struct ArchetypeRow(u32); +pub struct ArchetypeRow(NonMaxU32); impl ArchetypeRow { /// Index indicating an invalid archetype row. /// This is meant to be used as a placeholder. - pub const INVALID: ArchetypeRow = ArchetypeRow(u32::MAX); + pub const INVALID: ArchetypeRow = ArchetypeRow(NonMaxU32::MAX); /// Creates a `ArchetypeRow`. #[inline] - pub const fn new(index: usize) -> Self { - Self(index as u32) + pub const fn new(index: NonMaxU32) -> Self { + Self(index) } /// Gets the index of the row. #[inline] pub const fn index(self) -> usize { - self.0 as usize + self.0.get() as usize } } @@ -467,6 +468,27 @@ impl Archetype { &self.entities } + /// Fetches the entities contained in this archetype. + #[inline] + pub fn entities_with_location(&self) -> impl Iterator { + self.entities.iter().enumerate().map( + |(archetype_row, &ArchetypeEntity { entity, table_row })| { + ( + entity, + EntityLocation { + archetype_id: self.id, + // SAFETY: The entities in the archetype must be unique and there are never more than u32::MAX entities. + archetype_row: unsafe { + ArchetypeRow::new(NonMaxU32::new_unchecked(archetype_row as u32)) + }, + table_id: self.table_id, + table_row, + }, + ) + }, + ) + } + /// Gets an iterator of all of the components stored in [`Table`]s. /// /// All of the IDs are unique. @@ -569,7 +591,8 @@ impl Archetype { entity: Entity, table_row: TableRow, ) -> EntityLocation { - let archetype_row = ArchetypeRow::new(self.entities.len()); + // SAFETY: An entity can not have multiple archetype rows and there can not be more than u32::MAX entities. + let archetype_row = unsafe { ArchetypeRow::new(NonMaxU32::new_unchecked(self.len())) }; self.entities.push(ArchetypeEntity { entity, table_row }); EntityLocation { @@ -607,6 +630,8 @@ impl Archetype { /// Gets the total number of entities that belong to the archetype. #[inline] pub fn len(&self) -> u32 { + // No entity may have more than one archetype row, so there are no duplicates, + // and there may only ever be u32::MAX entities, so the length never exceeds u32's cappacity. self.entities.len() as u32 } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 87340551af477..e8db530727862 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -31,7 +31,7 @@ pub use identifier::WorldId; pub use spawn_batch::*; use crate::{ - archetype::{ArchetypeId, ArchetypeRow, Archetypes}, + archetype::{ArchetypeId, Archetypes}, bundle::{ Bundle, BundleEffect, BundleInfo, BundleInserter, BundleSpawner, Bundles, InsertMode, NoBundleEffect, @@ -959,18 +959,8 @@ impl World { pub fn iter_entities(&self) -> impl Iterator> + '_ { self.archetypes.iter().flat_map(|archetype| { archetype - .entities() - .iter() - .enumerate() - .map(|(archetype_row, archetype_entity)| { - let entity = archetype_entity.id(); - let location = EntityLocation { - archetype_id: archetype.id(), - archetype_row: ArchetypeRow::new(archetype_row), - table_id: archetype.table_id(), - table_row: archetype_entity.table_row(), - }; - + .entities_with_location() + .map(|(entity, location)| { // SAFETY: entity exists and location accurately specifies the archetype where the entity is stored. let cell = UnsafeEntityCell::new( self.as_unsafe_world_cell_readonly(), @@ -992,18 +982,8 @@ impl World { let world_cell = self.as_unsafe_world_cell(); world_cell.archetypes().iter().flat_map(move |archetype| { archetype - .entities() - .iter() - .enumerate() - .map(move |(archetype_row, archetype_entity)| { - let entity = archetype_entity.id(); - let location = EntityLocation { - archetype_id: archetype.id(), - archetype_row: ArchetypeRow::new(archetype_row), - table_id: archetype.table_id(), - table_row: archetype_entity.table_row(), - }; - + .entities_with_location() + .map(move |(entity, location)| { // SAFETY: entity exists and location accurately specifies the archetype where the entity is stored. let cell = UnsafeEntityCell::new( world_cell, From 67a468bd76b9ebcc226d47c2421037be1d7d9476 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 8 May 2025 17:15:31 -0400 Subject: [PATCH 04/10] consistent naming between table and archetype row --- crates/bevy_ecs/src/query/fetch.rs | 18 +-- crates/bevy_ecs/src/query/filter.rs | 4 +- crates/bevy_ecs/src/query/iter.rs | 8 +- crates/bevy_ecs/src/storage/sparse_set.rs | 35 +++--- crates/bevy_ecs/src/storage/table/column.rs | 126 ++++++++++---------- crates/bevy_ecs/src/storage/table/mod.rs | 62 +++++----- 6 files changed, 126 insertions(+), 127 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 3486087595aa8..e3d17cdcadcd2 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1505,7 +1505,7 @@ unsafe impl QueryData for &T { // SAFETY: set_table was previously called let table = unsafe { table.debug_checked_unwrap() }; // SAFETY: Caller ensures `table_row` is in range. - let item = unsafe { table.get(table_row.as_usize()) }; + let item = unsafe { table.get(table_row.index()) }; item.deref() }, |sparse_set| { @@ -1683,13 +1683,13 @@ unsafe impl<'__w, T: Component> QueryData for Ref<'__w, T> { unsafe { table.debug_checked_unwrap() }; // SAFETY: The caller ensures `table_row` is in range. - let component = unsafe { table_components.get(table_row.as_usize()) }; + let component = unsafe { table_components.get(table_row.index()) }; // SAFETY: The caller ensures `table_row` is in range. - let added = unsafe { added_ticks.get(table_row.as_usize()) }; + let added = unsafe { added_ticks.get(table_row.index()) }; // SAFETY: The caller ensures `table_row` is in range. - let changed = unsafe { changed_ticks.get(table_row.as_usize()) }; + let changed = unsafe { changed_ticks.get(table_row.index()) }; // SAFETY: The caller ensures `table_row` is in range. - let caller = callers.map(|callers| unsafe { callers.get(table_row.as_usize()) }); + let caller = callers.map(|callers| unsafe { callers.get(table_row.index()) }); Ref { value: component.deref(), @@ -1882,13 +1882,13 @@ unsafe impl<'__w, T: Component> QueryData for &'__w mut T unsafe { table.debug_checked_unwrap() }; // SAFETY: The caller ensures `table_row` is in range. - let component = unsafe { table_components.get(table_row.as_usize()) }; + let component = unsafe { table_components.get(table_row.index()) }; // SAFETY: The caller ensures `table_row` is in range. - let added = unsafe { added_ticks.get(table_row.as_usize()) }; + let added = unsafe { added_ticks.get(table_row.index()) }; // SAFETY: The caller ensures `table_row` is in range. - let changed = unsafe { changed_ticks.get(table_row.as_usize()) }; + let changed = unsafe { changed_ticks.get(table_row.index()) }; // SAFETY: The caller ensures `table_row` is in range. - let caller = callers.map(|callers| unsafe { callers.get(table_row.as_usize()) }); + let caller = callers.map(|callers| unsafe { callers.get(table_row.index()) }); Mut { value: component.deref_mut(), diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 38c7cfcb32c92..119afe32ab886 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -817,7 +817,7 @@ unsafe impl QueryFilter for Added { // SAFETY: set_table was previously called let table = unsafe { table.debug_checked_unwrap() }; // SAFETY: The caller ensures `table_row` is in range. - let tick = unsafe { table.get(table_row.as_usize()) }; + let tick = unsafe { table.get(table_row.index()) }; tick.deref().is_newer_than(fetch.last_run, fetch.this_run) }, @@ -1044,7 +1044,7 @@ unsafe impl QueryFilter for Changed { // SAFETY: set_table was previously called let table = unsafe { table.debug_checked_unwrap() }; // SAFETY: The caller ensures `table_row` is in range. - let tick = unsafe { table.get(table_row.as_usize()) }; + let tick = unsafe { table.get(table_row.index()) }; tick.deref().is_newer_than(fetch.last_run, fetch.this_run) }, diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 6ca4f48acd858..fb247719df51a 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -221,7 +221,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: Caller assures `row` in range of the current archetype. let entity = unsafe { entities.get_unchecked(row as usize) }; // SAFETY: This is from an exclusive range, so it can't be max. - let row = unsafe { TableRow::from_u32(NonMaxU32::new_unchecked(row)) }; + let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(row)) }; // SAFETY: set_table was called prior. // Caller assures `row` in range of the current archetype. @@ -352,7 +352,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: Caller assures `row` in range of the current archetype. let entity = unsafe { *entities.get_unchecked(row as usize) }; // SAFETY: This is from an exclusive range, so it can't be max. - let row = unsafe { TableRow::from_u32(NonMaxU32::new_unchecked(row)) }; + let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(row)) }; // SAFETY: set_table was called prior. // Caller assures `row` in range of the current archetype. @@ -2404,7 +2404,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { &mut self.fetch, *entity, // SAFETY: This is from an exclusive range, so it can't be max. - TableRow::from_u32(NonMaxU32::new_unchecked(index)), + TableRow::new(NonMaxU32::new_unchecked(index)), )) } } else { @@ -2483,7 +2483,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { let entity = unsafe { self.table_entities.get_unchecked(self.current_row as usize) }; // SAFETY: The row is less than the u32 len, so it must not be max. - let row = unsafe { TableRow::from_u32(NonMaxU32::new_unchecked(self.current_row)) }; + let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(self.current_row)) }; if !F::filter_fetch(&mut self.filter, *entity, row) { self.current_row += 1; continue; diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index e70a24bb3a65e..42adcd89dc3b3 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -170,7 +170,7 @@ impl ComponentSparseSet { ) { if let Some(&dense_index) = self.sparse.get(entity.row()) { #[cfg(debug_assertions)] - assert_eq!(entity, self.entities[dense_index.as_usize()]); + assert_eq!(entity, self.entities[dense_index.index()]); self.dense.replace(dense_index, value, change_tick, caller); } else { let dense_index = self.dense.len(); @@ -180,8 +180,7 @@ impl ComponentSparseSet { // SAFETY: This entity row does not exist here yet, so there are no duplicates, // and the entity index can not be the max, so the length must not be max either. // To do so would have caused a panic in the entity alloxator. - let table_row = - unsafe { TableRow::from_u32(NonMaxU32::new_unchecked(dense_index as u32)) }; + let table_row = unsafe { TableRow::new(NonMaxU32::new_unchecked(dense_index as u32)) }; self.sparse.insert(entity.row(), table_row); #[cfg(debug_assertions)] @@ -200,7 +199,7 @@ impl ComponentSparseSet { { if let Some(&dense_index) = self.sparse.get(entity.row()) { #[cfg(debug_assertions)] - assert_eq!(entity, self.entities[dense_index.as_usize()]); + assert_eq!(entity, self.entities[dense_index.index()]); true } else { false @@ -217,7 +216,7 @@ impl ComponentSparseSet { pub fn get(&self, entity: Entity) -> Option> { self.sparse.get(entity.row()).map(|&dense_index| { #[cfg(debug_assertions)] - assert_eq!(entity, self.entities[dense_index.as_usize()]); + assert_eq!(entity, self.entities[dense_index.index()]); // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { self.dense.get_data_unchecked(dense_index) } }) @@ -237,7 +236,7 @@ impl ComponentSparseSet { )> { let dense_index = *self.sparse.get(entity.row())?; #[cfg(debug_assertions)] - assert_eq!(entity, self.entities[dense_index.as_usize()]); + assert_eq!(entity, self.entities[dense_index.index()]); // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { Some(( @@ -258,7 +257,7 @@ impl ComponentSparseSet { pub fn get_added_tick(&self, entity: Entity) -> Option<&UnsafeCell> { let dense_index = *self.sparse.get(entity.row())?; #[cfg(debug_assertions)] - assert_eq!(entity, self.entities[dense_index.as_usize()]); + assert_eq!(entity, self.entities[dense_index.index()]); // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { Some(self.dense.get_added_tick_unchecked(dense_index)) } } @@ -270,7 +269,7 @@ impl ComponentSparseSet { pub fn get_changed_tick(&self, entity: Entity) -> Option<&UnsafeCell> { let dense_index = *self.sparse.get(entity.row())?; #[cfg(debug_assertions)] - assert_eq!(entity, self.entities[dense_index.as_usize()]); + assert_eq!(entity, self.entities[dense_index.index()]); // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { Some(self.dense.get_changed_tick_unchecked(dense_index)) } } @@ -282,7 +281,7 @@ impl ComponentSparseSet { pub fn get_ticks(&self, entity: Entity) -> Option { let dense_index = *self.sparse.get(entity.row())?; #[cfg(debug_assertions)] - assert_eq!(entity, self.entities[dense_index.as_usize()]); + assert_eq!(entity, self.entities[dense_index.index()]); // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { Some(self.dense.get_ticks_unchecked(dense_index)) } } @@ -298,7 +297,7 @@ impl ComponentSparseSet { MaybeLocation::new_with_flattened(|| { let dense_index = *self.sparse.get(entity.row())?; #[cfg(debug_assertions)] - assert_eq!(entity, self.entities[dense_index.as_usize()]); + assert_eq!(entity, self.entities[dense_index.index()]); // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { Some(self.dense.get_changed_by_unchecked(dense_index)) } }) @@ -317,13 +316,13 @@ impl ComponentSparseSet { pub(crate) fn remove_and_forget(&mut self, entity: Entity) -> Option> { self.sparse.remove(entity.row()).map(|dense_index| { #[cfg(debug_assertions)] - assert_eq!(entity, self.entities[dense_index.as_usize()]); - self.entities.swap_remove(dense_index.as_usize()); - let is_last = dense_index.as_usize() == self.dense.len() - 1; + assert_eq!(entity, self.entities[dense_index.index()]); + self.entities.swap_remove(dense_index.index()); + let is_last = dense_index.index() == self.dense.len() - 1; // SAFETY: dense_index was just removed from `sparse`, which ensures that it is valid let (value, _, _) = unsafe { self.dense.swap_remove_and_forget_unchecked(dense_index) }; if !is_last { - let swapped_entity = self.entities[dense_index.as_usize()]; + let swapped_entity = self.entities[dense_index.index()]; #[cfg(not(debug_assertions))] let index = swapped_entity; #[cfg(debug_assertions)] @@ -340,15 +339,15 @@ impl ComponentSparseSet { pub(crate) fn remove(&mut self, entity: Entity) -> bool { if let Some(dense_index) = self.sparse.remove(entity.row()) { #[cfg(debug_assertions)] - assert_eq!(entity, self.entities[dense_index.as_usize()]); - self.entities.swap_remove(dense_index.as_usize()); - let is_last = dense_index.as_usize() == self.dense.len() - 1; + assert_eq!(entity, self.entities[dense_index.index()]); + self.entities.swap_remove(dense_index.index()); + let is_last = dense_index.index() == self.dense.len() - 1; // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { self.dense.swap_remove_unchecked(dense_index); } if !is_last { - let swapped_entity = self.entities[dense_index.as_usize()]; + let swapped_entity = self.entities[dense_index.index()]; #[cfg(not(debug_assertions))] let index = swapped_entity; #[cfg(debug_assertions)] diff --git a/crates/bevy_ecs/src/storage/table/column.rs b/crates/bevy_ecs/src/storage/table/column.rs index 522df222c6379..e02659ed0e5ad 100644 --- a/crates/bevy_ecs/src/storage/table/column.rs +++ b/crates/bevy_ecs/src/storage/table/column.rs @@ -49,13 +49,13 @@ impl ThinColumn { row: TableRow, ) { self.data - .swap_remove_and_drop_unchecked_nonoverlapping(row.as_usize(), last_element_index); + .swap_remove_and_drop_unchecked_nonoverlapping(row.index(), last_element_index); self.added_ticks - .swap_remove_unchecked_nonoverlapping(row.as_usize(), last_element_index); + .swap_remove_unchecked_nonoverlapping(row.index(), last_element_index); self.changed_ticks - .swap_remove_unchecked_nonoverlapping(row.as_usize(), last_element_index); + .swap_remove_unchecked_nonoverlapping(row.index(), last_element_index); self.changed_by.as_mut().map(|changed_by| { - changed_by.swap_remove_unchecked_nonoverlapping(row.as_usize(), last_element_index); + changed_by.swap_remove_unchecked_nonoverlapping(row.index(), last_element_index); }); } @@ -71,13 +71,13 @@ impl ThinColumn { row: TableRow, ) { self.data - .swap_remove_and_drop_unchecked(row.as_usize(), last_element_index); + .swap_remove_and_drop_unchecked(row.index(), last_element_index); self.added_ticks - .swap_remove_and_drop_unchecked(row.as_usize(), last_element_index); + .swap_remove_and_drop_unchecked(row.index(), last_element_index); self.changed_ticks - .swap_remove_and_drop_unchecked(row.as_usize(), last_element_index); + .swap_remove_and_drop_unchecked(row.index(), last_element_index); self.changed_by.as_mut().map(|changed_by| { - changed_by.swap_remove_and_drop_unchecked(row.as_usize(), last_element_index); + changed_by.swap_remove_and_drop_unchecked(row.index(), last_element_index); }); } @@ -94,14 +94,14 @@ impl ThinColumn { ) { let _ = self .data - .swap_remove_unchecked(row.as_usize(), last_element_index); + .swap_remove_unchecked(row.index(), last_element_index); self.added_ticks - .swap_remove_unchecked(row.as_usize(), last_element_index); + .swap_remove_unchecked(row.index(), last_element_index); self.changed_ticks - .swap_remove_unchecked(row.as_usize(), last_element_index); + .swap_remove_unchecked(row.index(), last_element_index); self.changed_by .as_mut() - .map(|changed_by| changed_by.swap_remove_unchecked(row.as_usize(), last_element_index)); + .map(|changed_by| changed_by.swap_remove_unchecked(row.index(), last_element_index)); } /// Call [`realloc`](std::alloc::realloc) to expand / shrink the memory allocation for this [`ThinColumn`] @@ -148,15 +148,15 @@ impl ThinColumn { tick: Tick, caller: MaybeLocation, ) { - self.data.initialize_unchecked(row.as_usize(), data); - *self.added_ticks.get_unchecked_mut(row.as_usize()).get_mut() = tick; + self.data.initialize_unchecked(row.index(), data); + *self.added_ticks.get_unchecked_mut(row.index()).get_mut() = tick; *self .changed_ticks - .get_unchecked_mut(row.as_usize()) + .get_unchecked_mut(row.index()) .get_mut() = tick; self.changed_by .as_mut() - .map(|changed_by| changed_by.get_unchecked_mut(row.as_usize()).get_mut()) + .map(|changed_by| changed_by.get_unchecked_mut(row.index()).get_mut()) .assign(caller); } @@ -173,14 +173,14 @@ impl ThinColumn { change_tick: Tick, caller: MaybeLocation, ) { - self.data.replace_unchecked(row.as_usize(), data); + self.data.replace_unchecked(row.index(), data); *self .changed_ticks - .get_unchecked_mut(row.as_usize()) + .get_unchecked_mut(row.index()) .get_mut() = change_tick; self.changed_by .as_mut() - .map(|changed_by| changed_by.get_unchecked_mut(row.as_usize()).get_mut()) + .map(|changed_by| changed_by.get_unchecked_mut(row.index()).get_mut()) .assign(caller); } @@ -206,25 +206,25 @@ impl ThinColumn { // Init the data let src_val = other .data - .swap_remove_unchecked(src_row.as_usize(), other_last_element_index); - self.data.initialize_unchecked(dst_row.as_usize(), src_val); + .swap_remove_unchecked(src_row.index(), other_last_element_index); + self.data.initialize_unchecked(dst_row.index(), src_val); // Init added_ticks let added_tick = other .added_ticks - .swap_remove_unchecked(src_row.as_usize(), other_last_element_index); + .swap_remove_unchecked(src_row.index(), other_last_element_index); self.added_ticks - .initialize_unchecked(dst_row.as_usize(), added_tick); + .initialize_unchecked(dst_row.index(), added_tick); // Init changed_ticks let changed_tick = other .changed_ticks - .swap_remove_unchecked(src_row.as_usize(), other_last_element_index); + .swap_remove_unchecked(src_row.index(), other_last_element_index); self.changed_ticks - .initialize_unchecked(dst_row.as_usize(), changed_tick); + .initialize_unchecked(dst_row.index(), changed_tick); self.changed_by.as_mut().zip(other.changed_by.as_mut()).map( |(self_changed_by, other_changed_by)| { let changed_by = other_changed_by - .swap_remove_unchecked(src_row.as_usize(), other_last_element_index); - self_changed_by.initialize_unchecked(dst_row.as_usize(), changed_by); + .swap_remove_unchecked(src_row.index(), other_last_element_index); + self_changed_by.initialize_unchecked(dst_row.index(), changed_by); }, ); } @@ -384,15 +384,15 @@ impl Column { change_tick: Tick, caller: MaybeLocation, ) { - debug_assert!(row.as_usize() < self.len()); - self.data.replace_unchecked(row.as_usize(), data); + debug_assert!(row.index() < self.len()); + self.data.replace_unchecked(row.index(), data); *self .changed_ticks - .get_unchecked_mut(row.as_usize()) + .get_unchecked_mut(row.index()) .get_mut() = change_tick; self.changed_by .as_mut() - .map(|changed_by| changed_by.get_unchecked_mut(row.as_usize()).get_mut()) + .map(|changed_by| changed_by.get_unchecked_mut(row.index()).get_mut()) .assign(caller); } @@ -419,12 +419,12 @@ impl Column { /// `row` must be within the range `[0, self.len())`. #[inline] pub(crate) unsafe fn swap_remove_unchecked(&mut self, row: TableRow) { - 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()); + self.data.swap_remove_and_drop_unchecked(row.index()); + self.added_ticks.swap_remove(row.index()); + self.changed_ticks.swap_remove(row.index()); self.changed_by .as_mut() - .map(|changed_by| changed_by.swap_remove(row.as_usize())); + .map(|changed_by| changed_by.swap_remove(row.index())); } /// Removes an element from the [`Column`] and returns it and its change detection ticks. @@ -444,13 +444,13 @@ impl Column { &mut self, row: TableRow, ) -> (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(); + let data = self.data.swap_remove_and_forget_unchecked(row.index()); + let added = self.added_ticks.swap_remove(row.index()).into_inner(); + let changed = self.changed_ticks.swap_remove(row.index()).into_inner(); let caller = self .changed_by .as_mut() - .map(|changed_by| changed_by.swap_remove(row.as_usize()).into_inner()); + .map(|changed_by| changed_by.swap_remove(row.index()).into_inner()); (data, ComponentTicks { added, changed }, caller) } @@ -520,15 +520,15 @@ impl Column { /// Returns `None` if `row` is out of bounds. #[inline] pub fn get(&self, row: TableRow) -> Option<(Ptr<'_>, TickCells<'_>)> { - (row.as_usize() < self.data.len()) + (row.index() < self.data.len()) // SAFETY: The row is length checked before fetching the pointer. This is being // accessed through a read-only reference to the column. .then(|| unsafe { ( - self.data.get_unchecked(row.as_usize()), + self.data.get_unchecked(row.index()), TickCells { - added: self.added_ticks.get_unchecked(row.as_usize()), - changed: self.changed_ticks.get_unchecked(row.as_usize()), + added: self.added_ticks.get_unchecked(row.index()), + changed: self.changed_ticks.get_unchecked(row.index()), }, ) }) @@ -539,10 +539,10 @@ impl Column { /// Returns `None` if `row` is out of bounds. #[inline] pub fn get_data(&self, row: TableRow) -> Option> { - (row.as_usize() < self.data.len()).then(|| { + (row.index() < self.data.len()).then(|| { // SAFETY: The row is length checked before fetching the pointer. This is being // accessed through a read-only reference to the column. - unsafe { self.data.get_unchecked(row.as_usize()) } + unsafe { self.data.get_unchecked(row.index()) } }) } @@ -554,8 +554,8 @@ impl Column { /// - no other mutable reference to the data of the same row can exist at the same time #[inline] pub unsafe fn get_data_unchecked(&self, row: TableRow) -> Ptr<'_> { - debug_assert!(row.as_usize() < self.data.len()); - self.data.get_unchecked(row.as_usize()) + debug_assert!(row.index() < self.data.len()); + self.data.get_unchecked(row.index()) } /// Fetches a mutable reference to the data at `row`. @@ -563,10 +563,10 @@ impl Column { /// Returns `None` if `row` is out of bounds. #[inline] pub fn get_data_mut(&mut self, row: TableRow) -> Option> { - (row.as_usize() < self.data.len()).then(|| { + (row.index() < self.data.len()).then(|| { // SAFETY: The row is length checked before fetching the pointer. This is being // accessed through an exclusive reference to the column. - unsafe { self.data.get_unchecked_mut(row.as_usize()) } + unsafe { self.data.get_unchecked_mut(row.index()) } }) } @@ -579,7 +579,7 @@ impl Column { /// adhere to the safety invariants of [`UnsafeCell`]. #[inline] pub fn get_added_tick(&self, row: TableRow) -> Option<&UnsafeCell> { - self.added_ticks.get(row.as_usize()) + self.added_ticks.get(row.index()) } /// Fetches the "changed" change detection tick for the value at `row`. @@ -591,7 +591,7 @@ impl Column { /// adhere to the safety invariants of [`UnsafeCell`]. #[inline] pub fn get_changed_tick(&self, row: TableRow) -> Option<&UnsafeCell> { - self.changed_ticks.get(row.as_usize()) + self.changed_ticks.get(row.index()) } /// Fetches the change detection ticks for the value at `row`. @@ -599,7 +599,7 @@ impl Column { /// Returns `None` if `row` is out of bounds. #[inline] pub fn get_ticks(&self, row: TableRow) -> Option { - if row.as_usize() < self.data.len() { + if row.index() < self.data.len() { // SAFETY: The size of the column has already been checked. Some(unsafe { self.get_ticks_unchecked(row) }) } else { @@ -614,8 +614,8 @@ impl Column { /// `row` must be within the range `[0, self.len())`. #[inline] pub unsafe fn get_added_tick_unchecked(&self, row: TableRow) -> &UnsafeCell { - debug_assert!(row.as_usize() < self.added_ticks.len()); - self.added_ticks.get_unchecked(row.as_usize()) + debug_assert!(row.index() < self.added_ticks.len()); + self.added_ticks.get_unchecked(row.index()) } /// Fetches the "changed" change detection tick for the value at `row`. Unlike [`Column::get_changed_tick`] @@ -625,8 +625,8 @@ impl Column { /// `row` must be within the range `[0, self.len())`. #[inline] pub unsafe fn get_changed_tick_unchecked(&self, row: TableRow) -> &UnsafeCell { - debug_assert!(row.as_usize() < self.changed_ticks.len()); - self.changed_ticks.get_unchecked(row.as_usize()) + debug_assert!(row.index() < self.changed_ticks.len()); + self.changed_ticks.get_unchecked(row.index()) } /// Fetches the change detection ticks for the value at `row`. Unlike [`Column::get_ticks`] @@ -636,11 +636,11 @@ impl Column { /// `row` must be within the range `[0, self.len())`. #[inline] pub unsafe fn get_ticks_unchecked(&self, row: TableRow) -> ComponentTicks { - debug_assert!(row.as_usize() < self.added_ticks.len()); - debug_assert!(row.as_usize() < self.changed_ticks.len()); + debug_assert!(row.index() < self.added_ticks.len()); + debug_assert!(row.index() < self.changed_ticks.len()); ComponentTicks { - added: self.added_ticks.get_unchecked(row.as_usize()).read(), - changed: self.changed_ticks.get_unchecked(row.as_usize()).read(), + added: self.added_ticks.get_unchecked(row.index()).read(), + changed: self.changed_ticks.get_unchecked(row.index()).read(), } } @@ -678,7 +678,7 @@ impl Column { ) -> MaybeLocation>>> { self.changed_by .as_ref() - .map(|changed_by| changed_by.get(row.as_usize())) + .map(|changed_by| changed_by.get(row.index())) } /// Fetches the calling location that last changed the value at `row`. @@ -693,8 +693,8 @@ impl Column { row: TableRow, ) -> MaybeLocation<&UnsafeCell<&'static Location<'static>>> { self.changed_by.as_ref().map(|changed_by| { - debug_assert!(row.as_usize() < changed_by.len()); - changed_by.get_unchecked(row.as_usize()) + debug_assert!(row.index() < changed_by.len()); + changed_by.get_unchecked(row.index()) }) } diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index 8ceeb6ea5e478..011f98d2f32ff 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -109,20 +109,20 @@ impl TableRow { /// Creates a [`TableRow`]. #[inline] - pub const fn from_u32(index: NonMaxU32) -> Self { + pub const fn new(index: NonMaxU32) -> Self { Self(index) } /// Gets the index of the row as a [`usize`]. #[inline] - pub const fn as_usize(self) -> usize { + pub const fn index(self) -> usize { // usize is at least u32 in Bevy self.0.get() as usize } /// Gets the index of the row as a [`usize`]. #[inline] - pub const fn as_u32(self) -> u32 { + pub const fn index_u32(self) -> u32 { self.0.get() } } @@ -216,9 +216,9 @@ impl Table { /// # Safety /// `row` must be in-bounds (`row.as_usize()` < `self.len()`) pub(crate) unsafe fn swap_remove_unchecked(&mut self, row: TableRow) -> Option { - debug_assert!(row.as_u32() < self.entity_count()); + debug_assert!(row.index_u32() < self.entity_count()); let last_element_index = self.entity_count() - 1; - if row.as_u32() != last_element_index { + if row.index_u32() != last_element_index { // Instead of checking this condition on every `swap_remove` call, we // check it here and use `swap_remove_nonoverlapping`. for col in self.columns.values_mut() { @@ -241,13 +241,13 @@ impl Table { col.drop_last_component(last_element_index as usize); } } - let is_last = row.as_u32() == last_element_index; - self.entities.swap_remove(row.as_usize()); + let is_last = row.index_u32() == last_element_index; + self.entities.swap_remove(row.index()); if is_last { None } else { // SAFETY: This was sawp removed and was not last, so it must be in bounds. - unsafe { Some(*self.entities.get_unchecked(row.as_usize())) } + unsafe { Some(*self.entities.get_unchecked(row.index())) } } } @@ -264,10 +264,10 @@ impl Table { row: TableRow, new_table: &mut Table, ) -> TableMoveResult { - debug_assert!(row.as_u32() < self.entity_count()); + debug_assert!(row.index_u32() < self.entity_count()); let last_element_index = self.entity_count() - 1; - let is_last = row.as_u32() == last_element_index; - let new_row = new_table.allocate(self.entities.swap_remove(row.as_usize())); + let is_last = row.index_u32() == last_element_index; + let new_row = new_table.allocate(self.entities.swap_remove(row.index())); for (component_id, column) in self.columns.iter_mut() { if let Some(new_column) = new_table.get_column_mut(*component_id) { new_column.initialize_from_unchecked( @@ -287,7 +287,7 @@ impl Table { None } else { // SAFETY: This was sawp removed and was not last, so it must be in bounds. - unsafe { Some(*self.entities.get_unchecked(row.as_usize())) } + unsafe { Some(*self.entities.get_unchecked(row.index())) } }, } } @@ -303,10 +303,10 @@ impl Table { row: TableRow, new_table: &mut Table, ) -> TableMoveResult { - debug_assert!(row.as_u32() < self.entity_count()); + debug_assert!(row.index_u32() < self.entity_count()); let last_element_index = self.entity_count() - 1; - let is_last = row.as_u32() == last_element_index; - let new_row = new_table.allocate(self.entities.swap_remove(row.as_usize())); + let is_last = row.index_u32() == last_element_index; + let new_row = new_table.allocate(self.entities.swap_remove(row.index())); for (component_id, column) in self.columns.iter_mut() { if let Some(new_column) = new_table.get_column_mut(*component_id) { new_column.initialize_from_unchecked( @@ -325,7 +325,7 @@ impl Table { None } else { // SAFETY: This was sawp removed and was not last, so it must be in bounds. - unsafe { Some(*self.entities.get_unchecked(row.as_usize())) } + unsafe { Some(*self.entities.get_unchecked(row.index())) } }, } } @@ -342,10 +342,10 @@ impl Table { row: TableRow, new_table: &mut Table, ) -> TableMoveResult { - debug_assert!(row.as_u32() < self.entity_count()); + debug_assert!(row.index_u32() < self.entity_count()); let last_element_index = self.entity_count() - 1; - let is_last = row.as_u32() == last_element_index; - let new_row = new_table.allocate(self.entities.swap_remove(row.as_usize())); + let is_last = row.index_u32() == last_element_index; + let new_row = new_table.allocate(self.entities.swap_remove(row.index())); for (component_id, column) in self.columns.iter_mut() { new_table .get_column_mut(*component_id) @@ -358,7 +358,7 @@ impl Table { None } else { // SAFETY: This was sawp removed and was not last, so it must be in bounds. - unsafe { Some(*self.entities.get_unchecked(row.as_usize())) } + unsafe { Some(*self.entities.get_unchecked(row.index())) } }, } } @@ -414,12 +414,12 @@ impl Table { component_id: ComponentId, row: TableRow, ) -> Option<&UnsafeCell> { - (row.as_u32() < self.entity_count()).then_some( + (row.index_u32() < self.entity_count()).then_some( // SAFETY: `row.as_usize()` < `len` unsafe { self.get_column(component_id)? .changed_ticks - .get_unchecked(row.as_usize()) + .get_unchecked(row.index()) }, ) } @@ -430,12 +430,12 @@ impl Table { component_id: ComponentId, row: TableRow, ) -> Option<&UnsafeCell> { - (row.as_u32() < self.entity_count()).then_some( + (row.index_u32() < self.entity_count()).then_some( // SAFETY: `row.as_usize()` < `len` unsafe { self.get_column(component_id)? .added_ticks - .get_unchecked(row.as_usize()) + .get_unchecked(row.index()) }, ) } @@ -447,13 +447,13 @@ impl Table { row: TableRow, ) -> MaybeLocation>>> { MaybeLocation::new_with_flattened(|| { - (row.as_u32() < self.entity_count()).then_some( + (row.index_u32() < self.entity_count()).then_some( // SAFETY: `row.as_usize()` < `len` unsafe { self.get_column(component_id)? .changed_by .as_ref() - .map(|changed_by| changed_by.get_unchecked(row.as_usize())) + .map(|changed_by| changed_by.get_unchecked(row.index())) }, ) }) @@ -469,8 +469,8 @@ impl Table { row: TableRow, ) -> Option { self.get_column(component_id).map(|col| ComponentTicks { - added: col.added_ticks.get_unchecked(row.as_usize()).read(), - changed: col.changed_ticks.get_unchecked(row.as_usize()).read(), + added: col.added_ticks.get_unchecked(row.index()).read(), + changed: col.changed_ticks.get_unchecked(row.index()).read(), }) } @@ -578,7 +578,7 @@ impl Table { let len = self.entity_count(); // SAFETY: No entity row may be in more than one table row at once, so there are no duplicates, // and there can not be an entity row of u32::MAX. Therefore, this can not be max either. - let row = unsafe { TableRow::from_u32(NonMaxU32::new_unchecked(len)) }; + let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(len)) }; let len = len as usize; self.entities.push(entity); for col in self.columns.values_mut() { @@ -676,7 +676,7 @@ impl Table { self.get_column_mut(component_id) .debug_checked_unwrap() .data - .get_unchecked_mut(row.as_usize()) + .get_unchecked_mut(row.index()) .promote() } @@ -690,7 +690,7 @@ impl Table { row: TableRow, ) -> Option> { self.get_column(component_id) - .map(|col| col.data.get_unchecked(row.as_usize())) + .map(|col| col.data.get_unchecked(row.index())) } } From 7fd04e51d7c14d339c347901199cb976b03a119b Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Thu, 8 May 2025 23:47:43 -0400 Subject: [PATCH 05/10] formatting --- crates/bevy_ecs/src/storage/table/column.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/storage/table/column.rs b/crates/bevy_ecs/src/storage/table/column.rs index e02659ed0e5ad..78fafe0a2612d 100644 --- a/crates/bevy_ecs/src/storage/table/column.rs +++ b/crates/bevy_ecs/src/storage/table/column.rs @@ -150,10 +150,7 @@ impl ThinColumn { ) { self.data.initialize_unchecked(row.index(), data); *self.added_ticks.get_unchecked_mut(row.index()).get_mut() = tick; - *self - .changed_ticks - .get_unchecked_mut(row.index()) - .get_mut() = tick; + *self.changed_ticks.get_unchecked_mut(row.index()).get_mut() = tick; self.changed_by .as_mut() .map(|changed_by| changed_by.get_unchecked_mut(row.index()).get_mut()) @@ -174,10 +171,7 @@ impl ThinColumn { caller: MaybeLocation, ) { self.data.replace_unchecked(row.index(), data); - *self - .changed_ticks - .get_unchecked_mut(row.index()) - .get_mut() = change_tick; + *self.changed_ticks.get_unchecked_mut(row.index()).get_mut() = change_tick; self.changed_by .as_mut() .map(|changed_by| changed_by.get_unchecked_mut(row.index()).get_mut()) @@ -386,10 +380,7 @@ impl Column { ) { debug_assert!(row.index() < self.len()); self.data.replace_unchecked(row.index(), data); - *self - .changed_ticks - .get_unchecked_mut(row.index()) - .get_mut() = change_tick; + *self.changed_ticks.get_unchecked_mut(row.index()).get_mut() = change_tick; self.changed_by .as_mut() .map(|changed_by| changed_by.get_unchecked_mut(row.index()).get_mut()) From 52cc0bb445a37253e37b97f0a4bf137d9c28b4b1 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Fri, 9 May 2025 00:03:21 -0400 Subject: [PATCH 06/10] migration guide --- crates/bevy_ecs/src/archetype.rs | 6 +++++ .../migration-guides/entity_representation.md | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 8eae8e2a9014e..42263a328b57e 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -63,6 +63,12 @@ impl ArchetypeRow { pub const fn index(self) -> usize { self.0.get() as usize } + + /// Gets the index of the row. + #[inline] + pub const fn index_u32(self) -> u32 { + self.0.get() + } } /// An opaque unique ID for a single [`Archetype`] within a [`World`]. diff --git a/release-content/migration-guides/entity_representation.md b/release-content/migration-guides/entity_representation.md index cf7dd8d3d550d..107ac13717f25 100644 --- a/release-content/migration-guides/entity_representation.md +++ b/release-content/migration-guides/entity_representation.md @@ -55,3 +55,26 @@ The changes to serialization and the bit format are directly related. Effectively, this means that all serialized and transmuted entities will not work as expected and may crash. To migrate, invert the lower 32 bits of the 64 representation of the entity, and subtract 1 from the upper bits. Again, this is still subject to change, and serialized scenes may break between versions. + +### Length Representation + +Because the maximum index of an entity is now `NonZeroU32::MAX`, the maximum number of entities (and length of unique entity row colections) is `u32::MAX`. +As a result, a lot of APIs that returned `usize` have been changed to `u32`. + +These include: + + - `Archetype::len` + - `Table::entity_count` + +### Other kinds of entity rows + +Since the `EntityRow` is a `NonMaxU32`, `TableRow` and `ArchetypeRow` have been given the same treatment. +Additionally, they have been given new, standardized interfaces: + + - `fn new(NonMaxU32)` + - `fn index(self) -> usize` + - `fn index_u32(self) -> u32` + +The other interfaces for these types have been removed. +Although it's not usually recommended to be creating these types manually, if you run into any issues migrating here, please open an issue. +If all else fails, `TableRow` and `ArchetypeRow` are `repr(transparent)`, allowing careful tranumations. From 42e31962eb9f01498f9bd11ed202b7fb46129657 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Fri, 9 May 2025 00:21:43 -0400 Subject: [PATCH 07/10] fix ci --- release-content/migration-guides/entity_representation.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/release-content/migration-guides/entity_representation.md b/release-content/migration-guides/entity_representation.md index 107ac13717f25..5a8f12cfabc17 100644 --- a/release-content/migration-guides/entity_representation.md +++ b/release-content/migration-guides/entity_representation.md @@ -58,7 +58,7 @@ Again, this is still subject to change, and serialized scenes may break between ### Length Representation -Because the maximum index of an entity is now `NonZeroU32::MAX`, the maximum number of entities (and length of unique entity row colections) is `u32::MAX`. +Because the maximum index of an entity is now `NonZeroU32::MAX`, the maximum number of entities (and length of unique entity row collections) is `u32::MAX`. As a result, a lot of APIs that returned `usize` have been changed to `u32`. These include: @@ -69,6 +69,8 @@ These include: ### Other kinds of entity rows Since the `EntityRow` is a `NonMaxU32`, `TableRow` and `ArchetypeRow` have been given the same treatment. +They now wrap a `NonMaxU32`, allowing more performance optimizations. + Additionally, they have been given new, standardized interfaces: - `fn new(NonMaxU32)` From a8bd757b6c20aa322fe1cc12664e6303db5c95d1 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Fri, 9 May 2025 00:25:23 -0400 Subject: [PATCH 08/10] now fix markdown linter --- .../migration-guides/entity_representation.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/release-content/migration-guides/entity_representation.md b/release-content/migration-guides/entity_representation.md index 5a8f12cfabc17..a7d342b82e2cc 100644 --- a/release-content/migration-guides/entity_representation.md +++ b/release-content/migration-guides/entity_representation.md @@ -63,8 +63,8 @@ As a result, a lot of APIs that returned `usize` have been changed to `u32`. These include: - - `Archetype::len` - - `Table::entity_count` +- `Archetype::len` +- `Table::entity_count` ### Other kinds of entity rows @@ -73,9 +73,9 @@ They now wrap a `NonMaxU32`, allowing more performance optimizations. Additionally, they have been given new, standardized interfaces: - - `fn new(NonMaxU32)` - - `fn index(self) -> usize` - - `fn index_u32(self) -> u32` +- `fn new(NonMaxU32)` +- `fn index(self) -> usize` +- `fn index_u32(self) -> u32` The other interfaces for these types have been removed. Although it's not usually recommended to be creating these types manually, if you run into any issues migrating here, please open an issue. From a5e226b553c4cae18b05dc02269cbdc9022487d3 Mon Sep 17 00:00:00 2001 From: Eagster <79881080+ElliottjPierce@users.noreply.github.com> Date: Sat, 10 May 2025 22:10:06 -0400 Subject: [PATCH 09/10] Fix typo Co-authored-by: Christian Hughes <9044780+ItsDoot@users.noreply.github.com> --- release-content/migration-guides/entity_representation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release-content/migration-guides/entity_representation.md b/release-content/migration-guides/entity_representation.md index a7d342b82e2cc..e042f493dae1b 100644 --- a/release-content/migration-guides/entity_representation.md +++ b/release-content/migration-guides/entity_representation.md @@ -79,4 +79,4 @@ Additionally, they have been given new, standardized interfaces: The other interfaces for these types have been removed. Although it's not usually recommended to be creating these types manually, if you run into any issues migrating here, please open an issue. -If all else fails, `TableRow` and `ArchetypeRow` are `repr(transparent)`, allowing careful tranumations. +If all else fails, `TableRow` and `ArchetypeRow` are `repr(transparent)`, allowing careful transmutations. From f488f85f25ddea593679305afda3709973948795 Mon Sep 17 00:00:00 2001 From: Elliott Pierce Date: Fri, 23 May 2025 16:21:43 -0400 Subject: [PATCH 10/10] Add todos --- crates/bevy_ecs/src/archetype.rs | 1 + crates/bevy_ecs/src/storage/table/mod.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 42263a328b57e..516c90a873371 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -50,6 +50,7 @@ pub struct ArchetypeRow(NonMaxU32); impl ArchetypeRow { /// Index indicating an invalid archetype row. /// This is meant to be used as a placeholder. + // TODO: Deprecate in favor of options, since `INVALID` is, technically, valid. pub const INVALID: ArchetypeRow = ArchetypeRow(NonMaxU32::MAX); /// Creates a `ArchetypeRow`. diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index 011f98d2f32ff..cf579de8c751a 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -105,6 +105,7 @@ impl TableId { pub struct TableRow(NonMaxU32); impl TableRow { + // TODO: Deprecate in favor of options, since `INVALID` is, technically, valid. pub(crate) const INVALID: TableRow = TableRow(NonMaxU32::MAX); /// Creates a [`TableRow`].