Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove extra allocation in AccessFilters #14026

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions crates/bevy_ecs/src/query/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ impl<T: SparseSetIndex> Default for FilteredAccess<T> {
Self {
access: Access::default(),
required: FixedBitSet::default(),
filter_sets: vec![AccessFilters::default()],
filter_sets: vec![],
}
}
}
Expand Down Expand Up @@ -388,6 +388,9 @@ impl<T: SparseSetIndex> FilteredAccess<T> {
/// Suppose we begin with `Or<(With<A>, With<B>)>`, which is represented by an array of two `AccessFilter` instances.
/// Adding `AND With<C>` via this method transforms it into the equivalent of `Or<((With<A>, With<C>), (With<B>, With<C>))>`.
pub fn and_with(&mut self, index: T) {
if self.filter_sets.is_empty() {
self.filter_sets.push(AccessFilters::default());
}
for filter in &mut self.filter_sets {
filter.with.grow_and_insert(index.sparse_set_index());
}
Expand All @@ -398,6 +401,9 @@ impl<T: SparseSetIndex> FilteredAccess<T> {
/// Suppose we begin with `Or<(With<A>, With<B>)>`, which is represented by an array of two `AccessFilter` instances.
/// Adding `AND Without<C>` via this method transforms it into the equivalent of `Or<((With<A>, Without<C>), (With<B>, Without<C>))>`.
pub fn and_without(&mut self, index: T) {
if self.filter_sets.is_empty() {
self.filter_sets.push(AccessFilters::default());
}
for filter in &mut self.filter_sets {
filter.without.grow_and_insert(index.sparse_set_index());
}
Expand All @@ -409,7 +415,13 @@ impl<T: SparseSetIndex> FilteredAccess<T> {
/// where each element (`AccessFilters`) represents a conjunction,
/// we can simply append to the array.
pub fn append_or(&mut self, other: &FilteredAccess<T>) {
self.filter_sets.append(&mut other.filter_sets.clone());
let mut other_filter_sets = if other.filter_sets.is_empty() {
// we want to explicitly append_or with an empty filter set, for example in Or<((), With<A>)>
vec![AccessFilters::default()]
} else {
other.filter_sets.clone()
};
self.filter_sets.append(&mut other_filter_sets);
}

/// Adds all of the accesses from `other` to `self`.
Expand All @@ -430,12 +442,13 @@ impl<T: SparseSetIndex> FilteredAccess<T> {
//
// For example, `Query<&mut C, Or<(With<A>, Without<B>)>>` is compatible `Query<&mut C, (With<B>, Without<A>)>`,
// but `Query<&mut C, Or<(Without<A>, Without<B>)>>` isn't compatible with `Query<&mut C, Or<(With<A>, With<B>)>>`.
self.filter_sets.iter().all(|filter| {
other
.filter_sets
.iter()
.all(|other_filter| filter.is_ruled_out_by(other_filter))
})
!self.filter_sets.is_empty()
&& self.filter_sets.iter().all(|filter| {
other
.filter_sets
.iter()
.all(|other_filter| filter.is_ruled_out_by(other_filter))
})
}

/// Returns a vector of elements that this and `other` cannot access at the same time.
Expand All @@ -457,6 +470,14 @@ impl<T: SparseSetIndex> FilteredAccess<T> {
self.access.extend(&other.access);
self.required.union_with(&other.required);

if other.filter_sets.is_empty() {
return;
}
if self.filter_sets.is_empty() {
self.filter_sets.clone_from(&other.filter_sets);
return;
}

// We can avoid allocating a new array of bitsets if `other` contains just a single set of filters:
// in this case we can short-circuit by performing an in-place union for each bitset.
if other.filter_sets.len() == 1 {
Expand Down
24 changes: 7 additions & 17 deletions crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1862,28 +1862,18 @@ macro_rules! impl_anytuple_fetch {
}

fn update_component_access(state: &Self::State, _access: &mut FilteredAccess<ComponentId>) {
let mut _new_access = _access.clone();

// update the access (add the read/writes)
<($(Option<$name>,)*)>::update_component_access(state, _access);

// update the filters (Or<(With<$name>,)>)
let ($($name,)*) = state;
let mut _not_first = false;
let mut _new_access = FilteredAccess::default();
$(
if _not_first {
// we use an intermediate access because we only want to update the filters, not the access
let mut intermediate = FilteredAccess::default();
$name::update_component_access($name, &mut intermediate);
_new_access.append_or(&intermediate);
} else {
$name::update_component_access($name, &mut _new_access);
_new_access.required = _access.required.clone();
_not_first = true;
}
// we use an intermediate empty access because we only want to update the filters, not the access
let mut intermediate = _access.clone();
$name::update_component_access($name, &mut intermediate);
_new_access.append_or(&intermediate);
)*

_access.filter_sets = _new_access.filter_sets;

<($(Option<$name>,)*)>::update_component_access(state, _access);
}
#[allow(unused_variables)]
fn init_state(world: &mut World) -> Self::State {
Expand Down
16 changes: 4 additions & 12 deletions crates/bevy_ecs/src/query/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,19 +443,11 @@ macro_rules! impl_or_query_filter {
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
let ($($filter,)*) = state;

let mut _new_access = access.clone();
let mut _not_first = false;
let mut _new_access = FilteredAccess::default();
$(
if _not_first {
let mut intermediate = access.clone();
$filter::update_component_access($filter, &mut intermediate);
_new_access.append_or(&intermediate);
_new_access.extend_access(&intermediate);
} else {
$filter::update_component_access($filter, &mut _new_access);
_new_access.required = access.required.clone();
_not_first = true;
}
let mut intermediate = access.clone();
$filter::update_component_access($filter, &mut intermediate);
_new_access.append_or(&intermediate);
)*

*access = _new_access;
Expand Down
17 changes: 9 additions & 8 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,15 +434,16 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {

/// Returns `true` if this query matches a set of components. Otherwise, returns `false`.
pub fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
self.component_access.filter_sets.iter().any(|set| {
set.with
.ones()
.all(|index| set_contains_id(ComponentId::get_sparse_set_index(index)))
&& set
.without
self.component_access.filter_sets.is_empty()
|| self.component_access.filter_sets.iter().any(|set| {
set.with
.ones()
.all(|index| !set_contains_id(ComponentId::get_sparse_set_index(index)))
})
.all(|index| set_contains_id(ComponentId::get_sparse_set_index(index)))
&& set
.without
.ones()
.all(|index| !set_contains_id(ComponentId::get_sparse_set_index(index)))
})
}

/// For the given `archetype`, adds any component accessed used by this query's underlying [`FilteredAccess`] to `access`.
Expand Down