Skip to content

Commit

Permalink
Remove archetype_component_access from QueryState (#12474)
Browse files Browse the repository at this point in the history 
# 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<ArchetypeComponentId>`
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](james7132/bevy_asm_tests@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<ArchetypeComponentId>` argument, which will be updated with the
new accesses.
Changed: `QueryState::update_archetype_component_access` now takes a
`&mut Access<ArchetypeComponentId>` argument, which will be updated with
the new accesses.

## Migration Guide
TODO
  • Loading branch information
@james7132
james7132 authored Mar 17, 2024
1 parent 8327ce8 commit eebf3d6
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 29 deletions.
74 changes: 53 additions & 21 deletions crates/bevy_ecs/src/query/state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ pub struct QueryState<D: QueryData, F: QueryFilter = ()> {
pub(crate) archetype_generation: ArchetypeGeneration,
pub(crate) matched_tables: FixedBitSet,
pub(crate) matched_archetypes: FixedBitSet,
pub(crate) archetype_component_access: Access<ArchetypeComponentId>,
pub(crate) component_access: FilteredAccess<ComponentId>,
// NOTE: we maintain both a TableId bitset and a vec because iterating the vec is faster
pub(crate) matched_table_ids: Vec<TableId>,
Expand Down Expand Up @@ -96,11 +95,6 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
&*ptr::from_ref(self).cast::<QueryState<NewD, NewF>>()
}

/// Returns the archetype components accessed by this query.
pub fn archetype_component_access(&self) -> &Access<ArchetypeComponentId> {
&self.archetype_component_access
}

/// Returns the components accessed by this query.
pub fn component_access(&self) -> &FilteredAccess<ComponentId> {
&self.component_access
Expand All @@ -120,6 +114,31 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
/// 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<ArchetypeComponentId>,
) -> 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);

Expand All @@ -136,7 +155,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
// 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(),
Expand All @@ -146,16 +165,13 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
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::<D>(),
filter = std::any::type_name::<F>(),
),
};
state.update_archetypes(world);
state
}
}

/// Creates a new [`QueryState`] from a given [`QueryBuilder`] and inherits it's [`FilteredAccess`].
Expand All @@ -174,7 +190,6 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
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",
Expand Down Expand Up @@ -276,7 +291,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
std::mem::replace(&mut self.archetype_generation, archetypes.generation());

for archetype in &archetypes[old_generation..] {
self.new_archetype(archetype);
self.new_archetype_internal(archetype);
}
}

Expand All @@ -303,13 +318,23 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {

/// 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<ArchetypeComponentId>,
) {
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);
Expand All @@ -320,6 +345,9 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
self.matched_tables.grow_and_insert(table_index);
self.matched_table_ids.push(archetype.table_id());
}
true
} else {
false
}
}

Expand All @@ -337,15 +365,21 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
}

/// 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<ArchetypeComponentId>,
) {
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);
}
});
}
Expand Down Expand Up @@ -397,7 +431,6 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
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",
Expand Down Expand Up @@ -505,7 +538,6 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
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",
Expand Down
10 changes: 2 additions & 8 deletions crates/bevy_ecs/src/system/system_param.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ unsafe impl<D: QueryData + 'static, F: QueryFilter + 'static> 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::<D>(),
Expand All @@ -205,17 +205,11 @@ unsafe impl<D: QueryData + 'static, F: QueryFilter + 'static> 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]
Expand Down

0 comments on commit eebf3d6

Please sign in to comment.