Inline more ECS functions (#8083)

# Objective
Upon closer inspection, there are a few functions in the ECS that are
not being inlined, even with the highest optimizations and LTO enabled:

- Almost all
[WorldQuery::init_fetch](9fd5f20e25/results/query_get.s (L57))
calls. Affects `Query::get` calls in hot loops. In particular, the
`WorldQuery` implementation for `()` is used *everywhere* as the default
filter and is effectively a no-op.
-
[Entities::get](9fd5f20e25/results/query_get.s (L39)).
Affects `Query::get`, `World::get`, and any component insertion or
removal.
-
[Entities::set](9fd5f20e25/results/entity_remove.s (L2487)).
Affects any component insertion or removal.
-
[Tick::new](9fd5f20e25/results/entity_insert.s (L1368)).
I've only seen this in component insertion and spawning.
 - ArchetypeRow::new
 - BlobVec::set_len

Almost all of these have trivial or even empty implementations or have
significant opportunity to be optimized into surrounding code when
inlined with LTO enabled.

## Solution
Inline them
This commit is contained in:
James Liu 2023-04-12 12:52:06 -07:00 committed by GitHub
parent f3c7ccefc6
commit 2ec38d1467
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 40 additions and 1 deletions

View file

@ -47,6 +47,7 @@ impl ArchetypeRow {
pub const INVALID: ArchetypeRow = ArchetypeRow(u32::MAX);
/// Creates a `ArchetypeRow`.
#[inline]
pub const fn new(index: usize) -> Self {
Self(index as u32)
}
@ -433,6 +434,7 @@ impl Archetype {
/// # Safety
/// valid component values must be immediately written to the relevant storages
/// `table_row` must be valid
#[inline]
pub(crate) unsafe fn allocate(
&mut self,
entity: Entity,
@ -449,6 +451,7 @@ impl Archetype {
}
}
#[inline]
pub(crate) fn reserve(&mut self, additional: usize) {
self.entities.reserve(additional);
}
@ -458,6 +461,7 @@ impl Archetype {
///
/// # Panics
/// This function will panic if `index >= self.len()`
#[inline]
pub(crate) fn swap_remove(&mut self, row: ArchetypeRow) -> ArchetypeSwapRemoveResult {
let is_last = row.index() == self.entities.len() - 1;
let entity = self.entities.swap_remove(row.index());

View file

@ -263,6 +263,7 @@ impl SparseSetIndex for BundleId {
self.index()
}
#[inline]
fn get_sparse_set_index(value: usize) -> Self {
Self(value)
}

View file

@ -283,6 +283,7 @@ impl SparseSetIndex for ComponentId {
self.index()
}
#[inline]
fn get_sparse_set_index(value: usize) -> Self {
Self(value)
}
@ -601,6 +602,7 @@ impl Tick {
/// ticks are periodically scanned to ensure their relative values are below this.
pub const MAX: Self = Self::new(MAX_CHANGE_AGE);
#[inline]
pub const fn new(tick: u32) -> Self {
Self { tick }
}
@ -617,10 +619,10 @@ impl Tick {
self.tick = tick;
}
#[inline]
/// Returns `true` if this `Tick` occurred since the system's `last_run`.
///
/// `this_run` is the current tick of the system, used as a reference to help deal with wraparound.
#[inline]
pub fn is_newer_than(self, last_run: Tick, this_run: Tick) -> bool {
// This works even with wraparound because the world tick (`this_run`) is always "newer" than
// `last_run` and `self.tick`, and we scan periodically to clamp `ComponentTicks` values
@ -634,6 +636,7 @@ impl Tick {
}
/// Returns a change tick representing the relationship between `self` and `other`.
#[inline]
pub(crate) fn relative_to(self, other: Self) -> Self {
let tick = self.tick.wrapping_sub(other.tick);
Self { tick }
@ -642,6 +645,7 @@ impl Tick {
/// Wraps this change tick's value if it exceeds [`Tick::MAX`].
///
/// Returns `true` if wrapping was performed. Otherwise, returns `false`.
#[inline]
pub(crate) fn check_tick(&mut self, tick: Tick) -> bool {
let age = tick.relative_to(*self);
// This comparison assumes that `age` has not overflowed `u32::MAX` before, which will be true

View file

@ -234,10 +234,12 @@ impl fmt::Debug for Entity {
}
impl SparseSetIndex for Entity {
#[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)
}
@ -570,6 +572,7 @@ impl Entities {
/// Returns the location of an [`Entity`].
/// Note: for pending entities, returns `Some(EntityLocation::INVALID)`.
#[inline]
pub fn get(&self, entity: Entity) -> Option<EntityLocation> {
if let Some(meta) = self.meta.get(entity.index as usize) {
if meta.generation != entity.generation
@ -590,6 +593,7 @@ impl Entities {
/// - `index` must be a valid entity index.
/// - `location` must be valid for the entity at `index` or immediately made valid afterwards
/// before handing control to unknown code.
#[inline]
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;

View file

@ -550,6 +550,7 @@ unsafe impl<T: Component> WorldQuery for &T {
const IS_ARCHETYPAL: bool = true;
#[inline]
unsafe fn init_fetch<'w>(
world: &'w World,
&component_id: &ComponentId,
@ -695,6 +696,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> {
const IS_ARCHETYPAL: bool = true;
#[inline]
unsafe fn init_fetch<'w>(
world: &'w World,
&component_id: &ComponentId,
@ -856,6 +858,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
const IS_ARCHETYPAL: bool = true;
#[inline]
unsafe fn init_fetch<'w>(
world: &'w World,
&component_id: &ComponentId,
@ -1000,6 +1003,7 @@ unsafe impl<T: WorldQuery> WorldQuery for Option<T> {
const IS_ARCHETYPAL: bool = T::IS_ARCHETYPAL;
#[inline]
unsafe fn init_fetch<'w>(
world: &'w World,
state: &T::State,
@ -1104,6 +1108,7 @@ macro_rules! impl_tuple_fetch {
)*)
}
#[inline]
#[allow(clippy::unused_unit)]
unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> {
let ($($name,)*) = state;
@ -1213,6 +1218,7 @@ macro_rules! impl_anytuple_fetch {
)*)
}
#[inline]
#[allow(clippy::unused_unit)]
unsafe fn init_fetch<'w>(_world: &'w World, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> {
let ($($name,)*) = state;

View file

@ -50,6 +50,7 @@ unsafe impl<T: Component> WorldQuery for With<T> {
fn shrink<'wlong: 'wshort, 'wshort>(_: Self::Item<'wlong>) -> Self::Item<'wshort> {}
#[inline]
unsafe fn init_fetch(_world: &World, _state: &ComponentId, _last_run: Tick, _this_run: Tick) {}
unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {}
@ -146,6 +147,7 @@ unsafe impl<T: Component> WorldQuery for Without<T> {
fn shrink<'wlong: 'wshort, 'wshort>(_: Self::Item<'wlong>) -> Self::Item<'wshort> {}
#[inline]
unsafe fn init_fetch(_world: &World, _state: &ComponentId, _last_run: Tick, _this_run: Tick) {}
unsafe fn clone_fetch<'w>(_fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {}
@ -265,6 +267,7 @@ macro_rules! impl_query_filter_tuple {
const IS_ARCHETYPAL: bool = true $(&& $filter::IS_ARCHETYPAL)*;
#[inline]
unsafe fn init_fetch<'w>(world: &'w World, state: &Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> {
let ($($filter,)*) = state;
($(OrFetch {
@ -420,6 +423,7 @@ macro_rules! impl_tick_filter {
item
}
#[inline]
unsafe fn init_fetch<'w>(world: &'w World, &id: &ComponentId, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> {
Self::Fetch::<'w> {
table_ticks: None,

View file

@ -244,6 +244,7 @@ impl BlobVec {
/// Newly added items must be immediately populated with valid values and length must be
/// increased. For better unwind safety, call [`BlobVec::set_len`] _after_ populating a new
/// value.
#[inline]
pub unsafe fn set_len(&mut self, len: usize) {
debug_assert!(len <= self.capacity());
self.len = len;

View file

@ -546,10 +546,12 @@ pub trait SparseSetIndex: Clone + PartialEq + Eq + Hash {
macro_rules! impl_sparse_set_index {
($($ty:ty),+) => {
$(impl SparseSetIndex for $ty {
#[inline]
fn sparse_set_index(&self) -> usize {
*self as usize
}
#[inline]
fn get_sparse_set_index(value: usize) -> Self {
value as $ty
}
@ -587,6 +589,7 @@ impl SparseSets {
}
/// Gets a reference to the [`ComponentSparseSet`] of a [`ComponentId`].
#[inline]
pub fn get(&self, component_id: ComponentId) -> Option<&ComponentSparseSet> {
self.sets.get(component_id)
}

View file

@ -71,6 +71,7 @@ impl SparseSetIndex for WorldId {
self.0
}
#[inline]
fn get_sparse_set_index(value: usize) -> Self {
Self(value)
}

View file

@ -109,11 +109,13 @@ impl World {
}
/// Creates a new [`UnsafeWorldCell`] view with complete read+write access.
#[inline]
pub fn as_unsafe_world_cell(&mut self) -> UnsafeWorldCell<'_> {
UnsafeWorldCell::new_mutable(self)
}
/// Creates a new [`UnsafeWorldCell`] view with only read access to everything.
#[inline]
pub fn as_unsafe_world_cell_readonly(&self) -> UnsafeWorldCell<'_> {
UnsafeWorldCell::new_readonly(self)
}
@ -121,6 +123,7 @@ impl World {
/// Creates a new [`UnsafeWorldCell`] with read+write access from a [&World](World).
/// This is only a temporary measure until every `&World` that is semantically a [`UnsafeWorldCell`]
/// has been replaced.
#[inline]
pub(crate) fn as_unsafe_world_cell_migration_internal(&self) -> UnsafeWorldCell<'_> {
UnsafeWorldCell::new_readonly(self)
}

View file

@ -81,11 +81,13 @@ unsafe impl Sync for UnsafeWorldCell<'_> {}
impl<'w> UnsafeWorldCell<'w> {
/// Creates a [`UnsafeWorldCell`] that can be used to access everything immutably
#[inline]
pub(crate) fn new_readonly(world: &'w World) -> Self {
UnsafeWorldCell(world as *const World as *mut World, PhantomData)
}
/// Creates [`UnsafeWorldCell`] that can be used to access everything mutably
#[inline]
pub(crate) fn new_mutable(world: &'w mut World) -> Self {
Self(world as *mut World, PhantomData)
}
@ -99,6 +101,7 @@ impl<'w> UnsafeWorldCell<'w> {
/// - there must be no other borrows on world
/// - returned borrow must not be used in any way if the world is accessed
/// through other means than the `&mut World` after this call.
#[inline]
pub unsafe fn world_mut(self) -> &'w mut World {
// SAFETY:
// - caller ensures the created `&mut World` is the only borrow of world
@ -112,6 +115,7 @@ impl<'w> UnsafeWorldCell<'w> {
/// - must have permission to access the whole world immutably
/// - there must be no live exclusive borrows on world data
/// - there must be no live exclusive borrow of world
#[inline]
pub unsafe fn world(self) -> &'w World {
// SAFETY:
// - caller ensures there is no `&mut World` this makes it okay to make a `&World`
@ -128,6 +132,7 @@ impl<'w> UnsafeWorldCell<'w> {
///
/// # Safety
/// - must only be used to access world metadata
#[inline]
pub unsafe fn world_metadata(self) -> &'w World {
// SAFETY: caller ensures that returned reference is not used to violate aliasing rules
unsafe { self.unsafe_world() }
@ -144,6 +149,7 @@ impl<'w> UnsafeWorldCell<'w> {
/// # Safety
/// - must not be used in a way that would conflict with any
/// live exclusive borrows on world data
#[inline]
pub(crate) unsafe fn unsafe_world(self) -> &'w World {
// SAFETY:
// - caller ensures that the returned `&World` is not used in a way that would conflict
@ -240,6 +246,7 @@ impl<'w> UnsafeWorldCell<'w> {
/// Retrieves an [`UnsafeEntityCell`] that exposes read and write operations for the given `entity`.
/// Similar to the [`UnsafeWorldCell`], you are in charge of making sure that no aliasing rules are violated.
#[inline]
pub fn get_entity(self, entity: Entity) -> Option<UnsafeEntityCell<'w>> {
let location = self.entities().get(entity)?;
Some(UnsafeEntityCell::new(self, entity, location))
@ -502,6 +509,7 @@ pub struct UnsafeEntityCell<'w> {
}
impl<'w> UnsafeEntityCell<'w> {
#[inline]
pub(crate) fn new(
world: UnsafeWorldCell<'w>,
entity: Entity,