Skip to content

Commit

Permalink
Adjust how ArchetypeAccess tracks mutable & immutable deps (#660)
Browse files Browse the repository at this point in the history
`ArchetypeAccess` was tracking `immutable` and `mutable` separately.
This means that checking is_compatible requires three checks:
m+m, m+i, i+m.

Instead, continue tracking `mutable` accesses, but instead of
`immutable` track `immutable | mutable` as another `accessed` bit mask.
This drops the comparisons to two (m+a, a+m) and turns out to be
what the rest of the code base wants too, unifying various duplicated
checks and loops.
  • Loading branch information
mjhostet authored Oct 15, 2020
1 parent f66a725 commit 871790c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 35 deletions.
24 changes: 4 additions & 20 deletions crates/bevy_ecs/src/system/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,8 @@ impl<'a, Q: HecsQuery> Query<'a, Q> {
if let Some(location) = self.world.get_entity_location(entity) {
if self
.archetype_access
.immutable
.accessed
.contains(location.archetype as usize)
|| self
.archetype_access
.mutable
.contains(location.archetype as usize)
{
// SAFE: we have already checked that the entity/component matches our archetype access. and systems are scheduled to run with safe archetype access
unsafe {
Expand All @@ -71,12 +67,8 @@ impl<'a, Q: HecsQuery> Query<'a, Q> {
if let Some(location) = self.world.get_entity_location(entity) {
if self
.archetype_access
.immutable
.accessed
.contains(location.archetype as usize)
|| self
.archetype_access
.mutable
.contains(location.archetype as usize)
{
// SAFE: we have already checked that the entity matches our archetype. and systems are scheduled to run with safe archetype access
Ok(unsafe {
Expand Down Expand Up @@ -205,11 +197,7 @@ impl<'w, Q: HecsQuery> QueryBorrowChecked<'w, Q> {
);
}

for index in self.archetype_access.immutable.ones() {
Q::Fetch::borrow(&self.archetypes[index]);
}

for index in self.archetype_access.mutable.ones() {
for index in self.archetype_access.accessed.ones() {
Q::Fetch::borrow(&self.archetypes[index]);
}

Expand All @@ -224,11 +212,7 @@ impl<'w, Q: HecsQuery> Drop for QueryBorrowChecked<'w, Q> {
#[inline]
fn drop(&mut self) {
if self.borrowed {
for index in self.archetype_access.immutable.ones() {
Q::Fetch::release(&self.archetypes[index]);
}

for index in self.archetype_access.mutable.ones() {
for index in self.archetype_access.accessed.ones() {
Q::Fetch::release(&self.archetypes[index]);
}
}
Expand Down
31 changes: 16 additions & 15 deletions crates/bevy_ecs/src/system/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,19 @@ pub trait System: Send + Sync {
/// Provides information about the archetypes a [System] reads and writes
#[derive(Debug, Default)]
pub struct ArchetypeAccess {
pub immutable: FixedBitSet,
pub accessed: FixedBitSet, // union of both immutable and mutable
pub mutable: FixedBitSet,
}

// credit to Ratysz from the Yaks codebase
impl ArchetypeAccess {
pub fn is_compatible(&self, other: &ArchetypeAccess) -> bool {
self.mutable.is_disjoint(&other.mutable)
&& self.mutable.is_disjoint(&other.immutable)
&& self.immutable.is_disjoint(&other.mutable)
self.mutable.is_disjoint(&other.accessed) && self.accessed.is_disjoint(&other.mutable)
}

pub fn union(&mut self, other: &ArchetypeAccess) {
self.mutable.union_with(&other.mutable);
self.immutable.union_with(&other.immutable);
self.accessed.union_with(&other.accessed);
}

pub fn set_access_for_query<Q>(&mut self, world: &World)
Expand All @@ -60,20 +58,23 @@ impl ArchetypeAccess {
{
let iterator = world.archetypes();
let bits = iterator.len();
self.immutable.grow(bits);
self.accessed.grow(bits);
self.mutable.grow(bits);
iterator
.enumerate()
.filter_map(|(index, archetype)| archetype.access::<Q>().map(|access| (index, access)))
.for_each(|(archetype, access)| match access {
Access::Read => self.immutable.set(archetype, true),
Access::Write => self.mutable.set(archetype, true),
Access::Read => self.accessed.set(archetype, true),
Access::Write => {
self.accessed.set(archetype, true);
self.mutable.set(archetype, true);
}
Access::Iterate => (),
});
}

pub fn clear(&mut self) {
self.immutable.clear();
self.accessed.clear();
self.mutable.clear();
}
}
Expand Down Expand Up @@ -128,16 +129,16 @@ mod tests {
let e2_archetype = world.get_entity_location(e2).unwrap().archetype as usize;
let e3_archetype = world.get_entity_location(e3).unwrap().archetype as usize;

assert!(access.immutable.contains(e1_archetype));
assert!(access.immutable.contains(e2_archetype));
assert!(access.immutable.contains(e3_archetype));
assert!(access.accessed.contains(e1_archetype));
assert!(access.accessed.contains(e2_archetype));
assert!(access.accessed.contains(e3_archetype));

let mut access = ArchetypeAccess::default();
access.set_access_for_query::<(&A, &B)>(&world);

assert!(access.immutable.contains(e1_archetype) == false);
assert!(access.immutable.contains(e2_archetype));
assert!(access.immutable.contains(e3_archetype));
assert!(access.accessed.contains(e1_archetype) == false);
assert!(access.accessed.contains(e2_archetype));
assert!(access.accessed.contains(e3_archetype));
}

#[test]
Expand Down

0 comments on commit 871790c

Please sign in to comment.