From bca34395eacbb384daa144ac80f16b0aed8fd826 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Sun, 4 Feb 2024 21:26:07 -0800 Subject: [PATCH] Intern entity paths for faster comparisons. This patch places all entity paths into a shared table so that comparing them is as cheap as a pointer comparison. We don't use the pre-existing `Interner` type because that leaks all strings added to it, and I was uncomfortable with leaking names, as Bevy apps might dynamically generate them. Instead, I reference count all the names and use a linked list to stitch together paths into a tree. The interner uses a weak hash set from the [`weak-table`] crate. This patch is especially helpful for the two-phase animation PR #11707, because two-phase animation gets rid of the cache from name to animation-specific slot, thus increasing the load on the hash table that maps paths to bone indices. Note that the interned table is a global variable behind a `OnceLock` instead of a resource. This is because it must be accessed from the glTF `AssetLoader`, which unfortunately has no access to Bevy resources. [`weak-table`]: https://crates.io/crates/weak-table --- crates/bevy_animation/Cargo.toml | 4 + crates/bevy_animation/src/entity_path.rs | 153 +++++++++++++++++++++++ crates/bevy_animation/src/lib.rs | 26 ++-- crates/bevy_gltf/src/loader.rs | 4 +- examples/animation/animated_transform.rs | 16 +-- 5 files changed, 174 insertions(+), 29 deletions(-) create mode 100644 crates/bevy_animation/src/entity_path.rs diff --git a/crates/bevy_animation/Cargo.toml b/crates/bevy_animation/Cargo.toml index f2d5da4cd7dbf..c692083119a77 100644 --- a/crates/bevy_animation/Cargo.toml +++ b/crates/bevy_animation/Cargo.toml @@ -13,6 +13,7 @@ keywords = ["bevy"] bevy_app = { path = "../bevy_app", version = "0.12.0" } bevy_asset = { path = "../bevy_asset", version = "0.12.0" } bevy_core = { path = "../bevy_core", version = "0.12.0" } +bevy_derive = { path = "../bevy_derive", version = "0.12.0" } bevy_math = { path = "../bevy_math", version = "0.12.0" } bevy_reflect = { path = "../bevy_reflect", version = "0.12.0", features = [ "bevy", @@ -24,5 +25,8 @@ bevy_ecs = { path = "../bevy_ecs", version = "0.12.0" } bevy_transform = { path = "../bevy_transform", version = "0.12.0" } bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.12.0" } +# others +weak-table = "0.3" + [lints] workspace = true diff --git a/crates/bevy_animation/src/entity_path.rs b/crates/bevy_animation/src/entity_path.rs new file mode 100644 index 0000000000000..21021e3194009 --- /dev/null +++ b/crates/bevy_animation/src/entity_path.rs @@ -0,0 +1,153 @@ +//! Entity paths for referring to bones. + +use std::{ + fmt::{self, Debug, Formatter, Write}, + hash::{Hash, Hasher}, + sync::{Arc, Mutex, OnceLock, Weak}, +}; + +use bevy_core::Name; +use bevy_reflect::Reflect; +use bevy_utils::prelude::default; +use weak_table::WeakHashSet; + +static ENTITY_PATH_STORE: OnceLock> = OnceLock::new(); + +/// Path to an entity, with [`Name`]s. Each entity in a path must have a name. +#[derive(Clone, Reflect)] +#[reflect_value] +pub struct EntityPath(Arc); + +#[derive(PartialEq, Eq, Hash)] +struct EntityPathNode { + name: Name, + parent: Option, +} + +// This could use a `RwLock`, but we actually never read from this, so a mutex +// is actually slightly more efficient! +#[derive(Default)] +struct EntityPathStore(WeakHashSet>); + +pub struct EntityPathIter<'a>(Option<&'a EntityPath>); + +impl EntityPathStore { + fn create_path(&mut self, node: EntityPathNode) -> EntityPath { + match self.0.get(&node) { + Some(node) => EntityPath(node), + None => { + let node = Arc::new(node); + self.0.insert(node.clone()); + EntityPath(node) + } + } + } +} + +impl EntityPath { + pub fn from_name(name: Name) -> EntityPath { + ENTITY_PATH_STORE + .get_or_init(|| default()) + .lock() + .unwrap() + .create_path(EntityPathNode { name, parent: None }) + } + + pub fn from_names(names: &[Name]) -> EntityPath { + let mut store = ENTITY_PATH_STORE.get_or_init(|| default()).lock().unwrap(); + + let mut names = names.iter(); + let root_name = names + .next() + .expect("Entity path must have at least one name in it"); + + let mut path = store.create_path(EntityPathNode { + name: root_name.clone(), + parent: None, + }); + for name in names { + path = store.create_path(EntityPathNode { + name: name.clone(), + parent: Some(path), + }); + } + + path + } + + pub fn extend(&self, name: Name) -> EntityPath { + ENTITY_PATH_STORE + .get_or_init(|| default()) + .lock() + .unwrap() + .create_path(EntityPathNode { + name, + parent: Some(self.clone()), + }) + } + + pub fn iter(&self) -> EntityPathIter { + EntityPathIter(Some(self)) + } + + pub fn root(&self) -> &Name { + &self.iter().last().unwrap().0.name + } + + pub fn len(&self) -> usize { + self.iter().count() + } + + pub fn name(&self) -> &Name { + &self.0.name + } +} + +impl PartialEq for EntityPath { + fn eq(&self, other: &Self) -> bool { + Arc::ptr_eq(&self.0, &other.0) + } +} + +impl Eq for EntityPath {} + +impl Hash for EntityPath { + fn hash(&self, state: &mut H) { + // Hash by address. This is safe because entity paths are unique. + (self.0.as_ref() as *const EntityPathNode).hash(state) + } +} + +impl Debug for EntityPath { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let mut names = vec![]; + let mut current_path = Some(self.clone()); + while let Some(path) = current_path { + names.push(path.0.name.clone()); + current_path = path.0.parent.clone(); + } + + for (name_index, name) in names.iter().rev().enumerate() { + if name_index > 0 { + f.write_char('/')?; + } + f.write_str(name)?; + } + + Ok(()) + } +} + +impl<'a> Iterator for EntityPathIter<'a> { + type Item = &'a EntityPath; + + fn next(&mut self) -> Option { + match self.0 { + None => None, + Some(node) => { + self.0 = node.0.parent.as_ref(); + Some(node) + } + } + } +} diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs index 5771e2e9da510..c60d109093ee6 100644 --- a/crates/bevy_animation/src/lib.rs +++ b/crates/bevy_animation/src/lib.rs @@ -1,6 +1,7 @@ //! Animation for the game engine Bevy mod animatable; +mod entity_path; mod util; use std::ops::{Add, Deref, Mul}; @@ -16,17 +17,20 @@ use bevy_reflect::Reflect; use bevy_render::mesh::morph::MorphWeights; use bevy_time::Time; use bevy_transform::{prelude::Transform, TransformSystem}; +use bevy_utils::smallvec::SmallVec; use bevy_utils::{tracing::warn, HashMap}; #[allow(missing_docs)] pub mod prelude { #[doc(hidden)] pub use crate::{ - animatable::*, AnimationClip, AnimationPlayer, AnimationPlugin, EntityPath, Interpolation, - Keyframes, VariableCurve, + animatable::*, entity_path::*, AnimationClip, AnimationPlayer, AnimationPlugin, + Interpolation, Keyframes, VariableCurve, }; } +pub use crate::entity_path::EntityPath; + /// List of keyframes for one of the attribute of a [`Transform`]. #[derive(Reflect, Clone, Debug)] pub enum Keyframes { @@ -138,13 +142,6 @@ pub enum Interpolation { CubicSpline, } -/// Path to an entity, with [`Name`]s. Each entity in a path must have a name. -#[derive(Reflect, Clone, Debug, Hash, PartialEq, Eq, Default)] -pub struct EntityPath { - /// Parts of the path - pub parts: Vec, -} - /// A list of [`VariableCurve`], and the [`EntityPath`] to which they apply. #[derive(Asset, Reflect, Clone, Debug, Default)] pub struct AnimationClip { @@ -199,7 +196,7 @@ impl AnimationClip { /// Whether this animation clip can run on entity with given [`Name`]. pub fn compatible_with(&self, name: &Name) -> bool { - self.paths.keys().any(|path| &path.parts[0] == name) + self.paths.keys().any(|path| &path.root() == &name) } } @@ -488,9 +485,10 @@ fn entity_from_path( ) -> Option { // PERF: finding the target entity can be optimised let mut current_entity = root; - path_cache.resize(path.parts.len(), None); + path_cache.resize(path.len(), None); - let mut parts = path.parts.iter().enumerate(); + let parts_vec: SmallVec<[&Name; 8]> = path.iter().map(|path| path.name()).collect(); + let mut parts = parts_vec.iter().rev().enumerate(); // check the first name is the root node which we already have let Some((_, root_name)) = parts.next() else { @@ -506,7 +504,7 @@ fn entity_from_path( if let Some(cached) = path_cache[idx] { if children.contains(&cached) { if let Ok(name) = names.get(cached) { - if name == part { + if name == *part { current_entity = cached; found = true; } @@ -516,7 +514,7 @@ fn entity_from_path( if !found { for child in children.deref() { if let Ok(name) = names.get(*child) { - if name == part { + if name == *part { // Found a children with the right name, continue to the next part current_entity = *child; path_cache[idx] = Some(*child); diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index c3615ff5da259..04bce99ff5c0f 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -260,9 +260,7 @@ async fn load_gltf<'a, 'b, 'c>( if let Some((root_index, path)) = paths.get(&node.index()) { animation_roots.insert(root_index); animation_clip.add_curve_to_path( - bevy_animation::EntityPath { - parts: path.clone(), - }, + bevy_animation::EntityPath::from_names(&path), bevy_animation::VariableCurve { keyframe_timestamps, keyframes, diff --git a/examples/animation/animated_transform.rs b/examples/animation/animated_transform.rs index b9ee301a02ae6..1606ed69f4c0a 100644 --- a/examples/animation/animated_transform.rs +++ b/examples/animation/animated_transform.rs @@ -36,9 +36,7 @@ fn setup( let mut animation = AnimationClip::default(); // A curve can modify a single part of a transform, here the translation animation.add_curve_to_path( - EntityPath { - parts: vec![planet.clone()], - }, + EntityPath::from_name(planet.clone()), VariableCurve { keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0], keyframes: Keyframes::Translation(vec![ @@ -57,9 +55,7 @@ fn setup( // To find the entity to modify, the hierarchy will be traversed looking for // an entity with the right name at each level animation.add_curve_to_path( - EntityPath { - parts: vec![planet.clone(), orbit_controller.clone()], - }, + EntityPath::from_names(&[planet.clone(), orbit_controller.clone()]), VariableCurve { keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0], keyframes: Keyframes::Rotation(vec![ @@ -76,9 +72,7 @@ fn setup( // until all other curves are finished. In that case, another animation should // be created for each part that would have a different duration / period animation.add_curve_to_path( - EntityPath { - parts: vec![planet.clone(), orbit_controller.clone(), satellite.clone()], - }, + EntityPath::from_names(&[planet.clone(), orbit_controller.clone(), satellite.clone()]), VariableCurve { keyframe_timestamps: vec![0.0, 0.5, 1.0, 1.5, 2.0, 2.5, 3.0, 3.5, 4.0], keyframes: Keyframes::Scale(vec![ @@ -97,9 +91,7 @@ fn setup( ); // There can be more than one curve targeting the same entity path animation.add_curve_to_path( - EntityPath { - parts: vec![planet.clone(), orbit_controller.clone(), satellite.clone()], - }, + EntityPath::from_names(&[planet.clone(), orbit_controller.clone(), satellite.clone()]), VariableCurve { keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0], keyframes: Keyframes::Rotation(vec![