Skip to content

Commit

Permalink
Explicit execution order ambiguities API (#1469)
Browse files Browse the repository at this point in the history
Explicit execution order ambiguities API.
  • Loading branch information
Ratysz committed Feb 18, 2021
1 parent a5d2501 commit 82d0e84
Show file tree
Hide file tree
Showing 4 changed files with 217 additions and 3 deletions.
170 changes: 169 additions & 1 deletion crates/bevy_ecs/src/schedule/stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<FixedBitSet>::with_capacity(systems.len());
let mut all_dependencies = Vec::<FixedBitSet>::with_capacity(systems.len());
let mut all_dependants = Vec::<FixedBitSet>::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]);
Expand Down Expand Up @@ -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));
}
}
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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);
}
}
13 changes: 13 additions & 0 deletions crates/bevy_ecs/src/schedule/system_container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub(super) trait SystemContainer {
fn label(&self) -> &Option<Cow<'static, str>>;
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;
}

Expand All @@ -20,6 +21,7 @@ pub(super) struct ExclusiveSystemContainer {
label: Option<Cow<'static, str>>,
before: Vec<Cow<'static, str>>,
after: Vec<Cow<'static, str>>,
ambiguity_sets: Vec<Cow<'static, str>>,
}

impl ExclusiveSystemContainer {
Expand All @@ -31,6 +33,7 @@ impl ExclusiveSystemContainer {
label: descriptor.label,
before: descriptor.before,
after: descriptor.after,
ambiguity_sets: descriptor.ambiguity_sets,
}
}

Expand Down Expand Up @@ -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
}
Expand All @@ -85,6 +92,7 @@ pub struct ParallelSystemContainer {
label: Option<Cow<'static, str>>,
before: Vec<Cow<'static, str>>,
after: Vec<Cow<'static, str>>,
ambiguity_sets: Vec<Cow<'static, str>>,
}

impl SystemContainer for ParallelSystemContainer {
Expand Down Expand Up @@ -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()
Expand All @@ -144,6 +156,7 @@ impl ParallelSystemContainer {
label: descriptor.label,
before: descriptor.before,
after: descriptor.after,
ambiguity_sets: descriptor.ambiguity_sets,
}
}

Expand Down
34 changes: 34 additions & 0 deletions crates/bevy_ecs/src/schedule/system_descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ pub struct ParallelSystemDescriptor {
pub(crate) label: Option<Cow<'static, str>>,
pub(crate) before: Vec<Cow<'static, str>>,
pub(crate) after: Vec<Cow<'static, str>>,
pub(crate) ambiguity_sets: Vec<Cow<'static, str>>,
}

fn new_parallel_descriptor(system: BoxedSystem<(), ()>) -> ParallelSystemDescriptor {
Expand All @@ -83,6 +84,7 @@ fn new_parallel_descriptor(system: BoxedSystem<(), ()>) -> ParallelSystemDescrip
label: None,
before: Vec::new(),
after: Vec::new(),
ambiguity_sets: Vec::new(),
}
}

Expand All @@ -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<Cow<'static, str>>) -> 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<Cow<'static, str>>) -> ParallelSystemDescriptor;
}

impl ParallelSystemDescriptorCoercion for ParallelSystemDescriptor {
Expand All @@ -112,6 +118,11 @@ impl ParallelSystemDescriptorCoercion for ParallelSystemDescriptor {
self.after.push(label.into());
self
}

fn in_ambiguity_set(mut self, set: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor {
self.ambiguity_sets.push(set.into());
self
}
}

impl<S> ParallelSystemDescriptorCoercion for S
Expand All @@ -129,6 +140,10 @@ where
fn after(self, label: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor {
new_parallel_descriptor(Box::new(self)).after(label)
}

fn in_ambiguity_set(self, set: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor {
new_parallel_descriptor(Box::new(self)).in_ambiguity_set(set)
}
}

impl ParallelSystemDescriptorCoercion for BoxedSystem<(), ()> {
Expand All @@ -143,6 +158,10 @@ impl ParallelSystemDescriptorCoercion for BoxedSystem<(), ()> {
fn after(self, label: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor {
new_parallel_descriptor(self).after(label)
}

fn in_ambiguity_set(self, set: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor {
new_parallel_descriptor(self).in_ambiguity_set(set)
}
}

#[derive(Debug, Clone, Copy)]
Expand All @@ -158,6 +177,7 @@ pub struct ExclusiveSystemDescriptor {
pub(crate) label: Option<Cow<'static, str>>,
pub(crate) before: Vec<Cow<'static, str>>,
pub(crate) after: Vec<Cow<'static, str>>,
pub(crate) ambiguity_sets: Vec<Cow<'static, str>>,
pub(crate) insertion_point: InsertionPoint,
}

Expand All @@ -167,6 +187,7 @@ fn new_exclusive_descriptor(system: Box<dyn ExclusiveSystem>) -> ExclusiveSystem
label: None,
before: Vec::new(),
after: Vec::new(),
ambiguity_sets: Vec::new(),
insertion_point: InsertionPoint::AtStart,
}
}
Expand All @@ -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<Cow<'static, str>>) -> 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<Cow<'static, str>>) -> ExclusiveSystemDescriptor;

/// Specifies that the system should run with other exclusive systems at the start of stage.
fn at_start(self) -> ExclusiveSystemDescriptor;

Expand Down Expand Up @@ -208,6 +233,11 @@ impl ExclusiveSystemDescriptorCoercion for ExclusiveSystemDescriptor {
self
}

fn in_ambiguity_set(mut self, set: impl Into<Cow<'static, str>>) -> ExclusiveSystemDescriptor {
self.ambiguity_sets.push(set.into());
self
}

fn at_start(mut self) -> ExclusiveSystemDescriptor {
self.insertion_point = InsertionPoint::AtStart;
self
Expand Down Expand Up @@ -240,6 +270,10 @@ where
new_exclusive_descriptor(Box::new(self)).after(label)
}

fn in_ambiguity_set(self, set: impl Into<Cow<'static, str>>) -> ExclusiveSystemDescriptor {
new_exclusive_descriptor(Box::new(self)).in_ambiguity_set(set)
}

fn at_start(self) -> ExclusiveSystemDescriptor {
new_exclusive_descriptor(Box::new(self)).at_start()
}
Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down

0 comments on commit 82d0e84

Please sign in to comment.