From 830c98b133e8fe3afbb8eabda70d844239c9de71 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 14 Mar 2024 00:21:29 -0700 Subject: [PATCH 01/10] Remove archetye_component_access from QueryState --- crates/bevy_ecs/src/query/state.rs | 22 ++++++---------------- crates/bevy_ecs/src/system/system_param.rs | 11 +++-------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 1a97e2bd70b4e..a5815ac2882ed 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 @@ -146,7 +140,6 @@ 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", @@ -174,7 +167,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", @@ -268,7 +260,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(archetype, &mut Access::new()); } } @@ -295,12 +287,12 @@ 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) { + pub fn new_archetype(&mut self, archetype: &Archetype, access: &mut Access) { 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); + self.update_archetype_component_access(archetype, access); let archetype_index = archetype.id().index(); if !self.matched_archetypes.contains(archetype_index) { @@ -331,15 +323,15 @@ 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) { + 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); } }); } @@ -391,7 +383,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", @@ -499,7 +490,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 f9ecc9aaf62d6..87a6c7f5b88ce 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 mut state = QueryState::new(world); assert_component_access_compatibility( &system_meta.name, std::any::type_name::(), @@ -205,17 +205,12 @@ 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.update_archetypes(world); 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] From 54189fc5038b510ca3743b8aa438ab94c9f463d2 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 14 Mar 2024 00:51:41 -0700 Subject: [PATCH 02/10] Formatting --- crates/bevy_ecs/src/query/state.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index a5815ac2882ed..2785db808a027 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -287,7 +287,13 @@ 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, access: &mut Access) { + /// + /// 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 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)) @@ -323,7 +329,13 @@ 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, access: &mut Access) { + /// + /// 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) { access.add_read(id); From 2b69664600d1698abc851a6bc0c2b206320bc991 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 14 Mar 2024 00:57:24 -0700 Subject: [PATCH 03/10] Remove unnecessary update_archetypes call. --- crates/bevy_ecs/src/system/system_param.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 87a6c7f5b88ce..751b3334d6d2a 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -205,7 +205,6 @@ unsafe impl SystemParam for Qu system_meta .component_access_set .add(state.component_access.clone()); - state.update_archetypes(world); state } From ffb77c85500b5224d7b82f1943e40867d2d4ba93 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 14 Mar 2024 01:10:20 -0700 Subject: [PATCH 04/10] Shut up clippy --- crates/bevy_ecs/src/system/system_param.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 751b3334d6d2a..43cf075d62b7d 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 mut state = QueryState::new(world); + let state = QueryState::new(world); assert_component_access_compatibility( &system_meta.name, std::any::type_name::(), From 6a04362e82557d3ddadeedad60bfac4153b11191 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 14 Mar 2024 01:26:43 -0700 Subject: [PATCH 05/10] Avoid allocation on update_archetypes --- crates/bevy_ecs/src/query/state.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 2785db808a027..12956ebccb3dd 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -260,7 +260,7 @@ impl QueryState { std::mem::replace(&mut self.archetype_generation, archetypes.generation()); for archetype in &archetypes[old_generation..] { - self.new_archetype(archetype, &mut Access::new()); + self.new_archetype_internal(archetype); } } @@ -294,12 +294,19 @@ impl QueryState { 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, access); - let archetype_index = archetype.id().index(); if !self.matched_archetypes.contains(archetype_index) { self.matched_archetypes.grow(archetype_index + 1); @@ -312,6 +319,9 @@ impl QueryState { self.matched_tables.set(table_index, true); self.matched_table_ids.push(archetype.table_id()); } + true + } else { + false } } From cf9f7191392c3d922354a9c09f578bbfb7d07fb2 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 14 Mar 2024 01:27:11 -0700 Subject: [PATCH 06/10] Formatting --- crates/bevy_ecs/src/query/state.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 12956ebccb3dd..d04cd07d72444 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -299,10 +299,7 @@ impl QueryState { } } - fn new_archetype_internal( - &mut self, - archetype: &Archetype, - ) -> bool { + 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)) From 9256aca0ed87e4d2e1b3389bd7a3d777527ac6dd Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 14 Mar 2024 02:37:25 -0700 Subject: [PATCH 07/10] Fix behavior on late-constructed QueryStates --- crates/bevy_ecs/src/query/state.rs | 23 ++++++++++++++++++++-- crates/bevy_ecs/src/system/system_param.rs | 2 +- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index d04cd07d72444..422e5f11b948f 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -114,6 +114,25 @@ 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_internal(world); + state.update_archetypes(world); + state + } + + pub(crate) fn new_with_access( + world: &mut World, + access: &mut Access, + ) -> Self { + let mut state = Self::new_internal(world); + for archetype in world.archetypes.iter() { + if state.new_archetype_internal(archetype) { + state.update_archetype_component_access(archetype, access); + } + } + state + } + + fn new_internal(world: &mut World) -> Self { let fetch_state = D::init_state(world); let filter_state = F::init_state(world); @@ -130,7 +149,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 { + let state = Self { world_id: world.id(), archetype_generation: ArchetypeGeneration::initial(), matched_table_ids: Vec::new(), @@ -147,7 +166,7 @@ impl QueryState { filter = std::any::type_name::(), ), }; - state.update_archetypes(world); + state } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 43cf075d62b7d..7b9d7cfd1349a 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::(), From 77f30fdbf9d5b202179b485da8041cabdc392f34 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 14 Mar 2024 02:38:59 -0700 Subject: [PATCH 08/10] Update archetype generation properly --- crates/bevy_ecs/src/query/state.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 422e5f11b948f..049783c121425 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -129,6 +129,7 @@ impl QueryState { state.update_archetype_component_access(archetype, access); } } + state.archetype_generation = world.archetypes.generation(); state } From ae7fdf821d2d712f90a9840a5cc5c916a51b5d2b Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 14 Mar 2024 02:41:09 -0700 Subject: [PATCH 09/10] Small cleanup --- crates/bevy_ecs/src/query/state.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 049783c121425..f15a95e94bdef 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -150,7 +150,7 @@ impl QueryState { // properly considered in a global "cross-query" context (both within systems and across systems). component_access.extend(&filter_component_access); - let state = Self { + Self { world_id: world.id(), archetype_generation: ArchetypeGeneration::initial(), matched_table_ids: Vec::new(), @@ -166,9 +166,7 @@ impl QueryState { query = std::any::type_name::(), filter = std::any::type_name::(), ), - }; - - state + } } /// Creates a new [`QueryState`] from a given [`QueryBuilder`] and inherits it's [`FilteredAccess`]. From 0fceba3f87ad3118feac6c29e928ae466d750c8e Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 15 Mar 2024 14:24:52 -0700 Subject: [PATCH 10/10] new_uninitialized and comments --- crates/bevy_ecs/src/query/state.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index f15a95e94bdef..3abee168e0555 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -114,16 +114,17 @@ 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_internal(world); + 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_internal(world); + 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); @@ -133,7 +134,11 @@ impl QueryState { state } - fn new_internal(world: &mut World) -> Self { + /// 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);