diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index a25551f013da3..759e8861c105d 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -28,6 +28,7 @@ use crate::{ use std::{ collections::HashMap, hash::Hash, + num::{NonZeroU32, NonZeroUsize}, ops::{Index, IndexMut}, }; @@ -71,24 +72,42 @@ impl ArchetypeRow { #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] // SAFETY: Must be repr(transparent) due to the safety requirements on EntityLocation #[repr(transparent)] -pub struct ArchetypeId(u32); +pub struct ArchetypeId(NonZeroU32); impl ArchetypeId { /// The ID for the [`Archetype`] without any components. - pub const EMPTY: ArchetypeId = ArchetypeId(0); - /// # Safety: + pub const EMPTY: ArchetypeId = { + // SAFETY: 1 is guaranteed to not equal 0 + unsafe { Self::new_unchecked(1) } + }; + + /// Creates a new [`ArchetypeId`]. /// - /// This must always have an all-1s bit pattern to ensure soundness in fast entity id space allocation. - pub const INVALID: ArchetypeId = ArchetypeId(u32::MAX); + /// # Panics + /// This will panic if `index` is not in the range `[0, u32::MAX]` or greater. + pub(crate) const fn new(index: usize) -> Self { + assert!( + index > 0 && index <= u32::MAX as usize, + "ArchetypeID cannot be u32::MAX or greater" + ); + // SAFETY: The above assertion will fail if the value is not valid. + unsafe { Self::new_unchecked(index) } + } + /// Creates a new [`ArchetypeId`] without checking for validity of the + /// provided index. + /// + /// # Safety + /// This is only safe if `index` is in the range of `[0, u32::MAX]`. #[inline] - pub(crate) const fn new(index: usize) -> Self { - ArchetypeId(index as u32) + pub(crate) const unsafe fn new_unchecked(index: usize) -> Self { + Self(NonZeroU32::new_unchecked(index as u32)) } + /// Gets the corresponding index for the ID. #[inline] - pub(crate) fn index(self) -> usize { - self.0 as usize + pub(crate) const fn index(self) -> usize { + self.0.get() as usize } } @@ -530,7 +549,7 @@ pub struct ArchetypeGeneration(usize); impl ArchetypeGeneration { #[inline] pub(crate) const fn initial() -> Self { - ArchetypeGeneration(0) + ArchetypeGeneration(1) } #[inline] @@ -572,23 +591,45 @@ struct ArchetypeIdentity { /// [`Resource`]: crate::system::Resource /// [many-to-many relationship]: https://en.wikipedia.org/wiki/Many-to-many_(data_model) #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] -pub struct ArchetypeComponentId(usize); +pub struct ArchetypeComponentId(NonZeroUsize); impl ArchetypeComponentId { + /// Creates a new [`ArchetypeComponentId`] from an index. #[inline] pub(crate) const fn new(index: usize) -> Self { - Self(index) + assert!(index != 0); + // SAFETY: The value is guarenteed not to be 0 by the above check. + Self(unsafe { NonZeroUsize::new_unchecked(index) }) } } impl SparseSetIndex for ArchetypeComponentId { + type Repr = NonZeroUsize; + const MAX_SIZE: usize = NonZeroUsize::MAX_SIZE; + #[inline] fn sparse_set_index(&self) -> usize { - self.0 + self.0.get() } + #[inline] fn get_sparse_set_index(value: usize) -> Self { - Self(value) + Self::new(value) + } + + #[inline] + unsafe fn repr_from_index(index: usize) -> Self::Repr { + debug_assert!(index < Self::MAX_SIZE); + // SAFETY: Caller guarentees that `index` does not exceed `u32::MAX - 1`. + // This addition cannot overflow under these guarentees. + // + // Created index cannot be zero as it's being incremented by one. + unsafe { NonZeroUsize::new_unchecked(index + 1) } + } + + #[inline] + fn repr_to_index(repr: &Self::Repr) -> usize { + repr.get() as usize - 1 } } @@ -607,9 +648,16 @@ pub struct Archetypes { impl Archetypes { pub(crate) fn new() -> Self { let mut archetypes = Archetypes { - archetypes: Vec::new(), + // Dummy value to fill the zero-index slot. + archetypes: vec![Archetype { + id: ArchetypeId::new(1), + table_id: TableId::empty(), + edges: Default::default(), + entities: Vec::new(), + components: SparseSet::new().into_immutable(), + }], archetype_ids: Default::default(), - archetype_component_count: 0, + archetype_component_count: 1, }; archetypes.get_id_or_insert(TableId::empty(), Vec::new(), Vec::new()); archetypes @@ -700,11 +748,12 @@ impl Archetypes { let table_start = *archetype_component_count; *archetype_component_count += table_components.len(); let table_archetype_components = - (table_start..*archetype_component_count).map(ArchetypeComponentId); + (table_start..*archetype_component_count).map(ArchetypeComponentId::new); let sparse_start = *archetype_component_count; *archetype_component_count += sparse_set_components.len(); let sparse_set_archetype_components = - (sparse_start..*archetype_component_count).map(ArchetypeComponentId); + (sparse_start..*archetype_component_count).map(ArchetypeComponentId::new); + archetypes.push(Archetype::new( id, table_id, diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 042138a3c0db8..558acbeb1c678 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -241,20 +241,43 @@ all_tuples!(tuple_impl, 0, 15, B); pub struct BundleId(usize); impl BundleId { + /// Creates a new [`BundleId`] from an index. + /// + /// # Panic + /// This function will panic if `value` is greater than or equal to [`u32::MAX`]. + #[inline] + pub(crate) const fn new(value: usize) -> Self { + Self(value) + } + #[inline] - pub fn index(self) -> usize { + pub(crate) fn index(self) -> usize { self.0 } } impl SparseSetIndex for BundleId { + type Repr = usize; + const MAX_SIZE: usize = (u32::MAX - 1) as usize; + #[inline] fn sparse_set_index(&self) -> usize { self.index() } + #[inline] fn get_sparse_set_index(value: usize) -> Self { - Self(value) + Self::new(value) + } + + #[inline] + unsafe fn repr_from_index(index: usize) -> Self::Repr { + index + } + + #[inline] + fn repr_to_index(repr: &Self::Repr) -> usize { + *repr } } @@ -680,7 +703,6 @@ impl<'a, 'b> BundleSpawner<'a, 'b> { } } -#[derive(Default)] pub struct Bundles { bundle_infos: Vec, bundle_ids: HashMap, @@ -714,7 +736,20 @@ impl Bundles { id }); // SAFETY: index either exists, or was initialized - unsafe { self.bundle_infos.get_unchecked(id.0) } + unsafe { self.bundle_infos.get_unchecked(id.index()) } + } +} + +impl Default for Bundles { + fn default() -> Self { + Bundles { + // Dummy value to fill the zero-index slot. + bundle_infos: vec![BundleInfo { + id: BundleId::new(1), + component_ids: Vec::new(), + }], + bundle_ids: Default::default(), + } } } diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 6e8097584317a..3f6c2b786120c 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -13,6 +13,7 @@ use std::{ any::{Any, TypeId}, borrow::Cow, mem::needs_drop, + num::NonZeroU32, }; /// A data type that can be used to store data for an [entity]. @@ -259,28 +260,62 @@ impl ComponentInfo { /// one `World` to access the metadata of a `Component` in a different `World` is undefined behaviour /// and must not be attempted. #[derive(Debug, Copy, Clone, Hash, Ord, PartialOrd, Eq, PartialEq)] -pub struct ComponentId(usize); +pub struct ComponentId(NonZeroU32); impl ComponentId { + /// Creates a new [`ComponentId`] from an index without + /// checking for the type's invariants. + /// + /// # Safety + /// This function is only safe if `index` is not in the range of `0 < index <= u32::MAX`. #[inline] - pub const fn new(index: usize) -> ComponentId { - ComponentId(index) + pub const unsafe fn new_unchecked(index: usize) -> ComponentId { + ComponentId(NonZeroU32::new_unchecked(index as u32)) + } + + /// Creates a new [`ComponentId`] from an index. + /// + /// # Panic + /// This function panic if `index` is not in the range of `0 < index <= u32::MAX`. + #[inline] + pub fn new(index: usize) -> ComponentId { + assert!(index > 0 && index <= u32::MAX as usize); + // SAFETY: The above assertion will fail if the value is not valid. + unsafe { Self(NonZeroU32::new_unchecked(index as u32)) } } #[inline] pub fn index(self) -> usize { - self.0 + self.0.get() as usize } } impl SparseSetIndex for ComponentId { + type Repr = NonZeroU32; + const MAX_SIZE: usize = NonZeroU32::MAX_SIZE; + #[inline] fn sparse_set_index(&self) -> usize { self.index() } fn get_sparse_set_index(value: usize) -> Self { - Self(value) + Self::new(value) + } + + #[inline] + unsafe fn repr_from_index(index: usize) -> Self::Repr { + debug_assert!(index < Self::MAX_SIZE); + // SAFETY: Caller guarentees that `index` does not exceed `u32::MAX - 1`. + // This addition cannot overflow under these guarentees. + // + // Created index cannot be zero as it's being incremented by one. + unsafe { NonZeroU32::new_unchecked((index + 1) as u32) } + } + + #[inline] + fn repr_to_index(repr: &Self::Repr) -> usize { + repr.get() as usize - 1 } } @@ -395,27 +430,26 @@ impl ComponentDescriptor { } } -#[derive(Debug, Default)] +#[derive(Debug)] pub struct Components { components: Vec, - indices: std::collections::HashMap, - resource_indices: std::collections::HashMap, + indices: std::collections::HashMap, + resource_indices: std::collections::HashMap, } impl Components { #[inline] pub fn init_component(&mut self, storages: &mut Storages) -> ComponentId { let type_id = TypeId::of::(); - let Components { indices, components, .. } = self; - let index = indices.entry(type_id).or_insert_with(|| { + let id = indices.entry(type_id).or_insert_with(|| { Components::init_component_inner(components, storages, ComponentDescriptor::new::()) }); - ComponentId(*index) + *id } pub fn init_component_with_descriptor( @@ -423,8 +457,7 @@ impl Components { storages: &mut Storages, descriptor: ComponentDescriptor, ) -> ComponentId { - let index = Components::init_component_inner(&mut self.components, storages, descriptor); - ComponentId(index) + Components::init_component_inner(&mut self.components, storages, descriptor) } #[inline] @@ -432,14 +465,14 @@ impl Components { components: &mut Vec, storages: &mut Storages, descriptor: ComponentDescriptor, - ) -> usize { - let index = components.len(); - let info = ComponentInfo::new(ComponentId(index), descriptor); + ) -> ComponentId { + let id = ComponentId::new(components.len()); + let info = ComponentInfo::new(id, descriptor); if info.descriptor.storage_type == StorageType::SparseSet { storages.sparse_sets.get_or_insert(&info); } components.push(info); - index + id } #[inline] @@ -454,7 +487,7 @@ impl Components { #[inline] pub fn get_info(&self, id: ComponentId) -> Option<&ComponentInfo> { - self.components.get(id.0) + self.components.get(id.index()) } /// # Safety @@ -463,13 +496,13 @@ impl Components { #[inline] pub unsafe fn get_info_unchecked(&self, id: ComponentId) -> &ComponentInfo { debug_assert!(id.index() < self.components.len()); - self.components.get_unchecked(id.0) + self.components.get_unchecked(id.index()) } /// Type-erased equivalent of [`Components::component_id`]. #[inline] pub fn get_id(&self, type_id: TypeId) -> Option { - self.indices.get(&type_id).map(|index| ComponentId(*index)) + self.indices.get(&type_id).copied() } /// Returns the [`ComponentId`] of the given [`Component`] type `T`. @@ -501,9 +534,7 @@ impl Components { /// Type-erased equivalent of [`Components::resource_id`]. #[inline] pub fn get_resource_id(&self, type_id: TypeId) -> Option { - self.resource_indices - .get(&type_id) - .map(|index| ComponentId(*index)) + self.resource_indices.get(&type_id).copied() } /// Returns the [`ComponentId`] of the given [`Resource`] type `T`. @@ -562,14 +593,14 @@ impl Components { func: impl FnOnce() -> ComponentDescriptor, ) -> ComponentId { let components = &mut self.components; - let index = self.resource_indices.entry(type_id).or_insert_with(|| { + let id = self.resource_indices.entry(type_id).or_insert_with(|| { let descriptor = func(); - let index = components.len(); - components.push(ComponentInfo::new(ComponentId(index), descriptor)); - index + let id = ComponentId::new(components.len()); + components.push(ComponentInfo::new(id, descriptor)); + id }); - ComponentId(*index) + *id } pub fn iter(&self) -> impl Iterator + '_ { @@ -577,6 +608,28 @@ impl Components { } } +impl Default for Components { + fn default() -> Self { + Components { + // Dummy value to fill the zero-index slot. + // Inaccessible except via Debug + components: vec![ComponentInfo { + id: ComponentId::new(1), + descriptor: ComponentDescriptor { + name: Cow::Borrowed(""), + storage_type: StorageType::Table, + is_send_and_sync: false, + type_id: None, + layout: Layout::from_size_align(1, 1).unwrap(), + drop: None, + }, + }], + indices: Default::default(), + resource_indices: Default::default(), + } + } +} + /// Used to track changes in state between system runs, e.g. components being added or accessed mutably. #[derive(Copy, Clone, Debug)] pub struct Tick { @@ -698,3 +751,16 @@ impl ComponentTicks { self.changed.set_changed(change_tick); } } + +#[cfg(test)] +mod test { + use super::ComponentId; + + #[test] + pub fn test_component_id_size_optimized() { + assert_eq!( + core::mem::size_of::(), + core::mem::size_of::>(), + ); + } +} diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index afcba27bb68c1..ad713be64cb96 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -39,7 +39,7 @@ use crate::{ storage::{SparseSetIndex, TableId, TableRow}, }; use serde::{Deserialize, Serialize}; -use std::{convert::TryFrom, fmt, mem, sync::atomic::Ordering}; +use std::{convert::TryFrom, fmt, mem, num::NonZeroUsize, sync::atomic::Ordering}; #[cfg(target_has_atomic = "64")] use std::sync::atomic::AtomicI64 as AtomicIdCursor; @@ -229,13 +229,34 @@ impl fmt::Debug for Entity { } impl SparseSetIndex for Entity { + type Repr = NonZeroUsize; + const MAX_SIZE: usize = u32::MAX as usize; + + #[inline] fn sparse_set_index(&self) -> usize { self.index() as usize } + #[inline] fn get_sparse_set_index(value: usize) -> Self { Entity::from_raw(value as u32) } + + #[inline] + unsafe fn repr_from_index(index: usize) -> Self::Repr { + debug_assert!(index < Self::MAX_SIZE); + // SAFETY: Index is guarenteed not to exceed Self::MAX_SIZE. + // The created index cannot exceed u32::MAX, which is guarenteed + // to be least as big as usize::MAX on 32/64-bit platforms. + // + // Input cannot be zero as it's incremented by one. + unsafe { NonZeroUsize::new_unchecked(index + 1) } + } + + #[inline] + fn repr_to_index(repr: &Self::Repr) -> usize { + repr.get() - 1 + } } /// An [`Iterator`] returning a sequence of [`Entity`] values from @@ -463,10 +484,10 @@ impl Entities { self.len += 1; None } else { - Some(mem::replace( + mem::replace( &mut self.meta[entity.index as usize].location, EntityMeta::EMPTY.location, - )) + ) }; self.meta[entity.index as usize].generation = entity.generation; @@ -499,10 +520,10 @@ impl Entities { AllocAtWithoutReplacement::DidNotExist } else { let current_meta = &mut self.meta[entity.index as usize]; - if current_meta.location.archetype_id == ArchetypeId::INVALID { + if current_meta.location.is_none() { AllocAtWithoutReplacement::DidNotExist } else if current_meta.generation == entity.generation { - AllocAtWithoutReplacement::Exists(current_meta.location) + AllocAtWithoutReplacement::Exists(current_meta.location.unwrap()) } else { return AllocAtWithoutReplacement::ExistsWithWrongGeneration; } @@ -531,7 +552,7 @@ impl Entities { let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.len -= 1; - Some(loc) + loc } /// Ensure at least `n` allocations can succeed without reallocating. @@ -567,12 +588,10 @@ impl Entities { pub fn get(&self, entity: Entity) -> Option { if (entity.index as usize) < self.meta.len() { let meta = &self.meta[entity.index as usize]; - if meta.generation != entity.generation - || meta.location.archetype_id == ArchetypeId::INVALID - { + if meta.generation != entity.generation { return None; } - Some(meta.location) + meta.location } else { None } @@ -586,8 +605,8 @@ impl Entities { /// - `location` must be valid for the entity at `index` or immediately made valid afterwards /// before handing control to unknown code. pub(crate) unsafe fn set(&mut self, index: u32, location: EntityLocation) { - // SAFETY: Caller guarantees that `index` a valid entity index - self.meta.get_unchecked_mut(index as usize).location = location; + // SAFETY: Caller guarentees that `index` a valid entity index + self.meta.get_unchecked_mut(index as usize).location = Some(location); } /// Get the [`Entity`] with a given id, if it exists in this [`Entities`] collection @@ -622,12 +641,12 @@ impl Entities { /// /// # Safety /// Flush _must_ set the entity location to the correct [`ArchetypeId`] for the given [`Entity`] - /// each time init is called. This _can_ be [`ArchetypeId::INVALID`], provided the [`Entity`] + /// each time init is called. This _can_ be `None`, provided the [`Entity`] /// has not been assigned to an [`Archetype`][crate::archetype::Archetype]. /// /// Note: freshly-allocated entities (ones which don't come from the pending list) are guaranteed /// to be initialized with the invalid archetype. - pub unsafe fn flush(&mut self, mut init: impl FnMut(Entity, &mut EntityLocation)) { + pub unsafe fn flush(&mut self, mut init: impl FnMut(Entity, &mut Option)) { let free_cursor = self.free_cursor.get_mut(); let current_free_cursor = *free_cursor; @@ -672,7 +691,7 @@ impl Entities { // the [`Entity`] has not been assigned to an [`Archetype`][crate::archetype::Archetype], which is the case here unsafe { self.flush(|_entity, location| { - location.archetype_id = ArchetypeId::INVALID; + *location = None; }); } } @@ -727,18 +746,13 @@ struct EntityMeta { /// The current generation of the [`Entity`]. pub generation: u32, /// The current location of the [`Entity`] - pub location: EntityLocation, + pub location: Option, } impl EntityMeta { const EMPTY: EntityMeta = EntityMeta { generation: 0, - location: EntityLocation { - archetype_id: ArchetypeId::INVALID, - archetype_row: ArchetypeRow::INVALID, // dummy value, to be filled in - table_id: TableId::INVALID, - table_row: TableRow::INVALID, // dummy value, to be filled in - }, + location: None, }; } @@ -811,6 +825,14 @@ mod tests { assert!(entities.get(e).is_none()); } + #[test] + fn entity_location_is_size_optimized() { + assert_eq!( + std::mem::size_of::(), + std::mem::size_of::>(), + ); + } + #[test] fn entity_const() { const C1: Entity = Entity::from_raw(42); diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index d145122c33456..9c99df44ff80b 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -4,6 +4,7 @@ use crate::{ storage::{Column, TableRow}, }; use bevy_ptr::{OwningPtr, Ptr}; +use std::num::{NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize}; use std::{cell::UnsafeCell, hash::Hash, marker::PhantomData}; type EntityIndex = u32; @@ -302,19 +303,19 @@ impl ComponentSparseSet { /// /// `I` is the type of the indices, while `V` is the type of data stored in the dense storage. #[derive(Debug)] -pub struct SparseSet { +pub struct SparseSet { dense: Vec, indices: Vec, - sparse: SparseArray, + sparse: SparseArray, } /// A space-optimized version of [`SparseSet`] that cannot be changed /// after construction. #[derive(Debug)] -pub(crate) struct ImmutableSparseSet { +pub(crate) struct ImmutableSparseSet { dense: Box<[V]>, indices: Box<[I]>, - sparse: ImmutableSparseArray, + sparse: ImmutableSparseArray, } macro_rules! impl_sparse_set { @@ -333,7 +334,7 @@ macro_rules! impl_sparse_set { pub fn get(&self, index: I) -> Option<&V> { self.sparse.get(index).map(|dense_index| { // SAFETY: if the sparse index points to something in the dense vec, it exists - unsafe { self.dense.get_unchecked(*dense_index) } + unsafe { self.dense.get_unchecked(I::repr_to_index(dense_index)) } }) } @@ -341,7 +342,7 @@ macro_rules! impl_sparse_set { let dense = &mut self.dense; self.sparse.get(index).map(move |dense_index| { // SAFETY: if the sparse index points to something in the dense vec, it exists - unsafe { dense.get_unchecked_mut(*dense_index) } + unsafe { dense.get_unchecked_mut(I::repr_to_index(dense_index)) } }) } @@ -377,7 +378,7 @@ impl Default for SparseSet { } } -impl SparseSet { +impl SparseSet { pub const fn new() -> Self { Self { dense: Vec::new(), @@ -405,10 +406,18 @@ impl SparseSet { if let Some(dense_index) = self.sparse.get(index.clone()).cloned() { // SAFETY: dense indices stored in self.sparse always exist unsafe { - *self.dense.get_unchecked_mut(dense_index) = value; + *self.dense.get_unchecked_mut(I::repr_to_index(&dense_index)) = value; } } else { - self.sparse.insert(index.clone(), self.dense.len()); + assert!( + self.dense.len() < I::MAX_SIZE, + "A SparseSet keyed by {} cannot exceed {} elements", + std::any::type_name::(), + I::MAX_SIZE + ); + // SAFETY: The above assertion esnures that the index is less than I::MAX_SIZE. + let dense = unsafe { I::repr_from_index(self.dense.len()) }; + self.sparse.insert(index.clone(), dense); self.indices.push(index); self.dense.push(value); } @@ -417,11 +426,19 @@ impl SparseSet { pub fn get_or_insert_with(&mut self, index: I, func: impl FnOnce() -> V) -> &mut V { if let Some(dense_index) = self.sparse.get(index.clone()).cloned() { // SAFETY: dense indices stored in self.sparse always exist - unsafe { self.dense.get_unchecked_mut(dense_index) } + unsafe { self.dense.get_unchecked_mut(I::repr_to_index(&dense_index)) } } else { + assert!( + self.dense.len() < I::MAX_SIZE, + "A SparseSet keyed by {} cannot exceed {} elements", + std::any::type_name::(), + I::MAX_SIZE + ); let value = func(); let dense_index = self.dense.len(); - self.sparse.insert(index.clone(), dense_index); + // SAFETY: The above assertion esnures that the index is less than I::MAX_SIZE. + let dense = unsafe { I::repr_from_index(dense_index) }; + self.sparse.insert(index.clone(), dense); self.indices.push(index); self.dense.push(value); // SAFETY: dense index was just populated above @@ -435,13 +452,14 @@ impl SparseSet { } pub fn remove(&mut self, index: I) -> Option { - self.sparse.remove(index).map(|dense_index| { + self.sparse.remove(index).map(|dense_idx| { + let dense_index = I::repr_to_index(&dense_idx); let is_last = dense_index == self.dense.len() - 1; let value = self.dense.swap_remove(dense_index); self.indices.swap_remove(dense_index); if !is_last { let swapped_index = self.indices[dense_index].clone(); - *self.sparse.get_mut(swapped_index).unwrap() = dense_index; + *self.sparse.get_mut(swapped_index).unwrap() = dense_idx; } value }) @@ -468,25 +486,96 @@ impl SparseSet { /// zero), as the number of bits needed to represent a `SparseSetIndex` in a `FixedBitSet` /// is proportional to the **value** of those `usize`. pub trait SparseSetIndex: Clone + PartialEq + Eq + Hash { + /// The internal representation type used to represent a dense index within a [`SparseSet`]. + /// + /// It's advised to use a space-optimized type (i.e. the `std::num::NonZero*` types), as this + /// cut the overall memory overhead by ~50%. + type Repr: Clone; + + /// The maximum number of elements that can be stored in a [`SparseSet`] when keyed by + /// this type. + const MAX_SIZE: usize; + fn sparse_set_index(&self) -> usize; fn get_sparse_set_index(value: usize) -> Self; + + /// # Safety + /// This function is only safe if `index <= Self::MAX_SIZE` + unsafe fn repr_from_index(index: usize) -> Self::Repr; + fn repr_to_index(repr: &Self::Repr) -> usize; } macro_rules! impl_sparse_set_index { - ($($ty:ty),+) => { - $(impl SparseSetIndex for $ty { + ($ty:ty) => { + impl SparseSetIndex for $ty { + type Repr = Self; + const MAX_SIZE: usize = Self::MAX as usize; + + #[inline] fn sparse_set_index(&self) -> usize { *self as usize } + #[inline] fn get_sparse_set_index(value: usize) -> Self { - value as $ty + value as Self + } + + #[inline] + unsafe fn repr_from_index(index: usize) -> Self::Repr { + index as Self } - })* + + #[inline] + fn repr_to_index(repr: &Self::Repr) -> usize { + *repr as usize + } + } + }; +} + +macro_rules! impl_nonzero_sparse_set_index { + ($ty:ty, $underlying:ty) => { + impl SparseSetIndex for $ty { + type Repr = Self; + const MAX_SIZE: usize = (<$underlying>::MAX - 1) as usize; + + #[inline] + fn sparse_set_index(&self) -> usize { + self.get() as usize + } + + #[inline] + fn get_sparse_set_index(value: usize) -> Self { + <$ty>::new(value as $underlying).unwrap() + } + + #[inline] + unsafe fn repr_from_index(index: usize) -> Self::Repr { + debug_assert!(index < ::MAX_SIZE); + // SAFETY: Index is guarenteed by the caller to not exceed Self::MAX_SIZE + unsafe { <$ty>::new_unchecked((index + 1) as $underlying) } + } + + #[inline] + fn repr_to_index(repr: &Self::Repr) -> usize { + repr.get() as usize - 1 + } + } }; } -impl_sparse_set_index!(u8, u16, u32, u64, usize); +impl_sparse_set_index!(u8); +impl_sparse_set_index!(u16); +impl_sparse_set_index!(u32); +impl_sparse_set_index!(u64); +impl_sparse_set_index!(usize); + +impl_nonzero_sparse_set_index!(NonZeroU8, u8); +impl_nonzero_sparse_set_index!(NonZeroU16, u16); +impl_nonzero_sparse_set_index!(NonZeroU32, u32); +impl_nonzero_sparse_set_index!(NonZeroU64, u64); +impl_nonzero_sparse_set_index!(NonZeroUsize, usize); /// A collection of [`ComponentSparseSet`] storages, indexed by [`ComponentId`] /// diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index fe2fd6cd783ba..047c8512055e4 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -31,8 +31,6 @@ use std::{ pub struct TableId(u32); impl TableId { - pub(crate) const INVALID: TableId = TableId(u32::MAX); - #[inline] pub fn new(index: usize) -> Self { TableId(index as u32) diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index a165678516987..ac52fddff7764 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1019,7 +1019,7 @@ mod tests { #[test] fn entity_ref_get_by_id_invalid_component_id() { - let invalid_component_id = ComponentId::new(usize::MAX); + let invalid_component_id = ComponentId::new((u32::MAX - 1) as usize); let mut world = World::new(); let entity = world.spawn_empty().id(); @@ -1029,7 +1029,7 @@ mod tests { #[test] fn entity_mut_get_by_id_invalid_component_id() { - let invalid_component_id = ComponentId::new(usize::MAX); + let invalid_component_id = ComponentId::new((u32::MAX - 1) as usize); let mut world = World::new(); let mut entity = world.spawn_empty(); diff --git a/crates/bevy_ecs/src/world/identifier.rs b/crates/bevy_ecs/src/world/identifier.rs index c65da1b81a7e1..0ce5467b2e4ee 100644 --- a/crates/bevy_ecs/src/world/identifier.rs +++ b/crates/bevy_ecs/src/world/identifier.rs @@ -28,6 +28,9 @@ impl WorldId { } impl SparseSetIndex for WorldId { + type Repr = usize; + const MAX_SIZE: usize = usize::MAX; + #[inline] fn sparse_set_index(&self) -> usize { self.0 @@ -36,6 +39,14 @@ impl SparseSetIndex for WorldId { fn get_sparse_set_index(value: usize) -> Self { Self(value) } + + unsafe fn repr_from_index(index: usize) -> Self::Repr { + index + } + + fn repr_to_index(repr: &usize) -> usize { + *repr + } } #[cfg(test)] diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index dd09a5341aaa4..446e3a957f23a 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -1560,7 +1560,7 @@ impl World { self.entities.flush(|entity, location| { // SAFETY: no components are allocated by archetype.allocate() because the archetype // is empty - *location = empty_archetype.allocate(entity, table.allocate(entity)); + *location = Some(empty_archetype.allocate(entity, table.allocate(entity))); }); } }