From 82d0e84a5c563c81aa7253ff6340479d11ddc0c6 Mon Sep 17 00:00:00 2001 From: Alexander Sepity Date: Thu, 18 Feb 2021 22:30:13 +0300 Subject: [PATCH] Explicit execution order ambiguities API (#1469) Explicit execution order ambiguities API. --- crates/bevy_ecs/src/schedule/stage.rs | 170 +++++++++++++++++- .../bevy_ecs/src/schedule/system_container.rs | 13 ++ .../src/schedule/system_descriptor.rs | 34 ++++ crates/bevy_transform/src/lib.rs | 3 +- 4 files changed, 217 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index f22a30c0dafd8..cd596ca51c545 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -482,9 +482,20 @@ fn topological_order( /// Returns vector containing all pairs of indices of systems with ambiguous execution order. /// Systems must be topologically sorted beforehand. fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize)> { + let mut ambiguity_set_labels = HashMap::default(); + for set in systems.iter().flat_map(|c| c.ambiguity_sets()) { + let len = ambiguity_set_labels.len(); + ambiguity_set_labels.entry(set).or_insert(len); + } + let mut all_ambiguity_sets = Vec::::with_capacity(systems.len()); let mut all_dependencies = Vec::::with_capacity(systems.len()); let mut all_dependants = Vec::::with_capacity(systems.len()); for (index, container) in systems.iter().enumerate() { + let mut ambiguity_sets = FixedBitSet::with_capacity(ambiguity_set_labels.len()); + for set in container.ambiguity_sets() { + ambiguity_sets.insert(ambiguity_set_labels[set]); + } + all_ambiguity_sets.push(ambiguity_sets); let mut dependencies = FixedBitSet::with_capacity(systems.len()); for &dependency in container.dependencies() { dependencies.union_with(&all_dependencies[dependency]); @@ -522,7 +533,10 @@ fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize)> { for index_b in full_bitset.difference(&relations) /*.take(index_a)*/ { - if !processed.contains(index_b) && !systems[index_a].is_compatible(&systems[index_b]) { + if !processed.contains(index_b) + && all_ambiguity_sets[index_a].is_disjoint(&all_ambiguity_sets[index_b]) + && !systems[index_a].is_compatible(&systems[index_b]) + { ambiguities.push((index_a, index_b)); } } @@ -1208,6 +1222,27 @@ mod tests { ); assert_eq!(ambiguities.len(), 2); + let mut stage = SystemStage::parallel() + .with_system(component.system().label("0")) + .with_system( + resource + .system() + .label("1") + .after("0") + .in_ambiguity_set("a"), + ) + .with_system(empty.system().label("2")) + .with_system(component.system().label("3").after("2").before("4")) + .with_system(resource.system().label("4").in_ambiguity_set("a")); + stage.initialize_systems(&mut world, &mut resources); + stage.rebuild_orders_and_dependencies(); + let ambiguities = find_ambiguities_labels(&stage.parallel); + assert!( + ambiguities.contains(&("0".into(), "3".into())) + || ambiguities.contains(&("3".into(), "0".into())) + ); + assert_eq!(ambiguities.len(), 1); + let mut stage = SystemStage::parallel() .with_system(component.system().label("0").before("2")) .with_system(component.system().label("1").before("2")) @@ -1248,6 +1283,30 @@ mod tests { ); assert_eq!(ambiguities.len(), 1); + let mut stage = SystemStage::parallel() + .with_system(component.system().label("0").before("1").before("2")) + .with_system(component.system().label("1").in_ambiguity_set("a")) + .with_system(component.system().label("2").in_ambiguity_set("a")) + .with_system(component.system().label("3").after("1").after("2")); + stage.initialize_systems(&mut world, &mut resources); + stage.rebuild_orders_and_dependencies(); + let ambiguities = find_ambiguities_labels(&stage.parallel); + assert_eq!(ambiguities.len(), 0); + + let mut stage = SystemStage::parallel() + .with_system(component.system().label("0").before("1").before("2")) + .with_system(component.system().label("1").in_ambiguity_set("a")) + .with_system(component.system().label("2").in_ambiguity_set("b")) + .with_system(component.system().label("3").after("1").after("2")); + stage.initialize_systems(&mut world, &mut resources); + stage.rebuild_orders_and_dependencies(); + let ambiguities = find_ambiguities_labels(&stage.parallel); + assert!( + ambiguities.contains(&("1".into(), "2".into())) + || ambiguities.contains(&("2".into(), "1".into())) + ); + assert_eq!(ambiguities.len(), 1); + let mut stage = SystemStage::parallel() .with_system( component @@ -1300,6 +1359,76 @@ mod tests { ); assert_eq!(ambiguities.len(), 6); + let mut stage = SystemStage::parallel() + .with_system( + component + .system() + .label("0") + .before("1") + .before("2") + .before("3") + .before("4"), + ) + .with_system(component.system().label("1").in_ambiguity_set("a")) + .with_system(component.system().label("2").in_ambiguity_set("a")) + .with_system(component.system().label("3").in_ambiguity_set("a")) + .with_system(component.system().label("4").in_ambiguity_set("a")) + .with_system( + component + .system() + .label("5") + .after("1") + .after("2") + .after("3") + .after("4"), + ); + stage.initialize_systems(&mut world, &mut resources); + stage.rebuild_orders_and_dependencies(); + let ambiguities = find_ambiguities_labels(&stage.parallel); + assert_eq!(ambiguities.len(), 0); + + let mut stage = SystemStage::parallel() + .with_system( + component + .system() + .label("0") + .before("1") + .before("2") + .before("3") + .before("4"), + ) + .with_system(component.system().label("1").in_ambiguity_set("a")) + .with_system(component.system().label("2").in_ambiguity_set("a")) + .with_system( + component + .system() + .label("3") + .in_ambiguity_set("a") + .in_ambiguity_set("b"), + ) + .with_system(component.system().label("4").in_ambiguity_set("b")) + .with_system( + component + .system() + .label("5") + .after("1") + .after("2") + .after("3") + .after("4"), + ); + stage.initialize_systems(&mut world, &mut resources); + stage.rebuild_orders_and_dependencies(); + let ambiguities = find_ambiguities_labels(&stage.parallel); + assert!( + ambiguities.contains(&("1".into(), "4".into())) + || ambiguities.contains(&("4".into(), "1".into())) + ); + assert!( + ambiguities.contains(&("2".into(), "4".into())) + || ambiguities.contains(&("4".into(), "2".into())) + ); + assert_eq!(ambiguities.len(), 2); + let mut stage = SystemStage::parallel() .with_system(empty.exclusive_system().label("0")) .with_system(empty.exclusive_system().label("1").after("0")) @@ -1349,5 +1478,44 @@ mod tests { || ambiguities.contains(&("5".into(), "2".into())) ); assert_eq!(ambiguities.len(), 6); + + let mut stage = SystemStage::parallel() + .with_system(empty.exclusive_system().label("0").before("1").before("3")) + .with_system(empty.exclusive_system().label("1").in_ambiguity_set("a")) + .with_system(empty.exclusive_system().label("2").after("1")) + .with_system(empty.exclusive_system().label("3").in_ambiguity_set("a")) + .with_system(empty.exclusive_system().label("4").after("3").before("5")) + .with_system(empty.exclusive_system().label("5").in_ambiguity_set("a")) + .with_system(empty.exclusive_system().label("6").after("2").after("5")); + stage.initialize_systems(&mut world, &mut resources); + stage.rebuild_orders_and_dependencies(); + let ambiguities = find_ambiguities_labels(&stage.exclusive_at_start); + assert!( + ambiguities.contains(&("2".into(), "3".into())) + || ambiguities.contains(&("3".into(), "2".into())) + ); + assert!( + ambiguities.contains(&("1".into(), "4".into())) + || ambiguities.contains(&("4".into(), "1".into())) + ); + assert!( + ambiguities.contains(&("2".into(), "4".into())) + || ambiguities.contains(&("4".into(), "2".into())) + ); + assert!( + ambiguities.contains(&("2".into(), "5".into())) + || ambiguities.contains(&("5".into(), "2".into())) + ); + assert_eq!(ambiguities.len(), 4); + + let mut stage = SystemStage::parallel() + .with_system(empty.exclusive_system().label("0").in_ambiguity_set("a")) + .with_system(empty.exclusive_system().label("1").in_ambiguity_set("a")) + .with_system(empty.exclusive_system().label("2").in_ambiguity_set("a")) + .with_system(empty.exclusive_system().label("3").in_ambiguity_set("a")); + stage.initialize_systems(&mut world, &mut resources); + stage.rebuild_orders_and_dependencies(); + let ambiguities = find_ambiguities_labels(&stage.exclusive_at_start); + assert_eq!(ambiguities.len(), 0); } } diff --git a/crates/bevy_ecs/src/schedule/system_container.rs b/crates/bevy_ecs/src/schedule/system_container.rs index 32618c287f7df..a6aa1d3b3cb04 100644 --- a/crates/bevy_ecs/src/schedule/system_container.rs +++ b/crates/bevy_ecs/src/schedule/system_container.rs @@ -10,6 +10,7 @@ pub(super) trait SystemContainer { fn label(&self) -> &Option>; fn before(&self) -> &[Cow<'static, str>]; fn after(&self) -> &[Cow<'static, str>]; + fn ambiguity_sets(&self) -> &[Cow<'static, str>]; fn is_compatible(&self, other: &Self) -> bool; } @@ -20,6 +21,7 @@ pub(super) struct ExclusiveSystemContainer { label: Option>, before: Vec>, after: Vec>, + ambiguity_sets: Vec>, } impl ExclusiveSystemContainer { @@ -31,6 +33,7 @@ impl ExclusiveSystemContainer { label: descriptor.label, before: descriptor.before, after: descriptor.after, + ambiguity_sets: descriptor.ambiguity_sets, } } @@ -72,6 +75,10 @@ impl SystemContainer for ExclusiveSystemContainer { &self.after } + fn ambiguity_sets(&self) -> &[Cow<'static, str>] { + &self.ambiguity_sets + } + fn is_compatible(&self, _: &Self) -> bool { false } @@ -85,6 +92,7 @@ pub struct ParallelSystemContainer { label: Option>, before: Vec>, after: Vec>, + ambiguity_sets: Vec>, } impl SystemContainer for ParallelSystemContainer { @@ -120,6 +128,10 @@ impl SystemContainer for ParallelSystemContainer { &self.after } + fn ambiguity_sets(&self) -> &[Cow<'static, str>] { + &self.ambiguity_sets + } + fn is_compatible(&self, other: &Self) -> bool { self.system() .component_access() @@ -144,6 +156,7 @@ impl ParallelSystemContainer { label: descriptor.label, before: descriptor.before, after: descriptor.after, + ambiguity_sets: descriptor.ambiguity_sets, } } diff --git a/crates/bevy_ecs/src/schedule/system_descriptor.rs b/crates/bevy_ecs/src/schedule/system_descriptor.rs index dd368f9f453fe..39a40a352050e 100644 --- a/crates/bevy_ecs/src/schedule/system_descriptor.rs +++ b/crates/bevy_ecs/src/schedule/system_descriptor.rs @@ -75,6 +75,7 @@ pub struct ParallelSystemDescriptor { pub(crate) label: Option>, pub(crate) before: Vec>, pub(crate) after: Vec>, + pub(crate) ambiguity_sets: Vec>, } fn new_parallel_descriptor(system: BoxedSystem<(), ()>) -> ParallelSystemDescriptor { @@ -83,6 +84,7 @@ fn new_parallel_descriptor(system: BoxedSystem<(), ()>) -> ParallelSystemDescrip label: None, before: Vec::new(), after: Vec::new(), + ambiguity_sets: Vec::new(), } } @@ -95,6 +97,10 @@ pub trait ParallelSystemDescriptorCoercion { /// Specifies that the system should run after the system with given label. fn after(self, label: impl Into>) -> ParallelSystemDescriptor; + + /// Specifies that the system is exempt from execution order ambiguity detection + /// with other systems in this set. + fn in_ambiguity_set(self, set: impl Into>) -> ParallelSystemDescriptor; } impl ParallelSystemDescriptorCoercion for ParallelSystemDescriptor { @@ -112,6 +118,11 @@ impl ParallelSystemDescriptorCoercion for ParallelSystemDescriptor { self.after.push(label.into()); self } + + fn in_ambiguity_set(mut self, set: impl Into>) -> ParallelSystemDescriptor { + self.ambiguity_sets.push(set.into()); + self + } } impl ParallelSystemDescriptorCoercion for S @@ -129,6 +140,10 @@ where fn after(self, label: impl Into>) -> ParallelSystemDescriptor { new_parallel_descriptor(Box::new(self)).after(label) } + + fn in_ambiguity_set(self, set: impl Into>) -> ParallelSystemDescriptor { + new_parallel_descriptor(Box::new(self)).in_ambiguity_set(set) + } } impl ParallelSystemDescriptorCoercion for BoxedSystem<(), ()> { @@ -143,6 +158,10 @@ impl ParallelSystemDescriptorCoercion for BoxedSystem<(), ()> { fn after(self, label: impl Into>) -> ParallelSystemDescriptor { new_parallel_descriptor(self).after(label) } + + fn in_ambiguity_set(self, set: impl Into>) -> ParallelSystemDescriptor { + new_parallel_descriptor(self).in_ambiguity_set(set) + } } #[derive(Debug, Clone, Copy)] @@ -158,6 +177,7 @@ pub struct ExclusiveSystemDescriptor { pub(crate) label: Option>, pub(crate) before: Vec>, pub(crate) after: Vec>, + pub(crate) ambiguity_sets: Vec>, pub(crate) insertion_point: InsertionPoint, } @@ -167,6 +187,7 @@ fn new_exclusive_descriptor(system: Box) -> ExclusiveSystem label: None, before: Vec::new(), after: Vec::new(), + ambiguity_sets: Vec::new(), insertion_point: InsertionPoint::AtStart, } } @@ -181,6 +202,10 @@ pub trait ExclusiveSystemDescriptorCoercion { /// Specifies that the system should run after the system with given label. fn after(self, label: impl Into>) -> ExclusiveSystemDescriptor; + /// Specifies that the system is exempt from execution order ambiguity detection + /// with other systems in this set. + fn in_ambiguity_set(self, set: impl Into>) -> ExclusiveSystemDescriptor; + /// Specifies that the system should run with other exclusive systems at the start of stage. fn at_start(self) -> ExclusiveSystemDescriptor; @@ -208,6 +233,11 @@ impl ExclusiveSystemDescriptorCoercion for ExclusiveSystemDescriptor { self } + fn in_ambiguity_set(mut self, set: impl Into>) -> ExclusiveSystemDescriptor { + self.ambiguity_sets.push(set.into()); + self + } + fn at_start(mut self) -> ExclusiveSystemDescriptor { self.insertion_point = InsertionPoint::AtStart; self @@ -240,6 +270,10 @@ where new_exclusive_descriptor(Box::new(self)).after(label) } + fn in_ambiguity_set(self, set: impl Into>) -> ExclusiveSystemDescriptor { + new_exclusive_descriptor(Box::new(self)).in_ambiguity_set(set) + } + fn at_start(self) -> ExclusiveSystemDescriptor { new_exclusive_descriptor(Box::new(self)).at_start() } diff --git a/crates/bevy_transform/src/lib.rs b/crates/bevy_transform/src/lib.rs index 919406e792928..ce318a5492887 100644 --- a/crates/bevy_transform/src/lib.rs +++ b/crates/bevy_transform/src/lib.rs @@ -7,8 +7,7 @@ pub mod prelude { } use bevy_app::{prelude::*, startup_stage}; -use bevy_ecs::IntoSystem; -use bevy_ecs::ParallelSystemDescriptorCoercion; +use bevy_ecs::{IntoSystem, ParallelSystemDescriptorCoercion}; use bevy_reflect::RegisterTypeBuilder; use prelude::{parent_update_system, Children, GlobalTransform, Parent, PreviousParent, Transform};