forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Use T::Storage::STORAGE_TYPE to optimize out unused branches (#6800)

Browse Source 
# Objective
`EntityRef::get` and friends all type erase calls to fetch the target components by using passing in the `TypeId` instead of using generics. This is forcing a lookup to `Components` to fetch the storage type. This adds an extra memory lookup and forces a runtime branch instead of allowing the compiler to optimize out the unused branch.

## Solution
Leverage `Component::Storage::STORAGE_TYPE` as a constant instead of fetching the metadata from `Components`.

## Performance
This has a near 2x speedup for all calls to `World::get`. Microbenchmark results from my local machine. `Query::get_component`, which uses `EntityRef::get` internally also show a slight speed up. This has closed the gap between `World::get` and `Query::get` for the same use case.

```
group                                                             entity-ref-generics                     main
-----                                                             -------------------                     ----
query_get_component/50000_entities_sparse                         1.00   890.6±40.42µs        ? ?/sec     1.10   980.6±28.22µs        ? ?/sec
query_get_component/50000_entities_table                          1.00   968.5±73.73µs        ? ?/sec     1.08  1048.8±31.76µs        ? ?/sec
query_get_component_simple/system                                 1.00    703.2±4.37µs        ? ?/sec     1.00    702.1±6.13µs        ? ?/sec
query_get_component_simple/unchecked                              1.02    855.8±8.98µs        ? ?/sec     1.00    843.1±8.19µs        ? ?/sec
world_get/50000_entities_sparse                                   1.00    202.3±3.15µs        ? ?/sec     1.85   374.0±20.96µs        ? ?/sec
world_get/50000_entities_table                                    1.00    193.0±1.78µs        ? ?/sec     2.02   389.2±26.55µs        ? ?/sec
world_query_get/50000_entities_sparse                             1.01    162.4±2.23µs        ? ?/sec     1.00    161.3±0.95µs        ? ?/sec
world_query_get/50000_entities_table                              1.00    199.9±0.63µs        ? ?/sec     1.00    200.2±0.74µs        ? ?/sec
```

This should also, by proxy, speed up the `ReflectComponent` APIs as most of those use `World::get` variants internally.
This commit is contained in:
James Liu 2022-12-05 23:56:33 +00:00
parent f9c52f98b9
commit a3f203b504
2 changed files with 238 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

292
crates/bevy_ecs/src/world/entity_ref.rs
View File

@ -2,9 +2,12 @@ use crate::{
archetype::{Archetype, ArchetypeId, Archetypes}, archetype::{Archetype, ArchetypeId, Archetypes},
bundle::{Bundle, BundleInfo}, bundle::{Bundle, BundleInfo},
change_detection::{MutUntyped, Ticks}, change_detection::{MutUntyped, Ticks},
component::{Component, ComponentId, ComponentTicks, Components, StorageType, TickCells}, component::{
Component, ComponentId, ComponentStorage, ComponentTicks, Components, StorageType,
TickCells,
},
entity::{Entities, Entity, EntityLocation}, entity::{Entities, Entity, EntityLocation},
storage::{SparseSet, Storages}, storage::{Column, ComponentSparseSet, SparseSet, Storages},
world::{Mut, World}, world::{Mut, World},
}; };
use bevy_ptr::{OwningPtr, Ptr}; use bevy_ptr::{OwningPtr, Ptr};
@ -67,9 +70,18 @@ impl<'w> EntityRef<'w> {
#[inline] #[inline]
pub fn get<T: Component>(&self) -> Option<&'w T> { pub fn get<T: Component>(&self) -> Option<&'w T> {
// SAFETY: entity location is valid and returned component is of type T // SAFETY:
// - entity location and entity is valid
// - returned component is of type T
// - the storage type provided is correct for T
unsafe { unsafe {
get_component_with_type(self.world, TypeId::of::<T>(), self.entity, self.location) get_component_with_type(
self.world,
TypeId::of::<T>(),
T::Storage::STORAGE_TYPE,
self.entity,
self.location,
)
.map(|value| value.deref::<T>()) .map(|value| value.deref::<T>())
} }
} }
@ -78,8 +90,18 @@ impl<'w> EntityRef<'w> {
/// detection in custom runtimes. /// detection in custom runtimes.
#[inline] #[inline]
pub fn get_change_ticks<T: Component>(&self) -> Option<ComponentTicks> { pub fn get_change_ticks<T: Component>(&self) -> Option<ComponentTicks> {
// SAFETY: entity location is valid // SAFETY:
unsafe { get_ticks_with_type(self.world, TypeId::of::<T>(), self.entity, self.location) } // - entity location and entity is valid
// - the storage type provided is correct for T
unsafe {
get_ticks_with_type(
self.world,
TypeId::of::<T>(),
T::Storage::STORAGE_TYPE,
self.entity,
self.location,
)
}
} }
/// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change /// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change
@ -94,8 +116,17 @@ impl<'w> EntityRef<'w> {
return None; return None;
} }
let info = self.world.components().get_info(component_id)?;
// SAFETY: Entity location is valid and component_id exists. // SAFETY: Entity location is valid and component_id exists.
unsafe { get_ticks(self.world, component_id, self.entity, self.location) } unsafe {
get_ticks(
self.world,
component_id,
info.storage_type(),
self.entity,
self.location,
)
}
} }
/// Gets a mutable reference to the component of type `T` associated with /// Gets a mutable reference to the component of type `T` associated with
@ -114,7 +145,17 @@ impl<'w> EntityRef<'w> {
last_change_tick: u32, last_change_tick: u32,
change_tick: u32, change_tick: u32,
) -> Option<Mut<'w, T>> { ) -> Option<Mut<'w, T>> {
get_component_and_ticks_with_type(self.world, TypeId::of::<T>(), self.entity, self.location) // SAFETY:
// - entity location and entity is valid
// - returned component is of type T
// - the storage type provided is correct for T
get_component_and_ticks_with_type(
self.world,
TypeId::of::<T>(),
T::Storage::STORAGE_TYPE,
self.entity,
self.location,
)
.map(|(value, ticks)| Mut { .map(|(value, ticks)| Mut {
value: value.assert_unique().deref_mut::<T>(), value: value.assert_unique().deref_mut::<T>(),
ticks: Ticks::from_tick_cells(ticks, last_change_tick, change_tick), ticks: Ticks::from_tick_cells(ticks, last_change_tick, change_tick),
@ -133,9 +174,20 @@ impl<'w> EntityRef<'w> {
/// which is only valid while the `'w` borrow of the lifetime is active. /// which is only valid while the `'w` borrow of the lifetime is active.
#[inline] #[inline]
pub fn get_by_id(&self, component_id: ComponentId) -> Option<Ptr<'w>> { pub fn get_by_id(&self, component_id: ComponentId) -> Option<Ptr<'w>> {
self.world.components().get_info(component_id)?; let info = self.world.components().get_info(component_id)?;
// SAFETY: entity_location is valid, component_id is valid as checked by the line above // SAFETY:
unsafe { get_component(self.world, component_id, self.entity, self.location) } // - entity_location is valid,
// - component_id is valid as checked by the line above
// - the storage type is accurate as checked by the fetched ComponentInfo
unsafe {
get_component(
self.world,
component_id,
info.storage_type(),
self.entity,
self.location,
)
}
} }
} }
@ -201,9 +253,19 @@ impl<'w> EntityMut<'w> {
#[inline] #[inline]
pub fn get<T: Component>(&self) -> Option<&'_ T> { pub fn get<T: Component>(&self) -> Option<&'_ T> {
// SAFETY: lifetimes enforce correct usage of returned borrow // SAFETY:
// - lifetimes enforce correct usage of returned borrow
// - entity location and entity is valid
// - returned component is of type T
// - the storage type provided is correct for T
unsafe { unsafe {
get_component_with_type(self.world, TypeId::of::<T>(), self.entity, self.location) get_component_with_type(
self.world,
TypeId::of::<T>(),
T::Storage::STORAGE_TYPE,
self.entity,
self.location,
)
.map(|value| value.deref::<T>()) .map(|value| value.deref::<T>())
} }
} }
@ -218,8 +280,18 @@ impl<'w> EntityMut<'w> {
/// detection in custom runtimes. /// detection in custom runtimes.
#[inline] #[inline]
pub fn get_change_ticks<T: Component>(&self) -> Option<ComponentTicks> { pub fn get_change_ticks<T: Component>(&self) -> Option<ComponentTicks> {
// SAFETY: entity location is valid // SAFETY:
unsafe { get_ticks_with_type(self.world, TypeId::of::<T>(), self.entity, self.location) } // - entity location and entity is valid
// - the storage type provided is correct for T
unsafe {
get_ticks_with_type(
self.world,
TypeId::of::<T>(),
T::Storage::STORAGE_TYPE,
self.entity,
self.location,
)
}
} }
/// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change /// Retrieves the change ticks for the given [`ComponentId`]. This can be useful for implementing change
@ -234,8 +306,17 @@ impl<'w> EntityMut<'w> {
return None; return None;
} }
let info = self.world.components().get_info(component_id)?;
// SAFETY: Entity location is valid and component_id exists. // SAFETY: Entity location is valid and component_id exists.
unsafe { get_ticks(self.world, component_id, self.entity, self.location) } unsafe {
get_ticks(
self.world,
component_id,
info.storage_type(),
self.entity,
self.location,
)
}
} }
/// Gets a mutable reference to the component of type `T` associated with /// Gets a mutable reference to the component of type `T` associated with
@ -250,7 +331,17 @@ impl<'w> EntityMut<'w> {
/// operation on this world (non-exhaustive list). /// operation on this world (non-exhaustive list).
#[inline] #[inline]
pub unsafe fn get_unchecked_mut<T: Component>(&self) -> Option<Mut<'_, T>> { pub unsafe fn get_unchecked_mut<T: Component>(&self) -> Option<Mut<'_, T>> {
get_component_and_ticks_with_type(self.world, TypeId::of::<T>(), self.entity, self.location) // SAFETY:
// - entity location and entity is valid
// - returned component is of type T
// - the storage type provided is correct for T
get_component_and_ticks_with_type(
self.world,
TypeId::of::<T>(),
T::Storage::STORAGE_TYPE,
self.entity,
self.location,
)
.map(|(value, ticks)| Mut { .map(|(value, ticks)| Mut {
value: value.assert_unique().deref_mut::<T>(), value: value.assert_unique().deref_mut::<T>(),
ticks: Ticks::from_tick_cells( ticks: Ticks::from_tick_cells(
@ -573,9 +664,20 @@ impl<'w> EntityMut<'w> {
/// which is only valid while the [`EntityMut`] is alive. /// which is only valid while the [`EntityMut`] is alive.
#[inline] #[inline]
pub fn get_by_id(&self, component_id: ComponentId) -> Option<Ptr<'_>> { pub fn get_by_id(&self, component_id: ComponentId) -> Option<Ptr<'_>> {
self.world.components().get_info(component_id)?; let info = self.world.components().get_info(component_id)?;
// SAFETY: entity_location is valid, component_id is valid as checked by the line above // SAFETY:
unsafe { get_component(self.world, component_id, self.entity, self.location) } // - entity_location is valid
// - component_id is valid as checked by the line above
// - the storage type is accurate as checked by the fetched ComponentInfo
unsafe {
get_component(
self.world,
component_id,
info.storage_type(),
self.entity,
self.location,
)
}
} }
/// Gets a [`MutUntyped`] of the component of the given [`ComponentId`] from the entity. /// Gets a [`MutUntyped`] of the component of the given [`ComponentId`] from the entity.
@ -594,6 +696,24 @@ impl<'w> EntityMut<'w> {
} }
} }
#[inline]
fn fetch_table(
world: &World,
location: EntityLocation,
component_id: ComponentId,
) -> Option<(&Column, usize)> {
let archetype = &world.archetypes[location.archetype_id];
let table = &world.storages.tables[archetype.table_id()];
let components = table.get_column(component_id)?;
let table_row = archetype.entity_table_row(location.index);
Some((components, table_row))
}
#[inline]
fn fetch_sparse_set(world: &World, component_id: ComponentId) -> Option<&ComponentSparseSet> {
world.storages.sparse_sets.get(component_id)
}
// TODO: move to Storages? // TODO: move to Storages?
/// Get a raw pointer to a particular [`Component`] on a particular [`Entity`] in the provided [`World`]. /// Get a raw pointer to a particular [`Component`] on a particular [`Entity`] in the provided [`World`].
/// ///
@ -601,29 +721,22 @@ impl<'w> EntityMut<'w> {
/// - `entity_location` must be within bounds of the given archetype and `entity` must exist inside /// - `entity_location` must be within bounds of the given archetype and `entity` must exist inside
/// the archetype /// the archetype
/// - `component_id` must be valid /// - `component_id` must be valid
/// - `storage_type` must accurately reflect where the components for `component_id` are stored.
#[inline] #[inline]
pub(crate) unsafe fn get_component( pub(crate) unsafe fn get_component(
world: &World, world: &World,
component_id: ComponentId, component_id: ComponentId,
storage_type: StorageType,
entity: Entity, entity: Entity,
location: EntityLocation, location: EntityLocation,
) -> Option<Ptr<'_>> { ) -> Option<Ptr<'_>> {
let archetype = &world.archetypes[location.archetype_id]; match storage_type {
// SAFETY: component_id exists and is therefore valid
let component_info = world.components.get_info_unchecked(component_id);
match component_info.storage_type() {
StorageType::Table => { StorageType::Table => {
let table = &world.storages.tables[archetype.table_id()]; let (components, table_row) = fetch_table(world, location, component_id)?;
let components = table.get_column(component_id)?;
let table_row = archetype.entity_table_row(location.index);
// SAFETY: archetypes only store valid table_rows and the stored component type is T // SAFETY: archetypes only store valid table_rows and the stored component type is T
Some(components.get_data_unchecked(table_row)) Some(components.get_data_unchecked(table_row))
} }
StorageType::SparseSet => world StorageType::SparseSet => fetch_sparse_set(world, component_id)?.get(entity),
.storages
.sparse_sets
.get(component_id)
.and_then(|sparse_set| sparse_set.get(entity)),
} }
} }
@ -631,21 +744,21 @@ pub(crate) unsafe fn get_component(
/// Get a raw pointer to the [`ComponentTicks`] of a particular [`Component`] on a particular [`Entity`] in the provided [World]. /// Get a raw pointer to the [`ComponentTicks`] of a particular [`Component`] on a particular [`Entity`] in the provided [World].
/// ///
/// # Safety /// # Safety
/// Caller must ensure that `component_id` is valid /// - `entity_location` must be within bounds of the given archetype and `entity` must exist inside
/// the archetype
/// - `component_id` must be valid
/// - `storage_type` must accurately reflect where the components for `component_id` are stored.
#[inline] #[inline]
unsafe fn get_component_and_ticks( unsafe fn get_component_and_ticks(
world: &World, world: &World,
component_id: ComponentId, component_id: ComponentId,
storage_type: StorageType,
entity: Entity, entity: Entity,
location: EntityLocation, location: EntityLocation,
) -> Option<(Ptr<'_>, TickCells<'_>)> { ) -> Option<(Ptr<'_>, TickCells<'_>)> {
let archetype = &world.archetypes[location.archetype_id]; match storage_type {
let component_info = world.components.get_info_unchecked(component_id);
match component_info.storage_type() {
StorageType::Table => { StorageType::Table => {
let table = &world.storages.tables[archetype.table_id()]; let (components, table_row) = fetch_table(world, location, component_id)?;
let components = table.get_column(component_id)?;
let table_row = archetype.entity_table_row(location.index);
// SAFETY: archetypes only store valid table_rows and the stored component type is T // SAFETY: archetypes only store valid table_rows and the stored component type is T
Some(( Some((
components.get_data_unchecked(table_row), components.get_data_unchecked(table_row),
@ -655,36 +768,29 @@ unsafe fn get_component_and_ticks(
}, },
)) ))
} }
StorageType::SparseSet => world StorageType::SparseSet => fetch_sparse_set(world, component_id)?.get_with_ticks(entity),
.storages
.sparse_sets
.get(component_id)
.and_then(|sparse_set| sparse_set.get_with_ticks(entity)),
} }
} }
#[inline] /// # Safety
/// - `entity_location` must be within bounds of the given archetype and `entity` must exist inside
/// the archetype
/// - `component_id` must be valid
/// - `storage_type` must accurately reflect where the components for `component_id` are stored.
unsafe fn get_ticks( unsafe fn get_ticks(
world: &World, world: &World,
component_id: ComponentId, component_id: ComponentId,
storage_type: StorageType,
entity: Entity, entity: Entity,
location: EntityLocation, location: EntityLocation,
) -> Option<ComponentTicks> { ) -> Option<ComponentTicks> {
let archetype = &world.archetypes[location.archetype_id]; match storage_type {
let component_info = world.components.get_info_unchecked(component_id);
match component_info.storage_type() {
StorageType::Table => { StorageType::Table => {
let table = &world.storages.tables[archetype.table_id()]; let (components, table_row) = fetch_table(world, location, component_id)?;
let components = table.get_column(component_id)?;
let table_row = archetype.entity_table_row(location.index);
// SAFETY: archetypes only store valid table_rows and the stored component type is T // SAFETY: archetypes only store valid table_rows and the stored component type is T
Some(components.get_ticks_unchecked(table_row)) Some(components.get_ticks_unchecked(table_row))
} }
StorageType::SparseSet => world StorageType::SparseSet => fetch_sparse_set(world, component_id)?.get_ticks(entity),
.storages
.sparse_sets
.get(component_id)
.and_then(|sparse_set| sparse_set.get_ticks(entity)),
} }
} }
@ -733,41 +839,71 @@ unsafe fn take_component<'a>(
/// Get a raw pointer to a particular [`Component`] by [`TypeId`] on a particular [`Entity`] in the provided [`World`]. /// Get a raw pointer to a particular [`Component`] by [`TypeId`] on a particular [`Entity`] in the provided [`World`].
/// ///
/// # Safety /// # Safety
/// `entity_location` must be within bounds of an archetype that exists. /// - `entity_location` must be within bounds of the given archetype and `entity` must exist inside
/// the archetype
/// - `type_id` must be correspond to a type that implements [`Component`]
/// - `storage_type` must accurately reflect where the components for `component_id` are stored.
#[inline]
unsafe fn get_component_with_type( unsafe fn get_component_with_type(
world: &World, world: &World,
type_id: TypeId, type_id: TypeId,
storage_type: StorageType,
entity: Entity, entity: Entity,
location: EntityLocation, location: EntityLocation,
) -> Option<Ptr<'_>> { ) -> Option<Ptr<'_>> {
let component_id = world.components.get_id(type_id)?; get_component(
get_component(world, component_id, entity, location) world,
world.components.get_id(type_id)?,
storage_type,
entity,
location,
)
} }
/// Get a raw pointer to the [`ComponentTicks`] of a particular [`Component`] by [`TypeId`] on a particular [`Entity`] in the provided [`World`]. /// Get a raw pointer to the [`ComponentTicks`] of a particular [`Component`] by [`TypeId`] on a particular [`Entity`] in the provided [`World`].
/// ///
/// # Safety /// # Safety
/// `entity_location` must be within bounds of an archetype that exists. /// - `entity_location` must be within bounds of the given archetype and `entity` must exist inside
pub(crate) unsafe fn get_component_and_ticks_with_type( /// the archetype
/// - `type_id` must be correspond to a type that implements [`Component`]
/// - `storage_type` must accurately reflect where the components for `component_id` are stored.
#[inline]
unsafe fn get_component_and_ticks_with_type(
world: &World, world: &World,
type_id: TypeId, type_id: TypeId,
storage_type: StorageType,
entity: Entity, entity: Entity,
location: EntityLocation, location: EntityLocation,
) -> Option<(Ptr<'_>, TickCells<'_>)> { ) -> Option<(Ptr<'_>, TickCells<'_>)> {
let component_id = world.components.get_id(type_id)?; get_component_and_ticks(
get_component_and_ticks(world, component_id, entity, location) world,
world.components.get_id(type_id)?,
storage_type,
entity,
location,
)
} }
/// # Safety /// # Safety
/// `entity_location` must be within bounds of an archetype that exists. /// - `entity_location` must be within bounds of the given archetype and `entity` must exist inside
pub(crate) unsafe fn get_ticks_with_type( /// the archetype
/// - `type_id` must be correspond to a type that implements [`Component`]
/// - `storage_type` must accurately reflect where the components for `component_id` are stored.
#[inline]
unsafe fn get_ticks_with_type(
world: &World, world: &World,
type_id: TypeId, type_id: TypeId,
storage_type: StorageType,
entity: Entity, entity: Entity,
location: EntityLocation, location: EntityLocation,
) -> Option<ComponentTicks> { ) -> Option<ComponentTicks> {
let component_id = world.components.get_id(type_id)?; get_ticks(
get_ticks(world, component_id, entity, location) world,
world.components.get_id(type_id)?,
storage_type,
entity,
location,
)
} }
fn contains_component_with_type(world: &World, type_id: TypeId, location: EntityLocation) -> bool { fn contains_component_with_type(world: &World, type_id: TypeId, location: EntityLocation) -> bool {
@ -914,12 +1050,17 @@ pub(crate) unsafe fn get_mut<T: Component>(
// T // T
let change_tick = world.change_tick(); let change_tick = world.change_tick();
let last_change_tick = world.last_change_tick(); let last_change_tick = world.last_change_tick();
get_component_and_ticks_with_type(world, TypeId::of::<T>(), entity, location).map( get_component_and_ticks_with_type(
|(value, ticks)| Mut { world,
TypeId::of::<T>(),
T::Storage::STORAGE_TYPE,
entity,
location,
)
.map(|(value, ticks)| Mut {
value: value.assert_unique().deref_mut::<T>(), value: value.assert_unique().deref_mut::<T>(),
ticks: Ticks::from_tick_cells(ticks, last_change_tick, change_tick), ticks: Ticks::from_tick_cells(ticks, last_change_tick, change_tick),
}, })
)
} }
// SAFETY: EntityLocation must be valid, component_id must be valid // SAFETY: EntityLocation must be valid, component_id must be valid
@ -931,13 +1072,14 @@ pub(crate) unsafe fn get_mut_by_id(
component_id: ComponentId, component_id: ComponentId,
) -> Option<MutUntyped> { ) -> Option<MutUntyped> {
let change_tick = world.change_tick(); let change_tick = world.change_tick();
let info = world.components.get_info_unchecked(component_id);
// SAFETY: world access is unique, entity location and component_id required to be valid // SAFETY: world access is unique, entity location and component_id required to be valid
get_component_and_ticks(world, component_id, entity, location).map(|(value, ticks)| { get_component_and_ticks(world, component_id, info.storage_type(), entity, location).map(
MutUntyped { |(value, ticks)| MutUntyped {
value: value.assert_unique(), value: value.assert_unique(),
ticks: Ticks::from_tick_cells(ticks, world.last_change_tick(), change_tick), ticks: Ticks::from_tick_cells(ticks, world.last_change_tick(), change_tick),
} },
}) )
} }
#[cfg(test)] #[cfg(test)]

8
crates/bevy_ecs/src/world/mod.rs
View File

@ -1507,12 +1507,16 @@ impl World {
/// use this in cases where the actual types are not known at compile time.** /// use this in cases where the actual types are not known at compile time.**
#[inline] #[inline]
pub fn get_by_id(&self, entity: Entity, component_id: ComponentId) -> Option<Ptr<'_>> { pub fn get_by_id(&self, entity: Entity, component_id: ComponentId) -> Option<Ptr<'_>> {
self.components().get_info(component_id)?; let info = self.components().get_info(component_id)?;
// SAFETY: entity_location is valid, component_id is valid as checked by the line above // SAFETY:
// - entity_location is valid
// - component_id is valid as checked by the line above
// - the storage type is accurate as checked by the fetched ComponentInfo
unsafe { unsafe {
get_component( get_component(
self, self,
component_id, component_id,
info.storage_type(),
entity, entity,
self.get_entity(entity)?.location(), self.get_entity(entity)?.location(),
) )