Skip to content

Commit

Permalink
Inline trivial methods in bevy_hierarchy (#11332)
Browse files Browse the repository at this point in the history
# Objective

In #11330 I found out that `Parent::get` didn't get inlined, **even with
LTO on**!

This means that just to access a field, we have an instruction cache
invalidation, we will move some registers to the stack, will jump to new
instructions, move the field into a register, then do the same dance in
the other direction to go back to the call site.

## Solution

Mark trivial functions as `#[inline]`.

`inline(always)` may increase compilation time proportional to how many
time the function is called **and the size of the function marked with
`inline`**. Since we mark as `inline` functions that consists in a
single instruction, the cost is absolutely negligible.

I also took the opportunity to `inline` other functions. I'm not as
confident that marking functions calling other functions as `inline`
works similarly to very simple functions, so I used `inline` over
`inline(always)`, which doesn't have the same downsides as
`inline(always)`.

More information on inlining in rust:
https://nnethercote.github.io/perf-book/inlining.html
  • Loading branch information
nicopap authored Jan 13, 2024
1 parent 78b5f32 commit a634075
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 0 deletions.
10 changes: 10 additions & 0 deletions crates/bevy_hierarchy/src/components/children.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,21 @@ impl MapEntities for Children {
// However Children should only ever be set with a real user-defined entities. Its worth looking
// into better ways to handle cases like this.
impl FromWorld for Children {
#[inline]
fn from_world(_world: &mut World) -> Self {
Children(SmallVec::new())
}
}

impl Children {
/// Constructs a [`Children`] component with the given entities.
#[inline]
pub(crate) fn from_entities(entities: &[Entity]) -> Self {
Self(SmallVec::from_slice(entities))
}

/// Swaps the child at `a_index` with the child at `b_index`.
#[inline]
pub fn swap(&mut self, a_index: usize, b_index: usize) {
self.0.swap(a_index, b_index);
}
Expand All @@ -65,6 +68,7 @@ impl Children {
/// For the unstable version, see [`sort_unstable_by`](Children::sort_unstable_by).
///
/// See also [`sort_by_key`](Children::sort_by_key), [`sort_by_cached_key`](Children::sort_by_cached_key).
#[inline]
pub fn sort_by<F>(&mut self, compare: F)
where
F: FnMut(&Entity, &Entity) -> std::cmp::Ordering,
Expand All @@ -80,6 +84,7 @@ impl Children {
/// For the unstable version, see [`sort_unstable_by_key`](Children::sort_unstable_by_key).
///
/// See also [`sort_by`](Children::sort_by), [`sort_by_cached_key`](Children::sort_by_cached_key).
#[inline]
pub fn sort_by_key<K, F>(&mut self, compare: F)
where
F: FnMut(&Entity) -> K,
Expand All @@ -95,6 +100,7 @@ impl Children {
/// For the underlying implementation, see [`slice::sort_by_cached_key`].
///
/// See also [`sort_by`](Children::sort_by), [`sort_by_key`](Children::sort_by_key).
#[inline]
pub fn sort_by_cached_key<K, F>(&mut self, compare: F)
where
F: FnMut(&Entity) -> K,
Expand All @@ -111,6 +117,7 @@ impl Children {
/// For the stable version, see [`sort_by`](Children::sort_by).
///
/// See also [`sort_unstable_by_key`](Children::sort_unstable_by_key).
#[inline]
pub fn sort_unstable_by<F>(&mut self, compare: F)
where
F: FnMut(&Entity, &Entity) -> std::cmp::Ordering,
Expand All @@ -126,6 +133,7 @@ impl Children {
/// For the stable version, see [`sort_by_key`](Children::sort_by_key).
///
/// See also [`sort_unstable_by`](Children::sort_unstable_by).
#[inline]
pub fn sort_unstable_by_key<K, F>(&mut self, compare: F)
where
F: FnMut(&Entity) -> K,
Expand All @@ -138,6 +146,7 @@ impl Children {
impl Deref for Children {
type Target = [Entity];

#[inline(always)]
fn deref(&self) -> &Self::Target {
&self.0[..]
}
Expand All @@ -148,6 +157,7 @@ impl<'a> IntoIterator for &'a Children {

type IntoIter = slice::Iter<'a, Entity>;

#[inline(always)]
fn into_iter(self) -> Self::IntoIter {
self.0.iter()
}
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_hierarchy/src/components/parent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub struct Parent(pub(crate) Entity);

impl Parent {
/// Gets the [`Entity`] ID of the parent.
#[inline(always)]
pub fn get(&self) -> Entity {
self.0
}
Expand All @@ -37,6 +38,7 @@ impl Parent {
/// for both [`Children`] & [`Parent`] that is agnostic to edge direction.
///
/// [`Children`]: super::children::Children
#[inline(always)]
pub fn as_slice(&self) -> &[Entity] {
std::slice::from_ref(&self.0)
}
Expand All @@ -47,6 +49,7 @@ impl Parent {
// However Parent should only ever be set with a real user-defined entity. Its worth looking into
// better ways to handle cases like this.
impl FromWorld for Parent {
#[inline(always)]
fn from_world(_world: &mut World) -> Self {
Parent(Entity::PLACEHOLDER)
}
Expand All @@ -61,6 +64,7 @@ impl MapEntities for Parent {
impl Deref for Parent {
type Target = Entity;

#[inline(always)]
fn deref(&self) -> &Self::Target {
&self.0
}
Expand Down

0 comments on commit a634075

Please sign in to comment.