From 966c6444c0205c3fb0dde6d02a444004dae5de8c Mon Sep 17 00:00:00 2001 From: Mat Hostetter <4849531+mjhostet@users.noreply.github.com> Date: Thu, 15 Oct 2020 13:39:01 -0700 Subject: [PATCH] Adjust how `ArchetypeAccess` tracks mutable & immutable deps (#660) `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. --- crates/bevy_ecs/src/system/query.rs | 24 ++++----------------- crates/bevy_ecs/src/system/system.rs | 31 ++++++++++++++-------------- 2 files changed, 20 insertions(+), 35 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 0bf409699fc57f..18402ded211a38 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -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 { @@ -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 { @@ -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]); } @@ -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]); } } diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index 983190470f52cd..7eecf6c416992a 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -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(&mut self, world: &World) @@ -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::().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(); } } @@ -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]