From 5e595c5fd97f9202e9ada24018de400a6d4573cb Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sat, 11 Nov 2023 13:37:23 -0800 Subject: [PATCH] Speed up `Entity` by making it `u64` --- crates/bevy_ecs/src/entity/map_entities.rs | 9 +- crates/bevy_ecs/src/entity/mod.rs | 174 ++++++++++----------- 2 files changed, 83 insertions(+), 100 deletions(-) diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index e109b43dd5667..d8a7fffaaca61 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -69,10 +69,9 @@ impl<'m> EntityMapper<'m> { } // this new entity reference is specifically designed to never represent any living entity - let new = Entity { - generation: self.dead_start.generation + self.generations, - index: self.dead_start.index, - }; + let generation = self.dead_start.generation() + self.generations; + let index = self.dead_start.index(); + let new = Entity::from_raw_and_generation(index, generation); self.generations += 1; self.map.insert(entity, new); @@ -108,7 +107,7 @@ impl<'m> EntityMapper<'m> { // SAFETY: Entities data is kept in a valid state via `EntityMap::world_scope` let entities = unsafe { world.entities_mut() }; assert!(entities.free(self.dead_start).is_some()); - assert!(entities.reserve_generations(self.dead_start.index, self.generations)); + assert!(entities.reserve_generations(self.dead_start.index(), self.generations)); } /// Creates an [`EntityMapper`] from a provided [`World`] and [`EntityHashMap`], then calls the diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index b4543d8d1a810..1e9bc52dcca73 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -115,17 +115,13 @@ type IdCursor = isize; /// [`EntityCommands`]: crate::system::EntityCommands /// [`Query::get`]: crate::system::Query::get /// [`World`]: crate::world::World -#[derive(Clone, Copy, Eq, Ord, PartialEq, PartialOrd)] +#[derive(Clone, Copy, Hash, Eq, Ord, PartialEq, PartialOrd)] pub struct Entity { - generation: u32, - index: u32, -} - -impl Hash for Entity { - #[inline] - fn hash(&self, state: &mut H) { - self.to_bits().hash(state); - } + // Due to , two-field + // `derive(Eq)` is worse than we'd like. So instead store both the index + // and generation in one `u64`, with the generation in the high half + // so that the ordering matches what the `derive(Ord)` was doing before. + bits: u64, } pub(crate) enum AllocAtWithoutReplacement { @@ -137,7 +133,7 @@ pub(crate) enum AllocAtWithoutReplacement { impl Entity { #[cfg(test)] pub(crate) const fn new(index: u32, generation: u32) -> Entity { - Entity { index, generation } + Entity::from_raw_and_generation(index, generation) } /// An entity ID with a placeholder value. This may or may not correspond to an actual entity, @@ -189,10 +185,18 @@ impl Entity { /// `Entity` lines up between instances, but instead insert a secondary identifier as /// a component. pub const fn from_raw(index: u32) -> Entity { - Entity { - index, - generation: 0, - } + Entity::from_raw_and_generation(index, 0) + } + + #[inline] + const fn from_raw_and_generation(index: u32, generation: u32) -> Entity { + let bits = (generation as u64) << 32 | index as u64; + Entity { bits } + } + + #[inline] + const fn from_raw_and_meta(index: u32, meta: EntityMeta) -> Entity { + Entity::from_raw_and_generation(index, meta.generation) } /// Convert to a form convenient for passing outside of rust. @@ -202,17 +206,14 @@ impl Entity { /// /// No particular structure is guaranteed for the returned bits. pub const fn to_bits(self) -> u64 { - (self.generation as u64) << 32 | self.index as u64 + self.bits } /// Reconstruct an `Entity` previously destructured with [`Entity::to_bits`]. /// /// Only useful when applied to results from `to_bits` in the same instance of an application. pub const fn from_bits(bits: u64) -> Self { - Self { - generation: (bits >> 32) as u32, - index: bits as u32, - } + Entity { bits } } /// Return a transiently unique identifier. @@ -222,7 +223,7 @@ impl Entity { /// specific snapshot of the world, such as when serializing. #[inline] pub const fn index(self) -> u32 { - self.index + self.bits as u32 } /// Returns the generation of this Entity's index. The generation is incremented each time an @@ -230,7 +231,7 @@ impl Entity { /// given index has been reused (index, generation) pairs uniquely identify a given Entity. #[inline] pub const fn generation(self) -> u32 { - self.generation + (self.bits >> 32) as u32 } } @@ -255,7 +256,7 @@ impl<'de> Deserialize<'de> for Entity { impl fmt::Debug for Entity { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}v{}", self.index, self.generation) + write!(f, "{}v{}", self.index(), self.generation()) } } @@ -290,16 +291,8 @@ impl<'a> Iterator for ReserveEntitiesIterator<'a> { fn next(&mut self) -> Option { self.index_iter .next() - .map(|&index| Entity { - generation: self.meta[index as usize].generation, - index, - }) - .or_else(|| { - self.index_range.next().map(|index| Entity { - generation: 0, - index, - }) - }) + .map(|&index| Entity::from_raw_and_meta(index, self.meta[index as usize])) + .or_else(|| self.index_range.next().map(|index| Entity::from_raw(index))) } fn size_hint(&self) -> (usize, Option) { @@ -433,20 +426,15 @@ impl Entities { if n > 0 { // Allocate from the freelist. let index = self.pending[(n - 1) as usize]; - Entity { - generation: self.meta[index as usize].generation, - index, - } + Entity::from_raw_and_meta(index, self.meta[index as usize]) } else { // Grab a new ID, outside the range of `meta.len()`. `flush()` must // eventually be called to make it valid. // // As `self.free_cursor` goes more and more negative, we return IDs farther // and farther beyond `meta.len()`. - Entity { - generation: 0, - index: u32::try_from(self.meta.len() as IdCursor - n).expect("too many entities"), - } + let index = u32::try_from(self.meta.len() as IdCursor - n).expect("too many entities"); + Entity::from_raw(index) } } @@ -465,17 +453,11 @@ impl Entities { if let Some(index) = self.pending.pop() { let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; - Entity { - generation: self.meta[index as usize].generation, - index, - } + Entity::from_raw_and_meta(index, self.meta[index as usize]) } else { let index = u32::try_from(self.meta.len()).expect("too many entities"); self.meta.push(EntityMeta::EMPTY); - Entity { - generation: 0, - index, - } + Entity::from_raw(index) } } @@ -486,15 +468,16 @@ impl Entities { pub fn alloc_at(&mut self, entity: Entity) -> Option { self.verify_flushed(); - let loc = if entity.index as usize >= self.meta.len() { - self.pending.extend((self.meta.len() as u32)..entity.index); + let loc = if entity.index() as usize >= self.meta.len() { + self.pending + .extend((self.meta.len() as u32)..entity.index()); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.meta - .resize(entity.index as usize + 1, EntityMeta::EMPTY); + .resize(entity.index() as usize + 1, EntityMeta::EMPTY); self.len += 1; None - } else if let Some(index) = self.pending.iter().position(|item| *item == entity.index) { + } else if let Some(index) = self.pending.iter().position(|item| *item == entity.index()) { self.pending.swap_remove(index); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; @@ -502,12 +485,12 @@ impl Entities { None } else { Some(mem::replace( - &mut self.meta[entity.index as usize].location, + &mut self.meta[entity.index() as usize].location, EntityMeta::EMPTY.location, )) }; - self.meta[entity.index as usize].generation = entity.generation; + self.meta[entity.index() as usize].generation = entity.generation(); loc } @@ -521,32 +504,33 @@ impl Entities { ) -> AllocAtWithoutReplacement { self.verify_flushed(); - let result = if entity.index as usize >= self.meta.len() { - self.pending.extend((self.meta.len() as u32)..entity.index); + let result = if entity.index() as usize >= self.meta.len() { + self.pending + .extend((self.meta.len() as u32)..entity.index()); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.meta - .resize(entity.index as usize + 1, EntityMeta::EMPTY); + .resize(entity.index() as usize + 1, EntityMeta::EMPTY); self.len += 1; AllocAtWithoutReplacement::DidNotExist - } else if let Some(index) = self.pending.iter().position(|item| *item == entity.index) { + } else if let Some(index) = self.pending.iter().position(|item| *item == entity.index()) { self.pending.swap_remove(index); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.len += 1; AllocAtWithoutReplacement::DidNotExist } else { - let current_meta = &self.meta[entity.index as usize]; + let current_meta = &self.meta[entity.index() as usize]; if current_meta.location.archetype_id == ArchetypeId::INVALID { AllocAtWithoutReplacement::DidNotExist - } else if current_meta.generation == entity.generation { + } else if current_meta.generation == entity.generation() { AllocAtWithoutReplacement::Exists(current_meta.location) } else { return AllocAtWithoutReplacement::ExistsWithWrongGeneration; } }; - self.meta[entity.index as usize].generation = entity.generation; + self.meta[entity.index() as usize].generation = entity.generation(); result } @@ -556,15 +540,15 @@ impl Entities { pub fn free(&mut self, entity: Entity) -> Option { self.verify_flushed(); - let meta = &mut self.meta[entity.index as usize]; - if meta.generation != entity.generation { + let meta = &mut self.meta[entity.index() as usize]; + if meta.generation != entity.generation() { return None; } meta.generation += 1; let loc = mem::replace(&mut meta.location, EntityMeta::EMPTY.location); - self.pending.push(entity.index); + self.pending.push(entity.index()); let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; @@ -590,7 +574,7 @@ impl Entities { // not reallocated since the generation is incremented in `free` pub fn contains(&self, entity: Entity) -> bool { self.resolve_from_id(entity.index()) - .map_or(false, |e| e.generation() == entity.generation) + .map_or(false, |e| e.generation() == entity.generation()) } /// Clears all [`Entity`] from the World. @@ -605,8 +589,8 @@ impl Entities { /// Note: for pending entities, returns `Some(EntityLocation::INVALID)`. #[inline] pub fn get(&self, entity: Entity) -> Option { - if let Some(meta) = self.meta.get(entity.index as usize) { - if meta.generation != entity.generation + if let Some(meta) = self.meta.get(entity.index() as usize) { + if meta.generation != entity.generation() || meta.location.archetype_id == ArchetypeId::INVALID { return None; @@ -657,18 +641,15 @@ impl Entities { /// entities, since it checks the generation pub fn resolve_from_id(&self, index: u32) -> Option { let idu = index as usize; - if let Some(&EntityMeta { generation, .. }) = self.meta.get(idu) { - Some(Entity { generation, index }) + if let Some(&meta) = self.meta.get(idu) { + Some(Entity::from_raw_and_meta(index, meta)) } else { // `id` is outside of the meta list - check whether it is reserved but not yet flushed. let free_cursor = self.free_cursor.load(Ordering::Relaxed); // If this entity was manually created, then free_cursor might be positive // Returning None handles that case correctly let num_pending = usize::try_from(-free_cursor).ok()?; - (idu < self.meta.len() + num_pending).then_some(Entity { - generation: 0, - index, - }) + (idu < self.meta.len() + num_pending).then_some(Entity::from_raw(index)) } } @@ -699,10 +680,7 @@ impl Entities { self.len += -current_free_cursor as u32; for (index, meta) in self.meta.iter_mut().enumerate().skip(old_meta_len) { init( - Entity { - index: index as u32, - generation: meta.generation, - }, + Entity::from_raw_and_meta(index as u32, *meta), &mut meta.location, ); } @@ -714,13 +692,7 @@ impl Entities { self.len += (self.pending.len() - new_free_cursor) as u32; for index in self.pending.drain(new_free_cursor..) { let meta = &mut self.meta[index as usize]; - init( - Entity { - index, - generation: meta.generation, - }, - &mut meta.location, - ); + init(Entity::from_raw_and_meta(index, *meta), &mut meta.location); } } @@ -841,11 +813,23 @@ mod tests { use super::*; #[test] - fn entity_bits_roundtrip() { - let e = Entity { + fn entity_constructor() { + let e = Entity::from_raw_and_generation(0xBAADF00D, 0xDEADBEEF); + assert_eq!(e.index(), 0xBAADF00D); + assert_eq!(e.generation(), 0xDEADBEEF); + + let meta = EntityMeta { generation: 0xDEADBEEF, - index: 0xBAADF00D, + location: EntityLocation::INVALID, }; + let e = Entity::from_raw_and_meta(0xBAADF00D, meta); + assert_eq!(e.index(), 0xBAADF00D); + assert_eq!(e.generation(), 0xDEADBEEF); + } + + #[test] + fn entity_bits_roundtrip() { + let e = Entity::from_raw_and_generation(0xBAADF00D, 0xDEADBEEF); assert_eq!(Entity::from_bits(e.to_bits()), e); } @@ -879,12 +863,12 @@ mod tests { #[test] fn entity_const() { const C1: Entity = Entity::from_raw(42); - assert_eq!(42, C1.index); - assert_eq!(0, C1.generation); + assert_eq!(42, C1.index()); + assert_eq!(0, C1.generation()); const C2: Entity = Entity::from_bits(0x0000_00ff_0000_00cc); - assert_eq!(0x0000_00cc, C2.index); - assert_eq!(0x0000_00ff, C2.generation); + assert_eq!(0x0000_00cc, C2.index()); + assert_eq!(0x0000_00ff, C2.generation()); const C3: u32 = Entity::from_raw(33).index(); assert_eq!(33, C3); @@ -905,7 +889,7 @@ mod tests { let entity = entities.alloc(); entities.free(entity); - assert!(entities.reserve_generations(entity.index, 1)); + assert!(entities.reserve_generations(entity.index(), 1)); } #[test] @@ -916,12 +900,12 @@ mod tests { let entity = entities.alloc(); entities.free(entity); - assert!(entities.reserve_generations(entity.index, GENERATIONS)); + assert!(entities.reserve_generations(entity.index(), GENERATIONS)); // The very next entity allocated should be a further generation on the same index let next_entity = entities.alloc(); assert_eq!(next_entity.index(), entity.index()); - assert!(next_entity.generation > entity.generation + GENERATIONS); + assert!(next_entity.generation() > entity.generation() + GENERATIONS); } #[test]