Skip to content

Commit 0dce905

Browse files
authored
Faster FilteredEntity(Ref|Mut) and Entity(Ref|Mut)Except by borrowing Access (#20111)
# Objective Improve the performance of queries using `FilteredEntityRef`, `FilteredEntityMut`, `EntityRefExcept`, and `EntityMutExcept`. In particular, this appears to speed up `bevy_animation::animate_targets` by 10% in many-foxes. `FilteredEntity(Ref|Mut)` needs to store an `Access` to determine which components may be accessed. Prior to #15396, this required cloning the `Access` for each instance. Now, we can borrow the `Access` from the query state and make cheap pointer copies. `Entity(Ref|Mut)Except` avoided needing to clone an `Access` by calling functions on the `Bundle` trait. Unfortunately, that meant we needed to convert from a type to a `ComponentId` for every component in the bundle on every check. Now, we can do those conversions up front and pass references to an `Access`. Finally, fix a bug where `Entity(Ref|Mut)Except` would not initialize their components during `init_state`. I noticed this while updating `init_state` and fixed it while I was there. That was normally harmless because the components would be registered elsewhere, but a system like `fn system(_q1: Query<EntityMutExcept<C>>, _q2: Query<&mut C>) {}` would fail to find the `ComponentId` for `C` and not exclude it from the access for `q1`, and then panic with conflicting access from `q2`. ## Solution Change `FilteredEntityRef` and `FilteredEntityMut` to store `&'s Access` instead of `Access`, and change `EntityRefExcept` and `EntityMutExcept` to store an extra `&'s Access`. This adds the `'s` lifetime to those four types, and most changes are adding lifetimes as appropriate. Change the `WorldQuery::State` for `Entity(Ref|Mut)Except` to store an `Access` that can be borrowed from, replacing the `SmallVec<[ComponentId; 4]>` that was used only to set the query access. To support the conversions from `EntityRef` and `EntityMut`, we need to be able to create a `&'static Access` for read-all or write-all. I could not change `fn read_all_components()` to be `const` because it called the non-`const` `FixedBitSet::clear()`, so I created separate constructor functions. ## Testing Ran `cargo run --example many_foxes --features bevy/trace_tracy --release` before and after, and compared the results of `animate_targets`, since that is the only in-engine use of `EntityMutExcept` and was the motivation for creating it. Yellow is this PR, red is main: <img width="695" height="690" alt="image" src="https://github.com/user-attachments/assets/24531a3f-65bf-46d0-baa5-29ea9e56b16a" />
1 parent cb70675 commit 0dce905

File tree

8 files changed

+277
-267
lines changed

8 files changed

+277
-267
lines changed

crates/bevy_animation/src/animation_curves.rs

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -408,10 +408,7 @@ impl<A: Animatable> AnimationCurveEvaluator for AnimatableCurveEvaluator<A> {
408408
self.evaluator.push_blend_register(weight, graph_node)
409409
}
410410

411-
fn commit<'a>(
412-
&mut self,
413-
mut entity: AnimationEntityMut<'a>,
414-
) -> Result<(), AnimationEvaluationError> {
411+
fn commit(&mut self, mut entity: AnimationEntityMut) -> Result<(), AnimationEvaluationError> {
415412
let property = self.property.get_mut(&mut entity)?;
416413
*property = self
417414
.evaluator
@@ -596,10 +593,7 @@ impl AnimationCurveEvaluator for WeightsCurveEvaluator {
596593
Ok(())
597594
}
598595

599-
fn commit<'a>(
600-
&mut self,
601-
mut entity: AnimationEntityMut<'a>,
602-
) -> Result<(), AnimationEvaluationError> {
596+
fn commit(&mut self, mut entity: AnimationEntityMut) -> Result<(), AnimationEvaluationError> {
603597
if self.stack_morph_target_weights.is_empty() {
604598
return Ok(());
605599
}
@@ -905,10 +899,7 @@ pub trait AnimationCurveEvaluator: Downcast + Send + Sync + 'static {
905899
///
906900
/// The property on the component must be overwritten with the value from
907901
/// the stack, not blended with it.
908-
fn commit<'a>(
909-
&mut self,
910-
entity: AnimationEntityMut<'a>,
911-
) -> Result<(), AnimationEvaluationError>;
902+
fn commit(&mut self, entity: AnimationEntityMut) -> Result<(), AnimationEvaluationError>;
912903
}
913904

914905
impl_downcast!(AnimationCurveEvaluator);

crates/bevy_animation/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,8 +1021,8 @@ pub fn advance_animations(
10211021
}
10221022

10231023
/// A type alias for [`EntityMutExcept`] as used in animation.
1024-
pub type AnimationEntityMut<'w> =
1025-
EntityMutExcept<'w, (AnimationTarget, AnimationPlayer, AnimationGraphHandle)>;
1024+
pub type AnimationEntityMut<'w, 's> =
1025+
EntityMutExcept<'w, 's, (AnimationTarget, AnimationPlayer, AnimationGraphHandle)>;
10261026

10271027
/// A system that modifies animation targets (e.g. bones in a skinned mesh)
10281028
/// according to the currently-playing animations.

crates/bevy_ecs/src/query/access.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,32 @@ impl<T: SparseSetIndex> Access<T> {
165165
}
166166
}
167167

168+
/// Creates an [`Access`] with read access to all components.
169+
/// This is equivalent to calling `read_all()` on `Access::new()`,
170+
/// but is available in a `const` context.
171+
pub(crate) const fn new_read_all() -> Self {
172+
let mut access = Self::new();
173+
access.reads_all_resources = true;
174+
// Note that we cannot use `read_all_components()`
175+
// because `FixedBitSet::clear()` is not `const`.
176+
access.component_read_and_writes_inverted = true;
177+
access
178+
}
179+
180+
/// Creates an [`Access`] with read access to all components.
181+
/// This is equivalent to calling `read_all()` on `Access::new()`,
182+
/// but is available in a `const` context.
183+
pub(crate) const fn new_write_all() -> Self {
184+
let mut access = Self::new();
185+
access.reads_all_resources = true;
186+
access.writes_all_resources = true;
187+
// Note that we cannot use `write_all_components()`
188+
// because `FixedBitSet::clear()` is not `const`.
189+
access.component_read_and_writes_inverted = true;
190+
access.component_writes_inverted = true;
191+
access
192+
}
193+
168194
fn add_component_sparse_set_index_read(&mut self, index: usize) {
169195
if !self.component_read_and_writes_inverted {
170196
self.component_read_and_writes.grow_and_insert(index);

0 commit comments

Comments
 (0)