From eebf3d61ec8b65997063c90e54be89a2f48f3dd2 Mon Sep 17 00:00:00 2001 From: James Liu Date: Sun, 17 Mar 2024 12:01:52 -0700 Subject: [PATCH] Remove archetype_component_access from QueryState (#12474) # Objective `QueryState::archetype_component_access` is only really ever used to extend `SystemMeta`'s. It can be removed to save some memory for every `Query` in an app. ## Solution * Remove it. * Have `new_archetype` pass in a `&mut Access` instead and pull it from `SystemMeta` directly. * Split `QueryState::new` from `QueryState::new_with_access` and a common `QueryState::new_uninitialized`. * Split `new_archetype` into an internal and public version. Call the internal version in `update_archetypes`. This should make it faster to construct new QueryStates, and by proxy lenses and joins as well. `matched_tables` also similarly is only used to deduplicate inserting into `matched_table_ids`. If we can find another efficient way to do so, it might also be worth removing. The [generated assembly](https://github.com/james7132/bevy_asm_tests/compare/main...remove-query-state-archetype-component-access#diff-496530101f0b16e495b7e9b77c0e906ae3068c8adb69ed36c92d5a1be5a9efbe) reflects this well, with all of the access related updates in `QueryState` being removed. --- ## Changelog Removed: `QueryState::archetype_component_access`. Changed: `QueryState::new_archetype` now takes a `&mut Access` argument, which will be updated with the new accesses. Changed: `QueryState::update_archetype_component_access` now takes a `&mut Access` argument, which will be updated with the new accesses. ## Migration Guide TODO --- crates/bevy_ecs/src/query/state.rs | 74 ++++++++++++++++------ crates/bevy_ecs/src/system/system_param.rs | 10 +-- 2 files changed, 55 insertions(+), 29 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index cb69bc78e6..75da60c8d5 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -31,7 +31,6 @@ pub struct QueryState { pub(crate) archetype_generation: ArchetypeGeneration, pub(crate) matched_tables: FixedBitSet, pub(crate) matched_archetypes: FixedBitSet, - pub(crate) archetype_component_access: Access, pub(crate) component_access: FilteredAccess, // NOTE: we maintain both a TableId bitset and a vec because iterating the vec is faster pub(crate) matched_table_ids: Vec, @@ -96,11 +95,6 @@ impl QueryState { &*ptr::from_ref(self).cast::>() } - /// Returns the archetype components accessed by this query. - pub fn archetype_component_access(&self) -> &Access { - &self.archetype_component_access - } - /// Returns the components accessed by this query. pub fn component_access(&self) -> &FilteredAccess { &self.component_access @@ -120,6 +114,31 @@ impl QueryState { impl QueryState { /// Creates a new [`QueryState`] from a given [`World`] and inherits the result of `world.id()`. pub fn new(world: &mut World) -> Self { + let mut state = Self::new_uninitialized(world); + state.update_archetypes(world); + state + } + + /// Identical to `new`, but it populates the provided `access` with the matched results. + pub(crate) fn new_with_access( + world: &mut World, + access: &mut Access, + ) -> Self { + let mut state = Self::new_uninitialized(world); + for archetype in world.archetypes.iter() { + if state.new_archetype_internal(archetype) { + state.update_archetype_component_access(archetype, access); + } + } + state.archetype_generation = world.archetypes.generation(); + state + } + + /// Creates a new [`QueryState`] but does not populate it with the matched results from the World yet + /// + /// `new_archetype` and it's variants must be called on all of the World's archetypes before the + /// state can return valid query results. + fn new_uninitialized(world: &mut World) -> Self { let fetch_state = D::init_state(world); let filter_state = F::init_state(world); @@ -136,7 +155,7 @@ impl QueryState { // properly considered in a global "cross-query" context (both within systems and across systems). component_access.extend(&filter_component_access); - let mut state = Self { + Self { world_id: world.id(), archetype_generation: ArchetypeGeneration::initial(), matched_table_ids: Vec::new(), @@ -146,16 +165,13 @@ impl QueryState { component_access, matched_tables: Default::default(), matched_archetypes: Default::default(), - archetype_component_access: Default::default(), #[cfg(feature = "trace")] par_iter_span: bevy_utils::tracing::info_span!( "par_for_each", query = std::any::type_name::(), filter = std::any::type_name::(), ), - }; - state.update_archetypes(world); - state + } } /// Creates a new [`QueryState`] from a given [`QueryBuilder`] and inherits it's [`FilteredAccess`]. @@ -174,7 +190,6 @@ impl QueryState { component_access: builder.access().clone(), matched_tables: Default::default(), matched_archetypes: Default::default(), - archetype_component_access: Default::default(), #[cfg(feature = "trace")] par_iter_span: bevy_utils::tracing::info_span!( "par_for_each", @@ -276,7 +291,7 @@ impl QueryState { std::mem::replace(&mut self.archetype_generation, archetypes.generation()); for archetype in &archetypes[old_generation..] { - self.new_archetype(archetype); + self.new_archetype_internal(archetype); } } @@ -303,13 +318,23 @@ impl QueryState { /// Update the current [`QueryState`] with information from the provided [`Archetype`] /// (if applicable, i.e. if the archetype has any intersecting [`ComponentId`] with the current [`QueryState`]). - pub fn new_archetype(&mut self, archetype: &Archetype) { + /// + /// The passed in `access` will be updated with any new accesses introduced by the new archetype. + pub fn new_archetype( + &mut self, + archetype: &Archetype, + access: &mut Access, + ) { + if self.new_archetype_internal(archetype) { + self.update_archetype_component_access(archetype, access); + } + } + + fn new_archetype_internal(&mut self, archetype: &Archetype) -> bool { if D::matches_component_set(&self.fetch_state, &|id| archetype.contains(id)) && F::matches_component_set(&self.filter_state, &|id| archetype.contains(id)) && self.matches_component_set(&|id| archetype.contains(id)) { - self.update_archetype_component_access(archetype); - let archetype_index = archetype.id().index(); if !self.matched_archetypes.contains(archetype_index) { self.matched_archetypes.grow_and_insert(archetype_index); @@ -320,6 +345,9 @@ impl QueryState { self.matched_tables.grow_and_insert(table_index); self.matched_table_ids.push(archetype.table_id()); } + true + } else { + false } } @@ -337,15 +365,21 @@ impl QueryState { } /// For the given `archetype`, adds any component accessed used by this query's underlying [`FilteredAccess`] to `access`. - pub fn update_archetype_component_access(&mut self, archetype: &Archetype) { + /// + /// The passed in `access` will be updated with any new accesses introduced by the new archetype. + pub fn update_archetype_component_access( + &mut self, + archetype: &Archetype, + access: &mut Access, + ) { self.component_access.access.reads().for_each(|id| { if let Some(id) = archetype.get_archetype_component_id(id) { - self.archetype_component_access.add_read(id); + access.add_read(id); } }); self.component_access.access.writes().for_each(|id| { if let Some(id) = archetype.get_archetype_component_id(id) { - self.archetype_component_access.add_write(id); + access.add_write(id); } }); } @@ -397,7 +431,6 @@ impl QueryState { component_access: self.component_access.clone(), matched_tables: self.matched_tables.clone(), matched_archetypes: self.matched_archetypes.clone(), - archetype_component_access: self.archetype_component_access.clone(), #[cfg(feature = "trace")] par_iter_span: bevy_utils::tracing::info_span!( "par_for_each", @@ -505,7 +538,6 @@ impl QueryState { component_access: joined_component_access, matched_tables, matched_archetypes, - archetype_component_access: self.archetype_component_access.clone(), #[cfg(feature = "trace")] par_iter_span: bevy_utils::tracing::info_span!( "par_for_each", diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index f9ecc9aaf6..7b9d7cfd13 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -193,7 +193,7 @@ unsafe impl SystemParam for Qu type Item<'w, 's> = Query<'w, 's, D, F>; fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { - let state = QueryState::new(world); + let state = QueryState::new_with_access(world, &mut system_meta.archetype_component_access); assert_component_access_compatibility( &system_meta.name, std::any::type_name::(), @@ -205,17 +205,11 @@ unsafe impl SystemParam for Qu system_meta .component_access_set .add(state.component_access.clone()); - system_meta - .archetype_component_access - .extend(&state.archetype_component_access); state } fn new_archetype(state: &mut Self::State, archetype: &Archetype, system_meta: &mut SystemMeta) { - state.new_archetype(archetype); - system_meta - .archetype_component_access - .extend(&state.archetype_component_access); + state.new_archetype(archetype, &mut system_meta.archetype_component_access); } #[inline]