From b50be36c360db34e2169b130240b793b151e56f8 Mon Sep 17 00:00:00 2001 From: james7132 Date: Wed, 23 Jun 2021 05:20:56 -0700 Subject: [PATCH 01/23] Add SystemSet::as_sequential --- crates/bevy_ecs/src/schedule/system_set.rs | 43 ++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/crates/bevy_ecs/src/schedule/system_set.rs b/crates/bevy_ecs/src/schedule/system_set.rs index 0702445c79fd0..14a64de16aa6c 100644 --- a/crates/bevy_ecs/src/schedule/system_set.rs +++ b/crates/bevy_ecs/src/schedule/system_set.rs @@ -4,6 +4,10 @@ use crate::schedule::{ }; use super::IntoSystemDescriptor; +use std::sync::atomic::{AtomicU32, Ordering}; +use std::{fmt::Debug, hash::Hash}; + +static NEXT_SEQUENCE_ID: AtomicU32 = AtomicU32::new(0); /// A builder for describing several systems at the same time. #[derive(Default)] @@ -14,6 +18,7 @@ pub struct SystemSet { pub(crate) before: Vec, pub(crate) after: Vec, pub(crate) ambiguity_sets: Vec, + sequential: bool, } impl SystemSet { @@ -108,7 +113,13 @@ impl SystemSet { before, after, ambiguity_sets, + sequential, } = self; + + if sequential { + Self::sequentialize(&mut systems); + } + for descriptor in &mut systems { match descriptor { SystemDescriptor::Parallel(descriptor) => { @@ -131,4 +142,36 @@ impl SystemSet { } (run_criteria, systems) } + + fn sequentialize(systems: &mut Vec) { + let start = NEXT_SEQUENCE_ID.fetch_add(systems.len() as u32, Ordering::Relaxed); + let mut last_label: Option = None; + for (idx, descriptor) in systems.iter_mut().enumerate() { + let label = SequenceId(start + idx as u32); + match descriptor { + SystemDescriptor::Parallel(descriptor) => { + descriptor.labels.push(label.dyn_clone()); + if let Some(ref after) = last_label { + descriptor.after.push(after.dyn_clone()); + } + } + SystemDescriptor::Exclusive(descriptor) => { + descriptor.labels.push(label.dyn_clone()); + if let Some(ref after) = last_label { + descriptor.after.push(after.dyn_clone()); + } + } + } + last_label = Some(label); + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +struct SequenceId(u32); + +impl SystemLabel for SequenceId { + fn dyn_clone(&self) -> Box { + Box::new(::clone(self)) + } } From e8acce4c55e0ebd68467f925a9712ce239538d57 Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 23 Jun 2021 10:39:06 -0700 Subject: [PATCH 02/23] Update crates/bevy_ecs/src/schedule/system_set.rs Co-authored-by: Nathan Ward <43621845+NathanSWard@users.noreply.github.com> --- crates/bevy_ecs/src/schedule/system_set.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/system_set.rs b/crates/bevy_ecs/src/schedule/system_set.rs index 14a64de16aa6c..bc2e0704ff987 100644 --- a/crates/bevy_ecs/src/schedule/system_set.rs +++ b/crates/bevy_ecs/src/schedule/system_set.rs @@ -4,8 +4,11 @@ use crate::schedule::{ }; use super::IntoSystemDescriptor; -use std::sync::atomic::{AtomicU32, Ordering}; -use std::{fmt::Debug, hash::Hash}; +use std::{ + fmt::Debug, + hash::Hash, + sync::atomic::{AtomicU32, Ordering}, +}; static NEXT_SEQUENCE_ID: AtomicU32 = AtomicU32::new(0); From c312419b16a62b5a18675eda31bb057ac4f42e1d Mon Sep 17 00:00:00 2001 From: james7132 Date: Wed, 23 Jun 2021 12:54:18 -0700 Subject: [PATCH 03/23] Fix formatting and use wrapping_add --- crates/bevy_ecs/src/schedule/system_set.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/schedule/system_set.rs b/crates/bevy_ecs/src/schedule/system_set.rs index bc2e0704ff987..7dd82fe6b70bb 100644 --- a/crates/bevy_ecs/src/schedule/system_set.rs +++ b/crates/bevy_ecs/src/schedule/system_set.rs @@ -150,7 +150,7 @@ impl SystemSet { let start = NEXT_SEQUENCE_ID.fetch_add(systems.len() as u32, Ordering::Relaxed); let mut last_label: Option = None; for (idx, descriptor) in systems.iter_mut().enumerate() { - let label = SequenceId(start + idx as u32); + let label = SequenceId(start.wrapping_add(idx as u32)); match descriptor { SystemDescriptor::Parallel(descriptor) => { descriptor.labels.push(label.dyn_clone()); From 8502a27dd170b25b347cd1f38de8cec16f60c8a3 Mon Sep 17 00:00:00 2001 From: james7132 Date: Wed, 23 Jun 2021 14:17:26 -0700 Subject: [PATCH 04/23] Add unit test --- crates/bevy_ecs/src/schedule/system_set.rs | 57 ++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/crates/bevy_ecs/src/schedule/system_set.rs b/crates/bevy_ecs/src/schedule/system_set.rs index 7dd82fe6b70bb..05ff97975dcbc 100644 --- a/crates/bevy_ecs/src/schedule/system_set.rs +++ b/crates/bevy_ecs/src/schedule/system_set.rs @@ -178,3 +178,60 @@ impl SystemLabel for SequenceId { Box::new(::clone(self)) } } + +#[cfg(test)] +mod test { + use super::*; + use crate::prelude::*; + + fn dummy_system() {} + + fn labels(system: &SystemDescriptor) -> &Vec> { + match system { + SystemDescriptor::Parallel(descriptor) => &descriptor.labels, + SystemDescriptor::Exclusive(descriptor) => &descriptor.labels, + } + } + + fn after(system: &SystemDescriptor) -> &Vec> { + match system { + SystemDescriptor::Parallel(descriptor) => &descriptor.after, + SystemDescriptor::Exclusive(descriptor) => &descriptor.after, + } + } + + #[test] + pub fn sequential_adds_labels() { + let system_set = SystemSet::new() + .as_sequential() + .with_system(dummy_system.system()) + .with_system(dummy_system.system()) + .with_system(dummy_system.system()); + let (_, systems) = system_set.bake(); + + assert_eq!(systems.len(), 3); + assert_eq!(labels(&systems[0]), &vec![SequenceId(0).dyn_clone()]); + assert_eq!(labels(&systems[1]), &vec![SequenceId(1).dyn_clone()]); + assert_eq!(labels(&systems[2]), &vec![SequenceId(2).dyn_clone()]); + assert_eq!(after(&systems[0]), &vec![]); + assert_eq!(after(&systems[1]), &vec![SequenceId(0).dyn_clone()]); + assert_eq!(after(&systems[2]), &vec![SequenceId(1).dyn_clone()]); + } + + #[test] + pub fn non_sequential_has_no_labels_by_default() { + let system_set = SystemSet::new() + .with_system(dummy_system.system()) + .with_system(dummy_system.system()) + .with_system(dummy_system.system()); + let (_, systems) = system_set.bake(); + + assert_eq!(systems.len(), 3); + assert_eq!(labels(&systems[0]), &vec![]); + assert_eq!(labels(&systems[1]), &vec![]); + assert_eq!(labels(&systems[2]), &vec![]); + assert_eq!(after(&systems[0]), &vec![]); + assert_eq!(after(&systems[1]), &vec![]); + assert_eq!(after(&systems[2]), &vec![]); + } +} From b7c3a48e56a6a76cd8fcd0341fba3e53ac01bc1e Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Jun 2021 02:47:20 -0700 Subject: [PATCH 05/23] Add SystemGraph implementation --- crates/bevy_ecs/src/lib.rs | 3 +- crates/bevy_ecs/src/schedule/mod.rs | 2 + crates/bevy_ecs/src/schedule/system_graph.rs | 264 +++++++++++++++++++ 3 files changed, 268 insertions(+), 1 deletion(-) create mode 100644 crates/bevy_ecs/src/schedule/system_graph.rs diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index dfccb44440db5..86e1293c0fde0 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -30,7 +30,8 @@ pub mod prelude { schedule::{ AmbiguitySetLabel, ExclusiveSystemDescriptorCoercion, ParallelSystemDescriptorCoercion, RunCriteria, RunCriteriaDescriptorCoercion, RunCriteriaLabel, RunCriteriaPiping, - Schedule, Stage, StageLabel, State, SystemLabel, SystemSet, SystemStage, + Schedule, Stage, StageLabel, State, SystemGraph, SystemGraphJoinExt, SystemLabel, + SystemSet, SystemStage, }, system::{ Commands, ConfigurableSystem, In, IntoChainSystem, IntoExclusiveSystem, IntoSystem, diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 4af63a220a8b3..7b1b27873bbd9 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -12,6 +12,7 @@ mod stage; mod state; mod system_container; mod system_descriptor; +mod system_graph; mod system_set; pub use executor::*; @@ -23,6 +24,7 @@ pub use stage::*; pub use state::*; pub use system_container::*; pub use system_descriptor::*; +pub use system_graph::*; pub use system_set::*; use std::fmt::Debug; diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs new file mode 100644 index 0000000000000..632386ef03bae --- /dev/null +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -0,0 +1,264 @@ +use crate::schedule::{SystemDescriptor, SystemLabel, SystemSet}; +use std::{ + collections::HashMap, + fmt::Debug, + sync::{ + atomic::{AtomicU32, Ordering}, + Arc, Mutex, + }, +}; + +static NEXT_NODE_ID: AtomicU32 = AtomicU32::new(0); + +/// A builder for creating graphs of dependent parallel execution within a `SystemStage`. +/// +/// # Sequential Execution +/// The simplest graph is a sequence of systems that need to run one after another. +/// ``` +/// # use bevy_ecs::prelude::*; +/// # fn sys_a() {} +/// # fn sys_b() {} +/// # fn sys_c() {} +/// let graph = SystemGraph::new(); +/// graph +/// .root(sys_a.system()) +/// .then(sys_b.system()) +/// .then(sys_c.system()); +/// +/// // Convert into a SystemSet +/// let system_set: SystemSet = graph.into(); +/// ``` +/// +/// # Fan Out +/// `SystemGraphNode::then` can be called repeatedly on the same node to create multiple fan-out +/// branches. All fanned out systems will not execute until the original has finished. +/// ``` +/// # use bevy_ecs::prelude::*; +/// # fn sys_a() {} +/// # fn sys_b() {} +/// # fn sys_c() {} +/// # fn sys_d() {} +/// # fn sys_e() {} +/// let graph = SystemGraph::new(); +/// +/// let start_a = graph.root(sys_a.system()); +/// +/// start_a.then(sys_b.system()); +/// start_a.then(sys_c.system()); +/// +/// start_a +/// .then(sys_d.system()) +/// .then(sys_e.system()); +/// +/// // Convert into a SystemSet +/// let system_set: SystemSet = graph.into(); +/// ``` +/// +/// # Fan In +/// A graph node can wait on multiple systems before running. +/// `SystemGraphJoinExt::join_into` is implemented on any type that iterates over +/// `SystemGraphNode`. +/// ``` +/// # use bevy_ecs::prelude::*; +/// # fn sys_a() {} +/// # fn sys_b() {} +/// # fn sys_c() {} +/// # fn sys_d() {} +/// # fn sys_e() {} +/// let graph = SystemGraph::new(); +/// +/// let start_a = graph.root(sys_a.system()); +/// let start_b = graph.root(sys_b.system()); +/// let start_c = graph.root(sys_c.system()); +/// +/// vec![start_a, start_b, start_c] +/// .join_into(sys_d.system()) +/// .then(sys_e.system()); +/// +/// // Convert into a SystemSet +/// let system_set: SystemSet = graph.into(); +/// ``` +#[derive(Clone, Default)] +pub struct SystemGraph(Arc>>); + +impl SystemGraph { + pub fn new() -> Self { + Self::default() + } + + pub fn root(&self, system: impl Into) -> SystemGraphNode { + self.create_node(system.into()) + } + + pub fn is_same_graph(&self, other: &Self) -> bool { + Arc::ptr_eq(&self.0, &other.0) + } + + fn create_node(&self, mut system: SystemDescriptor) -> SystemGraphNode { + let id = NodeId(NEXT_NODE_ID.fetch_add(1, Ordering::Relaxed)); + match &mut system { + SystemDescriptor::Parallel(descriptor) => descriptor.labels.push(id.dyn_clone()), + SystemDescriptor::Exclusive(descriptor) => descriptor.labels.push(id.dyn_clone()), + } + self.0.lock().unwrap().insert(id, system); + SystemGraphNode { + id, + graph: self.clone(), + } + } + + fn add_dependency(&self, src: NodeId, dst: NodeId) { + if let Some(system) = self.0.lock().unwrap().get_mut(&dst) { + match system { + SystemDescriptor::Parallel(descriptor) => descriptor.after.push(src.dyn_clone()), + SystemDescriptor::Exclusive(descriptor) => descriptor.after.push(src.dyn_clone()), + } + } else { + panic!( + "Attempted to add dependency for {:?}, which doesn't exist.", + dst + ); + } + } +} + +/// A draining conversion from [SystemGraph] to [SystemSet]. All other clones of the same graph +/// will be empty afterwards. +/// +/// [SystemGraph]: crate::schedule::SystemSet +/// [SystemSet]: crate::schedule::SystemSet +impl From for SystemSet { + fn from(graph: SystemGraph) -> Self { + let mut system_set = SystemSet::new(); + for (_, system) in graph.0.lock().unwrap().drain() { + system_set = system_set.with_system(system); + } + system_set + } +} + +#[derive(Clone)] +pub struct SystemGraphNode { + id: NodeId, + graph: SystemGraph, +} + +impl SystemGraphNode { + pub fn then(&self, next: impl Into) -> SystemGraphNode { + let node = self.graph.create_node(next.into()); + self.add_dependent(node.id); + node + } + + fn add_dependent(&self, dst: NodeId) { + self.graph.add_dependency(self.id, dst); + } +} + +pub trait SystemGraphJoinExt: Sized + IntoIterator { + fn join_into(self, next: impl Into) -> SystemGraphNode { + let mut nodes = self.into_iter().peekable(); + let output = nodes + .peek() + .map(|node| { + let output_node = node.graph.create_node(next.into()); + node.add_dependent(output_node.id); + output_node + }) + .expect("Attempted to join a collection of zero nodes."); + + for node in nodes { + assert!( + output.graph.is_same_graph(&node.graph), + "Joined graph nodes should be from the same graph." + ); + node.add_dependent(output.id); + } + + output + } +} + +impl> SystemGraphJoinExt for T {} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +struct NodeId(u32); + +impl SystemLabel for NodeId { + fn dyn_clone(&self) -> Box { + Box::new(::clone(self)) + } +} + +#[cfg(test)] +mod test { + use crate::prelude::*; + use crate::schedule::SystemDescriptor; + + fn dummy_system() {} + + #[test] + pub fn graph_creates_accurate_system_counts() { + let graph = SystemGraph::new(); + let a = graph + .root(dummy_system.system()) + .then(dummy_system.system()) + .then(dummy_system.system()) + .then(dummy_system.system()); + let b = graph + .root(dummy_system.system()) + .then(dummy_system.system()); + let c = graph + .root(dummy_system.system()) + .then(dummy_system.system()) + .then(dummy_system.system()); + vec![a, b, c] + .join_into(dummy_system.system()) + .then(dummy_system.system()); + let system_set: SystemSet = graph.into(); + let (_, systems) = system_set.bake(); + + assert_eq!(systems.len(), 11); + } + + #[test] + pub fn all_nodes_are_labeled() { + let graph = SystemGraph::new(); + let a = graph + .root(dummy_system.system()) + .then(dummy_system.system()) + .then(dummy_system.system()) + .then(dummy_system.system()); + let b = graph + .root(dummy_system.system()) + .then(dummy_system.system()); + let c = graph + .root(dummy_system.system()) + .then(dummy_system.system()) + .then(dummy_system.system()); + vec![a, b, c] + .join_into(dummy_system.system()) + .then(dummy_system.system()); + let system_set: SystemSet = graph.into(); + let (_, systems) = system_set.bake(); + + let mut root_count = 0; + for system in systems { + match system { + SystemDescriptor::Parallel(desc) => { + assert!(!desc.labels.is_empty()); + if desc.after.is_empty() { + root_count += 1; + } + } + SystemDescriptor::Exclusive(desc) => { + assert!(!desc.labels.is_empty()); + if desc.after.is_empty() { + root_count += 1; + } + } + } + } + assert_eq!(root_count, 3); + } +} From 3ba637f56900194cbb8aabceba98461fadd3cd53 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Jun 2021 02:51:26 -0700 Subject: [PATCH 06/23] Revert SystemSet::as_sequential --- crates/bevy_ecs/src/schedule/system_set.rs | 95 ---------------------- 1 file changed, 95 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/system_set.rs b/crates/bevy_ecs/src/schedule/system_set.rs index 05ff97975dcbc..528edc4262b77 100644 --- a/crates/bevy_ecs/src/schedule/system_set.rs +++ b/crates/bevy_ecs/src/schedule/system_set.rs @@ -21,7 +21,6 @@ pub struct SystemSet { pub(crate) before: Vec, pub(crate) after: Vec, pub(crate) ambiguity_sets: Vec, - sequential: bool, } impl SystemSet { @@ -116,13 +115,8 @@ impl SystemSet { before, after, ambiguity_sets, - sequential, } = self; - if sequential { - Self::sequentialize(&mut systems); - } - for descriptor in &mut systems { match descriptor { SystemDescriptor::Parallel(descriptor) => { @@ -145,93 +139,4 @@ impl SystemSet { } (run_criteria, systems) } - - fn sequentialize(systems: &mut Vec) { - let start = NEXT_SEQUENCE_ID.fetch_add(systems.len() as u32, Ordering::Relaxed); - let mut last_label: Option = None; - for (idx, descriptor) in systems.iter_mut().enumerate() { - let label = SequenceId(start.wrapping_add(idx as u32)); - match descriptor { - SystemDescriptor::Parallel(descriptor) => { - descriptor.labels.push(label.dyn_clone()); - if let Some(ref after) = last_label { - descriptor.after.push(after.dyn_clone()); - } - } - SystemDescriptor::Exclusive(descriptor) => { - descriptor.labels.push(label.dyn_clone()); - if let Some(ref after) = last_label { - descriptor.after.push(after.dyn_clone()); - } - } - } - last_label = Some(label); - } - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -struct SequenceId(u32); - -impl SystemLabel for SequenceId { - fn dyn_clone(&self) -> Box { - Box::new(::clone(self)) - } -} - -#[cfg(test)] -mod test { - use super::*; - use crate::prelude::*; - - fn dummy_system() {} - - fn labels(system: &SystemDescriptor) -> &Vec> { - match system { - SystemDescriptor::Parallel(descriptor) => &descriptor.labels, - SystemDescriptor::Exclusive(descriptor) => &descriptor.labels, - } - } - - fn after(system: &SystemDescriptor) -> &Vec> { - match system { - SystemDescriptor::Parallel(descriptor) => &descriptor.after, - SystemDescriptor::Exclusive(descriptor) => &descriptor.after, - } - } - - #[test] - pub fn sequential_adds_labels() { - let system_set = SystemSet::new() - .as_sequential() - .with_system(dummy_system.system()) - .with_system(dummy_system.system()) - .with_system(dummy_system.system()); - let (_, systems) = system_set.bake(); - - assert_eq!(systems.len(), 3); - assert_eq!(labels(&systems[0]), &vec![SequenceId(0).dyn_clone()]); - assert_eq!(labels(&systems[1]), &vec![SequenceId(1).dyn_clone()]); - assert_eq!(labels(&systems[2]), &vec![SequenceId(2).dyn_clone()]); - assert_eq!(after(&systems[0]), &vec![]); - assert_eq!(after(&systems[1]), &vec![SequenceId(0).dyn_clone()]); - assert_eq!(after(&systems[2]), &vec![SequenceId(1).dyn_clone()]); - } - - #[test] - pub fn non_sequential_has_no_labels_by_default() { - let system_set = SystemSet::new() - .with_system(dummy_system.system()) - .with_system(dummy_system.system()) - .with_system(dummy_system.system()); - let (_, systems) = system_set.bake(); - - assert_eq!(systems.len(), 3); - assert_eq!(labels(&systems[0]), &vec![]); - assert_eq!(labels(&systems[1]), &vec![]); - assert_eq!(labels(&systems[2]), &vec![]); - assert_eq!(after(&systems[0]), &vec![]); - assert_eq!(after(&systems[1]), &vec![]); - assert_eq!(after(&systems[2]), &vec![]); - } } From 384f913f8c4d2f9be54185b258470e283eb3c14c Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Jun 2021 02:55:53 -0700 Subject: [PATCH 07/23] Remove extra space --- crates/bevy_ecs/src/schedule/system_set.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_ecs/src/schedule/system_set.rs b/crates/bevy_ecs/src/schedule/system_set.rs index 528edc4262b77..09214fba91122 100644 --- a/crates/bevy_ecs/src/schedule/system_set.rs +++ b/crates/bevy_ecs/src/schedule/system_set.rs @@ -116,7 +116,6 @@ impl SystemSet { after, ambiguity_sets, } = self; - for descriptor in &mut systems { match descriptor { SystemDescriptor::Parallel(descriptor) => { From 14e403ad693185751341807ca7e68a586ad66f38 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Jun 2021 12:49:06 -0700 Subject: [PATCH 08/23] Review changes + doc comments --- crates/bevy_ecs/src/schedule/system_graph.rs | 37 ++++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs index 632386ef03bae..3bfb9c95485b2 100644 --- a/crates/bevy_ecs/src/schedule/system_graph.rs +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -1,10 +1,12 @@ use crate::schedule::{SystemDescriptor, SystemLabel, SystemSet}; +use bevy_ecs_macros::all_tuples; +use bevy_utils::HashMap; +use parking_lot::Mutex; use std::{ - collections::HashMap, fmt::Debug, sync::{ atomic::{AtomicU32, Ordering}, - Arc, Mutex, + Arc, }, }; @@ -86,10 +88,13 @@ impl SystemGraph { Self::default() } + /// Creates a root graph node without any dependencies. A graph can have multiple distinct + /// root nodes. pub fn root(&self, system: impl Into) -> SystemGraphNode { self.create_node(system.into()) } + /// Checks if two graphs instances point to the same logical underlying graph. pub fn is_same_graph(&self, other: &Self) -> bool { Arc::ptr_eq(&self.0, &other.0) } @@ -100,7 +105,7 @@ impl SystemGraph { SystemDescriptor::Parallel(descriptor) => descriptor.labels.push(id.dyn_clone()), SystemDescriptor::Exclusive(descriptor) => descriptor.labels.push(id.dyn_clone()), } - self.0.lock().unwrap().insert(id, system); + self.0.lock().insert(id, system); SystemGraphNode { id, graph: self.clone(), @@ -108,7 +113,7 @@ impl SystemGraph { } fn add_dependency(&self, src: NodeId, dst: NodeId) { - if let Some(system) = self.0.lock().unwrap().get_mut(&dst) { + if let Some(system) = self.0.lock().get_mut(&dst) { match system { SystemDescriptor::Parallel(descriptor) => descriptor.after.push(src.dyn_clone()), SystemDescriptor::Exclusive(descriptor) => descriptor.after.push(src.dyn_clone()), @@ -122,15 +127,14 @@ impl SystemGraph { } } -/// A draining conversion from [SystemGraph] to [SystemSet]. All other clones of the same graph -/// will be empty afterwards. +/// A draining conversion to [SystemSet]. All other clones of the same graph will be empty +/// afterwards. /// -/// [SystemGraph]: crate::schedule::SystemSet /// [SystemSet]: crate::schedule::SystemSet impl From for SystemSet { fn from(graph: SystemGraph) -> Self { let mut system_set = SystemSet::new(); - for (_, system) in graph.0.lock().unwrap().drain() { + for (_, system) in graph.0.lock().drain() { system_set = system_set.with_system(system); } system_set @@ -144,15 +148,14 @@ pub struct SystemGraphNode { } impl SystemGraphNode { + /// Creates a new node in the graph and adds the current node as it's dependency. + /// + /// This function can be called multiple times to create pub fn then(&self, next: impl Into) -> SystemGraphNode { let node = self.graph.create_node(next.into()); - self.add_dependent(node.id); + self.graph.add_dependency(self.id, node.id); node } - - fn add_dependent(&self, dst: NodeId) { - self.graph.add_dependency(self.id, dst); - } } pub trait SystemGraphJoinExt: Sized + IntoIterator { @@ -160,11 +163,7 @@ pub trait SystemGraphJoinExt: Sized + IntoIterator { let mut nodes = self.into_iter().peekable(); let output = nodes .peek() - .map(|node| { - let output_node = node.graph.create_node(next.into()); - node.add_dependent(output_node.id); - output_node - }) + .map(|node| node.graph.create_node(next.into())) .expect("Attempted to join a collection of zero nodes."); for node in nodes { @@ -172,7 +171,7 @@ pub trait SystemGraphJoinExt: Sized + IntoIterator { output.graph.is_same_graph(&node.graph), "Joined graph nodes should be from the same graph." ); - node.add_dependent(output.id); + output.graph.add_dependency(node.id, output.id); } output From 1415e904d4c676f0a6ee1529a522eb96955fa909 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Jun 2021 15:36:58 -0700 Subject: [PATCH 09/23] Use Rc/RefCell instead of Arc/Mutex --- crates/bevy_ecs/src/schedule/system_graph.rs | 22 +++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs index 3bfb9c95485b2..a704f37e7ff41 100644 --- a/crates/bevy_ecs/src/schedule/system_graph.rs +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -1,13 +1,11 @@ use crate::schedule::{SystemDescriptor, SystemLabel, SystemSet}; use bevy_ecs_macros::all_tuples; use bevy_utils::HashMap; -use parking_lot::Mutex; use std::{ fmt::Debug, - sync::{ - atomic::{AtomicU32, Ordering}, - Arc, - }, + rc::Rc, + cell::RefCell, + sync::atomic::{AtomicU32, Ordering}, }; static NEXT_NODE_ID: AtomicU32 = AtomicU32::new(0); @@ -80,8 +78,12 @@ static NEXT_NODE_ID: AtomicU32 = AtomicU32::new(0); /// // Convert into a SystemSet /// let system_set: SystemSet = graph.into(); /// ``` +/// +/// # Cloning +/// This type is backed by a Rc, so cloning it will still point to the same logical +/// underlying graph. #[derive(Clone, Default)] -pub struct SystemGraph(Arc>>); +pub struct SystemGraph(Rc>>); impl SystemGraph { pub fn new() -> Self { @@ -96,7 +98,7 @@ impl SystemGraph { /// Checks if two graphs instances point to the same logical underlying graph. pub fn is_same_graph(&self, other: &Self) -> bool { - Arc::ptr_eq(&self.0, &other.0) + Rc::ptr_eq(&self.0, &other.0) } fn create_node(&self, mut system: SystemDescriptor) -> SystemGraphNode { @@ -105,7 +107,7 @@ impl SystemGraph { SystemDescriptor::Parallel(descriptor) => descriptor.labels.push(id.dyn_clone()), SystemDescriptor::Exclusive(descriptor) => descriptor.labels.push(id.dyn_clone()), } - self.0.lock().insert(id, system); + self.0.borrow_mut().insert(id, system); SystemGraphNode { id, graph: self.clone(), @@ -113,7 +115,7 @@ impl SystemGraph { } fn add_dependency(&self, src: NodeId, dst: NodeId) { - if let Some(system) = self.0.lock().get_mut(&dst) { + if let Some(system) = self.0.borrow_mut().get_mut(&dst) { match system { SystemDescriptor::Parallel(descriptor) => descriptor.after.push(src.dyn_clone()), SystemDescriptor::Exclusive(descriptor) => descriptor.after.push(src.dyn_clone()), @@ -134,7 +136,7 @@ impl SystemGraph { impl From for SystemSet { fn from(graph: SystemGraph) -> Self { let mut system_set = SystemSet::new(); - for (_, system) in graph.0.lock().drain() { + for (_, system) in graph.0.borrow_mut().drain() { system_set = system_set.with_system(system); } system_set From a1e10d5081163cfd6c4a6f337eacd845a0665e2a Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 24 Jun 2021 16:56:15 -0700 Subject: [PATCH 10/23] Support split_into as a utilty function for fan-out topologies --- crates/bevy_ecs/macros/src/lib.rs | 4 +- crates/bevy_ecs/src/lib.rs | 4 +- crates/bevy_ecs/src/schedule/system_graph.rs | 70 +++++++++++++++++--- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index ed6fd79f95e8f..7b577bd59a5ba 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -47,9 +47,9 @@ impl Parse for AllTuples { #[proc_macro] pub fn all_tuples(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as AllTuples); - let len = (input.start..=input.end).count(); + let len = (0..=input.end).count(); let mut ident_tuples = Vec::with_capacity(len); - for i in input.start..=input.end { + for i in 0..=input.end { let idents = input .idents .iter() diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 86e1293c0fde0..649dcd5574999 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -30,8 +30,8 @@ pub mod prelude { schedule::{ AmbiguitySetLabel, ExclusiveSystemDescriptorCoercion, ParallelSystemDescriptorCoercion, RunCriteria, RunCriteriaDescriptorCoercion, RunCriteriaLabel, RunCriteriaPiping, - Schedule, Stage, StageLabel, State, SystemGraph, SystemGraphJoinExt, SystemLabel, - SystemSet, SystemStage, + Schedule, Stage, StageLabel, State, SystemGraph, SystemGraphJoinExt, SystemGroup, + SystemLabel, SystemSet, SystemStage, }, system::{ Commands, ConfigurableSystem, In, IntoChainSystem, IntoExclusiveSystem, IntoSystem, diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs index a704f37e7ff41..be907474e6805 100644 --- a/crates/bevy_ecs/src/schedule/system_graph.rs +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -2,9 +2,9 @@ use crate::schedule::{SystemDescriptor, SystemLabel, SystemSet}; use bevy_ecs_macros::all_tuples; use bevy_utils::HashMap; use std::{ + cell::RefCell, fmt::Debug, rc::Rc, - cell::RefCell, sync::atomic::{AtomicU32, Ordering}, }; @@ -39,16 +39,20 @@ static NEXT_NODE_ID: AtomicU32 = AtomicU32::new(0); /// # fn sys_c() {} /// # fn sys_d() {} /// # fn sys_e() {} +/// # fn sys_f() {} /// let graph = SystemGraph::new(); /// -/// let start_a = graph.root(sys_a.system()); -/// -/// start_a.then(sys_b.system()); -/// start_a.then(sys_c.system()); +/// // Split out from one original node. +/// let (c, b, d) = graph.root(sys_a.system()) +/// .split_into(( +/// sys_b.system(), +/// sys_c.system(), +/// sys_d.system(), +/// )); /// -/// start_a -/// .then(sys_d.system()) -/// .then(sys_e.system()); +/// // The returned nodes can then be continued. +/// d.then(sys_e.system()) +/// .then(sys_f.system()); /// /// // Convert into a SystemSet /// let system_set: SystemSet = graph.into(); @@ -78,7 +82,7 @@ static NEXT_NODE_ID: AtomicU32 = AtomicU32::new(0); /// // Convert into a SystemSet /// let system_set: SystemSet = graph.into(); /// ``` -/// +/// /// # Cloning /// This type is backed by a Rc, so cloning it will still point to the same logical /// underlying graph. @@ -152,14 +156,60 @@ pub struct SystemGraphNode { impl SystemGraphNode { /// Creates a new node in the graph and adds the current node as it's dependency. /// - /// This function can be called multiple times to create + /// This function can be called multiple times to add mulitple systems to the graph, + /// all of which will not execute until original node's system has finished running. pub fn then(&self, next: impl Into) -> SystemGraphNode { let node = self.graph.create_node(next.into()); self.graph.add_dependency(self.id, node.id); node } + + /// Fans out from the given node into multiple dependent systems. All provided + /// systems will not run until the original node's system finishes running. + /// + /// Functionally equivalent to calling `SystemGraphNode::then` multiple times. + pub fn split_into(&self, system_group: T) -> T::Output { + system_group.split_into(self) + } +} + +pub trait SystemGroup { + type Output; + fn split_into(self, src: &SystemGraphNode) -> Self::Output; +} + +impl> SystemGroup for Vec { + type Output = Vec; + fn split_into(self, src: &SystemGraphNode) -> Self::Output { + self.into_iter().map(|sys| src.then(sys.into())).collect() + } } +macro_rules! ignore_first { + ($_first:ident, $second:ty) => { + $second + }; +} + +macro_rules! impl_system_group_tuple { + ($($param: ident),*) => { + impl<$($param: Into),*> SystemGroup for ($($param,)*) { + // HACK: using repeat macros without using the param in it fails to compile. + // The ignore_first here "uses" the parameter and just discards it. + type Output = ($(ignore_first!($param, SystemGraphNode),)*); + + #[inline] + #[allow(non_snake_case)] + fn split_into(self, src: &SystemGraphNode) -> Self::Output { + let ($($param,)*) = self; + ($(src.then($param::into($param)),)*) + } + } + }; +} + +all_tuples!(impl_system_group_tuple, 2, 16, T); + pub trait SystemGraphJoinExt: Sized + IntoIterator { fn join_into(self, next: impl Into) -> SystemGraphNode { let mut nodes = self.into_iter().peekable(); From 40aa65879b7f257eefed43d3513bb8ce3ec8ec19 Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 25 Jun 2021 01:33:59 -0700 Subject: [PATCH 11/23] Use separate graph and node counters --- crates/bevy_ecs/src/schedule/system_graph.rs | 38 ++++++++++++++------ 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs index be907474e6805..c715167771644 100644 --- a/crates/bevy_ecs/src/schedule/system_graph.rs +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -8,7 +8,7 @@ use std::{ sync::atomic::{AtomicU32, Ordering}, }; -static NEXT_NODE_ID: AtomicU32 = AtomicU32::new(0); +static NEXT_GRAPH_ID: AtomicU32 = AtomicU32::new(0); /// A builder for creating graphs of dependent parallel execution within a `SystemStage`. /// @@ -86,12 +86,18 @@ static NEXT_NODE_ID: AtomicU32 = AtomicU32::new(0); /// # Cloning /// This type is backed by a Rc, so cloning it will still point to the same logical /// underlying graph. -#[derive(Clone, Default)] -pub struct SystemGraph(Rc>>); +#[derive(Clone)] +pub struct SystemGraph { + id: u32, + nodes: Rc>>, +} impl SystemGraph { pub fn new() -> Self { - Self::default() + Self { + id: NEXT_GRAPH_ID.fetch_add(1, Ordering::Relaxed), + nodes: Default::default(), + } } /// Creates a root graph node without any dependencies. A graph can have multiple distinct @@ -102,16 +108,25 @@ impl SystemGraph { /// Checks if two graphs instances point to the same logical underlying graph. pub fn is_same_graph(&self, other: &Self) -> bool { - Rc::ptr_eq(&self.0, &other.0) + self.id == other.id } fn create_node(&self, mut system: SystemDescriptor) -> SystemGraphNode { - let id = NodeId(NEXT_NODE_ID.fetch_add(1, Ordering::Relaxed)); + let mut nodes = self.nodes.borrow_mut(); + assert!( + nodes.len() <= u32::MAX as usize, + "Cannot add more than {} systems to a SystemGraph", + u32::MAX + ); + let id = NodeId { + graph_id: self.id, + node_id: nodes.len() as u32, + }; match &mut system { SystemDescriptor::Parallel(descriptor) => descriptor.labels.push(id.dyn_clone()), SystemDescriptor::Exclusive(descriptor) => descriptor.labels.push(id.dyn_clone()), } - self.0.borrow_mut().insert(id, system); + nodes.insert(id, system); SystemGraphNode { id, graph: self.clone(), @@ -119,7 +134,7 @@ impl SystemGraph { } fn add_dependency(&self, src: NodeId, dst: NodeId) { - if let Some(system) = self.0.borrow_mut().get_mut(&dst) { + if let Some(system) = self.nodes.borrow_mut().get_mut(&dst) { match system { SystemDescriptor::Parallel(descriptor) => descriptor.after.push(src.dyn_clone()), SystemDescriptor::Exclusive(descriptor) => descriptor.after.push(src.dyn_clone()), @@ -140,7 +155,7 @@ impl SystemGraph { impl From for SystemSet { fn from(graph: SystemGraph) -> Self { let mut system_set = SystemSet::new(); - for (_, system) in graph.0.borrow_mut().drain() { + for (_, system) in graph.nodes.borrow_mut().drain() { system_set = system_set.with_system(system); } system_set @@ -233,7 +248,10 @@ pub trait SystemGraphJoinExt: Sized + IntoIterator { impl> SystemGraphJoinExt for T {} #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -struct NodeId(u32); +struct NodeId { + graph_id: u32, + node_id: u32, +} impl SystemLabel for NodeId { fn dyn_clone(&self) -> Box { From b0fc2523a0b97c1186b2ccf4eae29370a3b6af80 Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 25 Jun 2021 16:03:16 -0700 Subject: [PATCH 12/23] Add tuple implementations for SystemJoin --- crates/bevy_ecs/src/lib.rs | 4 +- crates/bevy_ecs/src/schedule/system_graph.rs | 131 ++++++++++++------- 2 files changed, 89 insertions(+), 46 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 649dcd5574999..90dc89dcd3c6a 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -30,8 +30,8 @@ pub mod prelude { schedule::{ AmbiguitySetLabel, ExclusiveSystemDescriptorCoercion, ParallelSystemDescriptorCoercion, RunCriteria, RunCriteriaDescriptorCoercion, RunCriteriaLabel, RunCriteriaPiping, - Schedule, Stage, StageLabel, State, SystemGraph, SystemGraphJoinExt, SystemGroup, - SystemLabel, SystemSet, SystemStage, + Schedule, Stage, StageLabel, State, SystemGraph, SystemGroup, SystemJoin, SystemLabel, + SystemSet, SystemStage, }, system::{ Commands, ConfigurableSystem, In, IntoChainSystem, IntoExclusiveSystem, IntoSystem, diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs index c715167771644..a336ea9ce7756 100644 --- a/crates/bevy_ecs/src/schedule/system_graph.rs +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -30,8 +30,8 @@ static NEXT_GRAPH_ID: AtomicU32 = AtomicU32::new(0); /// ``` /// /// # Fan Out -/// `SystemGraphNode::then` can be called repeatedly on the same node to create multiple fan-out -/// branches. All fanned out systems will not execute until the original has finished. +/// [fork] can be used to fan out into multiple branches. All fanned out systems will not execute +/// until the original has finished. /// ``` /// # use bevy_ecs::prelude::*; /// # fn sys_a() {} @@ -42,26 +42,25 @@ static NEXT_GRAPH_ID: AtomicU32 = AtomicU32::new(0); /// # fn sys_f() {} /// let graph = SystemGraph::new(); /// -/// // Split out from one original node. +/// // Fork out from one original node. /// let (c, b, d) = graph.root(sys_a.system()) -/// .split_into(( +/// .fork(( /// sys_b.system(), /// sys_c.system(), /// sys_d.system(), /// )); /// -/// // The returned nodes can then be continued. -/// d.then(sys_e.system()) -/// .then(sys_f.system()); +/// // Alternatively, calling "then" repeatedly achieves the same thing. +/// let e = d.then(sys_e.system()); +/// let f = d.then(sys_f.system()); /// /// // Convert into a SystemSet /// let system_set: SystemSet = graph.into(); /// ``` /// /// # Fan In -/// A graph node can wait on multiple systems before running. -/// `SystemGraphJoinExt::join_into` is implemented on any type that iterates over -/// `SystemGraphNode`. +/// A graph node can wait on multiple systems before running via [join]. The system will not run +/// until all prior systems are finished. /// ``` /// # use bevy_ecs::prelude::*; /// # fn sys_a() {} @@ -75,17 +74,40 @@ static NEXT_GRAPH_ID: AtomicU32 = AtomicU32::new(0); /// let start_b = graph.root(sys_b.system()); /// let start_c = graph.root(sys_c.system()); /// -/// vec![start_a, start_b, start_c] -/// .join_into(sys_d.system()) +/// (start_a, start_b, start_c) +/// .join(sys_d.system()) /// .then(sys_e.system()); /// /// // Convert into a SystemSet /// let system_set: SystemSet = graph.into(); /// ``` /// +/// # Fan In +/// The types used to implement [fork] and [join] are composable. +/// ``` +/// # use bevy_ecs::prelude::*; +/// # fn sys_a() {} +/// # fn sys_b() {} +/// # fn sys_c() {} +/// # fn sys_d() {} +/// # fn sys_e() {} +/// # fn sys_f() {} +/// let graph = SystemGraph::new(); +/// graph.root(sys_a.system()) +/// .fork((sys_b.system(), sys_c.system(), sys_d.system())) +/// .join(sys_e.system()) +/// .then(sys_f.system()); +/// +/// // Convert into a SystemSet +/// let system_set: SystemSet = graph.into(); +/// ``` +/// /// # Cloning /// This type is backed by a Rc, so cloning it will still point to the same logical /// underlying graph. +/// +/// [fork]: crate::schedule::SystemGraphNode::join +/// [join]: crate::schedule::SystemJoin::join #[derive(Clone)] pub struct SystemGraph { id: u32, @@ -169,6 +191,10 @@ pub struct SystemGraphNode { } impl SystemGraphNode { + pub fn graph(&self) -> SystemGraph { + self.graph.clone() + } + /// Creates a new node in the graph and adds the current node as it's dependency. /// /// This function can be called multiple times to add mulitple systems to the graph, @@ -183,69 +209,86 @@ impl SystemGraphNode { /// systems will not run until the original node's system finishes running. /// /// Functionally equivalent to calling `SystemGraphNode::then` multiple times. - pub fn split_into(&self, system_group: T) -> T::Output { - system_group.split_into(self) + pub fn fork(&self, system_group: T) -> T::Output { + system_group.fork_from(self) } } pub trait SystemGroup { type Output; - fn split_into(self, src: &SystemGraphNode) -> Self::Output; + fn fork_from(self, src: &SystemGraphNode) -> Self::Output; +} + +pub trait SystemJoin { + fn join(self, next: impl Into) -> SystemGraphNode; } impl> SystemGroup for Vec { type Output = Vec; - fn split_into(self, src: &SystemGraphNode) -> Self::Output { + fn fork_from(self, src: &SystemGraphNode) -> Self::Output { self.into_iter().map(|sys| src.then(sys.into())).collect() } } +impl SystemJoin for Vec { + fn join(self, next: impl Into) -> SystemGraphNode { + let mut nodes = self.into_iter().peekable(); + let output = nodes + .peek() + .map(|node| node.graph.create_node(next.into())) + .expect("Attempted to join a collection of zero nodes."); + + for node in nodes { + assert!( + output.graph.is_same_graph(&node.graph), + "Joined graph nodes should be from the same graph." + ); + output.graph.add_dependency(node.id, output.id); + } + + output + } +} + +// HACK: using repeat macros without using the param in it fails to compile. The ignore_first +// here "uses" the parameter by discarding it. macro_rules! ignore_first { ($_first:ident, $second:ty) => { $second }; } -macro_rules! impl_system_group_tuple { +macro_rules! impl_system_tuple { ($($param: ident),*) => { impl<$($param: Into),*> SystemGroup for ($($param,)*) { - // HACK: using repeat macros without using the param in it fails to compile. - // The ignore_first here "uses" the parameter and just discards it. type Output = ($(ignore_first!($param, SystemGraphNode),)*); #[inline] #[allow(non_snake_case)] - fn split_into(self, src: &SystemGraphNode) -> Self::Output { + fn fork_from(self, src: &SystemGraphNode) -> Self::Output { let ($($param,)*) = self; ($(src.then($param::into($param)),)*) } } - }; -} - -all_tuples!(impl_system_group_tuple, 2, 16, T); - -pub trait SystemGraphJoinExt: Sized + IntoIterator { - fn join_into(self, next: impl Into) -> SystemGraphNode { - let mut nodes = self.into_iter().peekable(); - let output = nodes - .peek() - .map(|node| node.graph.create_node(next.into())) - .expect("Attempted to join a collection of zero nodes."); - for node in nodes { - assert!( - output.graph.is_same_graph(&node.graph), - "Joined graph nodes should be from the same graph." - ); - output.graph.add_dependency(node.id, output.id); + impl SystemJoin for ($(ignore_first!($param, SystemGraphNode),)*) { + #[inline] + #[allow(non_snake_case)] + fn join(self, next: impl Into) -> SystemGraphNode { + let output = self.0.graph.create_node(next.into()); + let ($($param,)*) = self; + $( + assert!(output.graph.is_same_graph(&$param.graph), + "Joined graph nodes should be from the same graph."); + output.graph.add_dependency($param.id, output.id); + )* + output + } } - - output - } + }; } -impl> SystemGraphJoinExt for T {} +all_tuples!(impl_system_tuple, 2, 16, T); #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] struct NodeId { @@ -282,7 +325,7 @@ mod test { .then(dummy_system.system()) .then(dummy_system.system()); vec![a, b, c] - .join_into(dummy_system.system()) + .join(dummy_system.system()) .then(dummy_system.system()); let system_set: SystemSet = graph.into(); let (_, systems) = system_set.bake(); @@ -306,7 +349,7 @@ mod test { .then(dummy_system.system()) .then(dummy_system.system()); vec![a, b, c] - .join_into(dummy_system.system()) + .join(dummy_system.system()) .then(dummy_system.system()); let system_set: SystemSet = graph.into(); let (_, systems) = system_set.bake(); From 9f3eee1a642c84e320357022045bab8ad3be4a86 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 26 Jun 2021 21:08:42 -0700 Subject: [PATCH 13/23] Use IntoSystemDescriptor instead of Into --- crates/bevy_ecs/src/schedule/system_graph.rs | 119 +++++++++---------- 1 file changed, 59 insertions(+), 60 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs index a336ea9ce7756..5386c0302103c 100644 --- a/crates/bevy_ecs/src/schedule/system_graph.rs +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -1,4 +1,4 @@ -use crate::schedule::{SystemDescriptor, SystemLabel, SystemSet}; +use crate::schedule::{IntoSystemDescriptor, SystemDescriptor, SystemLabel, SystemSet}; use bevy_ecs_macros::all_tuples; use bevy_utils::HashMap; use std::{ @@ -21,9 +21,9 @@ static NEXT_GRAPH_ID: AtomicU32 = AtomicU32::new(0); /// # fn sys_c() {} /// let graph = SystemGraph::new(); /// graph -/// .root(sys_a.system()) -/// .then(sys_b.system()) -/// .then(sys_c.system()); +/// .root(sys_a) +/// .then(sys_b) +/// .then(sys_c); /// /// // Convert into a SystemSet /// let system_set: SystemSet = graph.into(); @@ -45,14 +45,14 @@ static NEXT_GRAPH_ID: AtomicU32 = AtomicU32::new(0); /// // Fork out from one original node. /// let (c, b, d) = graph.root(sys_a.system()) /// .fork(( -/// sys_b.system(), -/// sys_c.system(), -/// sys_d.system(), +/// sys_b, +/// sys_c, +/// sys_d, /// )); /// /// // Alternatively, calling "then" repeatedly achieves the same thing. -/// let e = d.then(sys_e.system()); -/// let f = d.then(sys_f.system()); +/// let e = d.then(sys_e); +/// let f = d.then(sys_f); /// /// // Convert into a SystemSet /// let system_set: SystemSet = graph.into(); @@ -70,13 +70,13 @@ static NEXT_GRAPH_ID: AtomicU32 = AtomicU32::new(0); /// # fn sys_e() {} /// let graph = SystemGraph::new(); /// -/// let start_a = graph.root(sys_a.system()); -/// let start_b = graph.root(sys_b.system()); -/// let start_c = graph.root(sys_c.system()); +/// let start_a = graph.root(sys_a); +/// let start_b = graph.root(sys_b); +/// let start_c = graph.root(sys_c); /// /// (start_a, start_b, start_c) -/// .join(sys_d.system()) -/// .then(sys_e.system()); +/// .join(sys_d) +/// .then(sys_e); /// /// // Convert into a SystemSet /// let system_set: SystemSet = graph.into(); @@ -93,10 +93,10 @@ static NEXT_GRAPH_ID: AtomicU32 = AtomicU32::new(0); /// # fn sys_e() {} /// # fn sys_f() {} /// let graph = SystemGraph::new(); -/// graph.root(sys_a.system()) -/// .fork((sys_b.system(), sys_c.system(), sys_d.system())) -/// .join(sys_e.system()) -/// .then(sys_f.system()); +/// graph.root(sys_a) +/// .fork((sys_b, sys_c, sys_d)) +/// .join(sys_e) +/// .then(sys_f); /// /// // Convert into a SystemSet /// let system_set: SystemSet = graph.into(); @@ -124,8 +124,8 @@ impl SystemGraph { /// Creates a root graph node without any dependencies. A graph can have multiple distinct /// root nodes. - pub fn root(&self, system: impl Into) -> SystemGraphNode { - self.create_node(system.into()) + pub fn root(&self, system: impl IntoSystemDescriptor) -> SystemGraphNode { + self.create_node(system.into_descriptor()) } /// Checks if two graphs instances point to the same logical underlying graph. @@ -178,7 +178,14 @@ impl From for SystemSet { fn from(graph: SystemGraph) -> Self { let mut system_set = SystemSet::new(); for (_, system) in graph.nodes.borrow_mut().drain() { - system_set = system_set.with_system(system); + match system { + SystemDescriptor::Parallel(descriptor) => { + system_set = system_set.with_system(descriptor); + } + SystemDescriptor::Exclusive(descriptor) => { + system_set = system_set.with_system(descriptor); + } + } } system_set } @@ -199,8 +206,8 @@ impl SystemGraphNode { /// /// This function can be called multiple times to add mulitple systems to the graph, /// all of which will not execute until original node's system has finished running. - pub fn then(&self, next: impl Into) -> SystemGraphNode { - let node = self.graph.create_node(next.into()); + pub fn then(&self, next: impl IntoSystemDescriptor) -> SystemGraphNode { + let node = self.graph.create_node(next.into_descriptor()); self.graph.add_dependency(self.id, node.id); node } @@ -209,33 +216,33 @@ impl SystemGraphNode { /// systems will not run until the original node's system finishes running. /// /// Functionally equivalent to calling `SystemGraphNode::then` multiple times. - pub fn fork(&self, system_group: T) -> T::Output { + pub fn fork>(&self, system_group: T) -> T::Output { system_group.fork_from(self) } } -pub trait SystemGroup { +pub trait SystemGroup { type Output; fn fork_from(self, src: &SystemGraphNode) -> Self::Output; } pub trait SystemJoin { - fn join(self, next: impl Into) -> SystemGraphNode; + fn join(self, next: impl IntoSystemDescriptor) -> SystemGraphNode; } -impl> SystemGroup for Vec { +impl> SystemGroup for Vec { type Output = Vec; fn fork_from(self, src: &SystemGraphNode) -> Self::Output { - self.into_iter().map(|sys| src.then(sys.into())).collect() + self.into_iter().map(|sys| src.then(sys)).collect() } } impl SystemJoin for Vec { - fn join(self, next: impl Into) -> SystemGraphNode { + fn join(self, next: impl IntoSystemDescriptor) -> SystemGraphNode { let mut nodes = self.into_iter().peekable(); let output = nodes .peek() - .map(|node| node.graph.create_node(next.into())) + .map(|node| node.graph.create_node(next.into_descriptor())) .expect("Attempted to join a collection of zero nodes."); for node in nodes { @@ -260,22 +267,22 @@ macro_rules! ignore_first { macro_rules! impl_system_tuple { ($($param: ident),*) => { - impl<$($param: Into),*> SystemGroup for ($($param,)*) { + impl),*> SystemGroup for ($($param,)*) { type Output = ($(ignore_first!($param, SystemGraphNode),)*); #[inline] #[allow(non_snake_case)] fn fork_from(self, src: &SystemGraphNode) -> Self::Output { let ($($param,)*) = self; - ($(src.then($param::into($param)),)*) + ($(src.then($param),)*) } } impl SystemJoin for ($(ignore_first!($param, SystemGraphNode),)*) { #[inline] #[allow(non_snake_case)] - fn join(self, next: impl Into) -> SystemGraphNode { - let output = self.0.graph.create_node(next.into()); + fn join(self, next: impl IntoSystemDescriptor) -> SystemGraphNode { + let output = self.0.graph.create_node(next.into_descriptor()); let ($($param,)*) = self; $( assert!(output.graph.is_same_graph(&$param.graph), @@ -313,20 +320,16 @@ mod test { pub fn graph_creates_accurate_system_counts() { let graph = SystemGraph::new(); let a = graph - .root(dummy_system.system()) - .then(dummy_system.system()) - .then(dummy_system.system()) - .then(dummy_system.system()); - let b = graph - .root(dummy_system.system()) - .then(dummy_system.system()); + .root(dummy_system) + .then(dummy_system) + .then(dummy_system) + .then(dummy_system); + let b = graph.root(dummy_system).then(dummy_system); let c = graph - .root(dummy_system.system()) - .then(dummy_system.system()) - .then(dummy_system.system()); - vec![a, b, c] - .join(dummy_system.system()) - .then(dummy_system.system()); + .root(dummy_system) + .then(dummy_system) + .then(dummy_system); + vec![a, b, c].join(dummy_system).then(dummy_system); let system_set: SystemSet = graph.into(); let (_, systems) = system_set.bake(); @@ -337,20 +340,16 @@ mod test { pub fn all_nodes_are_labeled() { let graph = SystemGraph::new(); let a = graph - .root(dummy_system.system()) - .then(dummy_system.system()) - .then(dummy_system.system()) - .then(dummy_system.system()); - let b = graph - .root(dummy_system.system()) - .then(dummy_system.system()); + .root(dummy_system) + .then(dummy_system) + .then(dummy_system) + .then(dummy_system); + let b = graph.root(dummy_system).then(dummy_system); let c = graph - .root(dummy_system.system()) - .then(dummy_system.system()) - .then(dummy_system.system()); - vec![a, b, c] - .join(dummy_system.system()) - .then(dummy_system.system()); + .root(dummy_system) + .then(dummy_system) + .then(dummy_system); + vec![a, b, c].join(dummy_system).then(dummy_system); let system_set: SystemSet = graph.into(); let (_, systems) = system_set.bake(); From d4043cdba6bce476ef0f37521365676551fd312a Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 26 Jun 2021 21:18:27 -0700 Subject: [PATCH 14/23] Use impl Into instead of SystemSet to simply addition --- crates/bevy_ecs/src/schedule/mod.rs | 2 +- crates/bevy_ecs/src/schedule/stage.rs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 7b1b27873bbd9..f74c088246f9d 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -247,7 +247,7 @@ impl Schedule { pub fn add_system_set_to_stage( &mut self, stage_label: impl StageLabel, - system_set: SystemSet, + system_set: impl Into, ) -> &mut Self { self.stage(stage_label, |stage: &mut SystemStage| { stage.add_system_set(system_set) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 117fc2c6c81b4..bd08c2a2a8d20 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -251,12 +251,13 @@ impl SystemStage { &self.exclusive_before_commands } - pub fn with_system_set(mut self, system_set: SystemSet) -> Self { + pub fn with_system_set(mut self, system_set: impl Into) -> Self { self.add_system_set(system_set); self } - pub fn add_system_set(&mut self, system_set: SystemSet) -> &mut Self { + pub fn add_system_set(&mut self, system_set: impl Into) -> &mut Self { + let system_set = system_set.into(); self.systems_modified = true; let (run_criteria, mut systems) = system_set.bake(); let set_run_criteria_index = run_criteria.and_then(|criteria| { From 02b8a3a447c227be98917b37c3f48bd386293108 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 26 Jun 2021 22:10:41 -0700 Subject: [PATCH 15/23] Add SystemGraph example --- Cargo.toml | 4 +++ examples/ecs/system_graph.rs | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 examples/ecs/system_graph.rs diff --git a/Cargo.toml b/Cargo.toml index 858b58ac9e7b0..4e8fe274994b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -320,6 +320,10 @@ path = "examples/ecs/state.rs" name = "system_chaining" path = "examples/ecs/system_chaining.rs" +[[example]] +name = "system_graph" +path = "examples/ecs/system_graph.rs" + [[example]] name = "system_param" path = "examples/ecs/system_param.rs" diff --git a/examples/ecs/system_graph.rs b/examples/ecs/system_graph.rs new file mode 100644 index 0000000000000..e6d336afaf443 --- /dev/null +++ b/examples/ecs/system_graph.rs @@ -0,0 +1,50 @@ +use bevy::prelude::*; + +fn main() { + // SystemGraphs can be used to specify explicit system dependencies. + // Labels can be used alongside SystemGraphs to help specify dependency relationships. + + // These three systems will run sequentially one after another. + let sequential = SystemGraph::new(); + sequential + .root(print_system("Sequential 1")) + .then(print_system("Sequential 2")) + .then( + print_system("Sequential 3") + .system() + .label("Sequential End"), + ); + + // Graphs nodes can fork into multiple dependencies. + let wide = SystemGraph::new(); + let (mid_1, mid_2, mid_3) = wide + .root(print_system("Wide Start").system().after("Sequential End")) + .fork(( + print_system("Wide 1"), + print_system("Wide 2"), + print_system("Wide 3"), + )); + + // Graphs can have multiple root systems. + let side = wide.root(print_system("Wide Side")); + + // Branches can be continued separately from each other. + mid_3.then(print_system("Wide 3 Continuation")); + + // Multiple branches can be merged. This system will only run when all dependencies + // finish running. + (mid_1, mid_2, side).join(print_system("Wide 1 & Wide 2 Continuation")); + + // SystemGraph implements Into and can be used to add of the graph's systems to an + // App. + App::build() + .add_system_set(sequential) + .add_system_set(wide) + .run(); +} + +fn print_system(message: &'static str) -> impl Fn() { + move || { + println!("{}", message); + } +} From f8727a7a9b8a937cfb9e1cbea4e3574329708692 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 26 Jun 2021 22:26:14 -0700 Subject: [PATCH 16/23] Use a more descriptive comment in SystemGraph example --- examples/ecs/system_graph.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/examples/ecs/system_graph.rs b/examples/ecs/system_graph.rs index e6d336afaf443..17669126cd161 100644 --- a/examples/ecs/system_graph.rs +++ b/examples/ecs/system_graph.rs @@ -1,8 +1,11 @@ use bevy::prelude::*; fn main() { - // SystemGraphs can be used to specify explicit system dependencies. - // Labels can be used alongside SystemGraphs to help specify dependency relationships. + // SystemGraphs can be used to specify explicit system dependencies without specifying + // explicit labels for each system. + // + // Labels can be used alongside SystemGraphs to help specify dependency relationships + // between graphs, betwen standalone systems, or between crate boundaries. // These three systems will run sequentially one after another. let sequential = SystemGraph::new(); From a9c0cc94e64a11a7669701e6e58e076338f40c03 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 27 Jun 2021 06:53:08 -0700 Subject: [PATCH 17/23] Fixes for CI and review comments --- crates/bevy_ecs/macros/src/lib.rs | 2 +- crates/bevy_ecs/src/schedule/system_graph.rs | 14 ++++++++++---- examples/README.md | 1 + 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 7b577bd59a5ba..a1273da39a3ee 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -47,7 +47,7 @@ impl Parse for AllTuples { #[proc_macro] pub fn all_tuples(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as AllTuples); - let len = (0..=input.end).count(); + let len = input.end + 1; let mut ident_tuples = Vec::with_capacity(len); for i in 0..=input.end { let idents = input diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs index 5386c0302103c..c2698f11d5aa1 100644 --- a/crates/bevy_ecs/src/schedule/system_graph.rs +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -82,7 +82,7 @@ static NEXT_GRAPH_ID: AtomicU32 = AtomicU32::new(0); /// let system_set: SystemSet = graph.into(); /// ``` /// -/// # Fan In +/// # Fan Out into Fan In /// The types used to implement [fork] and [join] are composable. /// ``` /// # use bevy_ecs::prelude::*; @@ -106,7 +106,7 @@ static NEXT_GRAPH_ID: AtomicU32 = AtomicU32::new(0); /// This type is backed by a Rc, so cloning it will still point to the same logical /// underlying graph. /// -/// [fork]: crate::schedule::SystemGraphNode::join +/// [fork]: crate::schedule::SystemGraphNode::fork /// [join]: crate::schedule::SystemJoin::join #[derive(Clone)] pub struct SystemGraph { @@ -114,13 +114,19 @@ pub struct SystemGraph { nodes: Rc>>, } -impl SystemGraph { - pub fn new() -> Self { +impl Default for SystemGraph { + fn default() -> Self { Self { id: NEXT_GRAPH_ID.fetch_add(1, Ordering::Relaxed), nodes: Default::default(), } } +} + +impl SystemGraph { + pub fn new() -> Self { + Self::default() + } /// Creates a root graph node without any dependencies. A graph can have multiple distinct /// root nodes. diff --git a/examples/README.md b/examples/README.md index ca7bb98fe844f..c3ae7aeb3f81a 100644 --- a/examples/README.md +++ b/examples/README.md @@ -167,6 +167,7 @@ Example | File | Description `removal_detection` | [`ecs/removal_detection.rs`](./ecs/removal_detection.rs) | Query for entities that had a specific component removed in a previous stage during the current frame. `startup_system` | [`ecs/startup_system.rs`](./ecs/startup_system.rs) | Demonstrates a startup system (one that runs once when the app starts up) `state` | [`ecs/state.rs`](./ecs/state.rs) | Illustrates how to use States to control transitioning from a Menu state to an InGame state +`system_graph` | [`ecs/system_graph.rs`](./ecs/system_graph.rs) | Constructing complex dependency graphs between multiple systems without labels `system_chaining` | [`ecs/system_chaining.rs`](./ecs/system_chaining.rs) | Chain two systems together, specifying a return type in a system (such as `Result`) `system_param` | [`ecs/system_param.rs`](./ecs/system_param.rs) | Illustrates creating custom system parameters with `SystemParam` `system_sets` | [`ecs/system_sets.rs`](./ecs/system_sets.rs) | Shows `SystemSet` use along with run criterion From 204ff3e9c43d059e9b1becc9c1430228578966ef Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 27 Jun 2021 07:50:42 -0700 Subject: [PATCH 18/23] Add tests for accurate labelling --- crates/bevy_ecs/src/schedule/system_graph.rs | 85 +++++++++++++++++--- 1 file changed, 76 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs index c2698f11d5aa1..f431fb1a66ab8 100644 --- a/crates/bevy_ecs/src/schedule/system_graph.rs +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -128,6 +128,14 @@ impl SystemGraph { Self::default() } + #[cfg(test)] + pub(crate) fn with_id(id: u32) -> Self { + Self { + id, + nodes: Default::default(), + } + } + /// Creates a root graph node without any dependencies. A graph can have multiple distinct /// root nodes. pub fn root(&self, system: impl IntoSystemDescriptor) -> SystemGraphNode { @@ -146,10 +154,7 @@ impl SystemGraph { "Cannot add more than {} systems to a SystemGraph", u32::MAX ); - let id = NodeId { - graph_id: self.id, - node_id: nodes.len() as u32, - }; + let id = NodeId(self.id, nodes.len() as u32); match &mut system { SystemDescriptor::Parallel(descriptor) => descriptor.labels.push(id.dyn_clone()), SystemDescriptor::Exclusive(descriptor) => descriptor.labels.push(id.dyn_clone()), @@ -304,10 +309,7 @@ macro_rules! impl_system_tuple { all_tuples!(impl_system_tuple, 2, 16, T); #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -struct NodeId { - graph_id: u32, - node_id: u32, -} +struct NodeId(u32, u32); impl SystemLabel for NodeId { fn dyn_clone(&self) -> Box { @@ -317,11 +319,76 @@ impl SystemLabel for NodeId { #[cfg(test)] mod test { - use crate::prelude::*; + use super::*; use crate::schedule::SystemDescriptor; fn dummy_system() {} + fn assert_eq_after(sys: &SystemDescriptor, expected: Vec) { + let deps = match sys { + SystemDescriptor::Parallel(desc) => &desc.after, + SystemDescriptor::Exclusive(desc) => &desc.after, + }; + let after: Vec> = + expected.into_iter().map(|id| id.dyn_clone()).collect(); + assert_eq!(deps, &after); + } + + #[test] + pub fn then_creates_accurate_dependencies() { + let graph = SystemGraph::with_id(0); + graph + .root(dummy_system) + .then(dummy_system) + .then(dummy_system) + .then(dummy_system); + + let systems = graph.nodes.borrow(); + + assert_eq!(systems.len(), 4); + assert_eq_after(&systems[&NodeId(0, 0)], vec![]); + assert_eq_after(&systems[&NodeId(0, 1)], vec![NodeId(0, 0)]); + assert_eq_after(&systems[&NodeId(0, 2)], vec![NodeId(0, 1)]); + assert_eq_after(&systems[&NodeId(0, 3)], vec![NodeId(0, 2)]); + } + + #[test] + pub fn fork_creates_accurate_dependencies() { + let graph = SystemGraph::with_id(0); + graph + .root(dummy_system) + .fork((dummy_system, dummy_system, dummy_system)); + + let systems = graph.nodes.borrow(); + + assert_eq!(systems.len(), 4); + assert_eq_after(&systems[&NodeId(0, 0)], vec![]); + assert_eq_after(&systems[&NodeId(0, 1)], vec![NodeId(0, 0)]); + assert_eq_after(&systems[&NodeId(0, 2)], vec![NodeId(0, 0)]); + assert_eq_after(&systems[&NodeId(0, 3)], vec![NodeId(0, 0)]); + } + + #[test] + pub fn join_creates_accurate_dependencies() { + let graph = SystemGraph::with_id(0); + let a = graph.root(dummy_system); + let b = graph.root(dummy_system); + let c = graph.root(dummy_system); + + (a, b, c).join(dummy_system); + + let systems = graph.nodes.borrow(); + + assert_eq!(systems.len(), 4); + assert_eq_after(&systems[&NodeId(0, 0)], vec![]); + assert_eq_after(&systems[&NodeId(0, 1)], vec![]); + assert_eq_after(&systems[&NodeId(0, 2)], vec![]); + assert_eq_after( + &systems[&NodeId(0, 3)], + vec![NodeId(0, 0), NodeId(0, 1), NodeId(0, 2)], + ); + } + #[test] pub fn graph_creates_accurate_system_counts() { let graph = SystemGraph::new(); From bc58b6befda230b1b16105b9ae5984cfd28607d0 Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 2 Jul 2021 14:32:09 -0700 Subject: [PATCH 19/23] Systemyeet --- crates/bevy_ecs/src/schedule/system_graph.rs | 6 +++--- examples/ecs/system_graph.rs | 8 ++------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs index f431fb1a66ab8..af3b3cc344abb 100644 --- a/crates/bevy_ecs/src/schedule/system_graph.rs +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -43,7 +43,7 @@ static NEXT_GRAPH_ID: AtomicU32 = AtomicU32::new(0); /// let graph = SystemGraph::new(); /// /// // Fork out from one original node. -/// let (c, b, d) = graph.root(sys_a.system()) +/// let (c, b, d) = graph.root(sys_a) /// .fork(( /// sys_b, /// sys_c, @@ -213,7 +213,7 @@ impl SystemGraphNode { self.graph.clone() } - /// Creates a new node in the graph and adds the current node as it's dependency. + /// Creates a new node in the graph and adds the current node as its dependency. /// /// This function can be called multiple times to add mulitple systems to the graph, /// all of which will not execute until original node's system has finished running. @@ -297,7 +297,7 @@ macro_rules! impl_system_tuple { let ($($param,)*) = self; $( assert!(output.graph.is_same_graph(&$param.graph), - "Joined graph nodes should be from the same graph."); + "Joined graph nodes must be from the same graph."); output.graph.add_dependency($param.id, output.id); )* output diff --git a/examples/ecs/system_graph.rs b/examples/ecs/system_graph.rs index 17669126cd161..1d90140868b9a 100644 --- a/examples/ecs/system_graph.rs +++ b/examples/ecs/system_graph.rs @@ -12,16 +12,12 @@ fn main() { sequential .root(print_system("Sequential 1")) .then(print_system("Sequential 2")) - .then( - print_system("Sequential 3") - .system() - .label("Sequential End"), - ); + .then(print_system("Sequential 3").label("Sequential End")); // Graphs nodes can fork into multiple dependencies. let wide = SystemGraph::new(); let (mid_1, mid_2, mid_3) = wide - .root(print_system("Wide Start").system().after("Sequential End")) + .root(print_system("Wide Start").after("Sequential End")) .fork(( print_system("Wide 1"), print_system("Wide 2"), From 3a7ebc9c7a6ec8f97bfedc37ebbde96db350505a Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 2 Jul 2021 16:01:11 -0700 Subject: [PATCH 20/23] Add par_join for easier explicit many-to-many dependencies --- crates/bevy_ecs/src/schedule/system_graph.rs | 34 +++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs index af3b3cc344abb..35637585de8e8 100644 --- a/crates/bevy_ecs/src/schedule/system_graph.rs +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -209,6 +209,7 @@ pub struct SystemGraphNode { } impl SystemGraphNode { + #[inline] pub fn graph(&self) -> SystemGraph { self.graph.clone() } @@ -227,6 +228,7 @@ impl SystemGraphNode { /// systems will not run until the original node's system finishes running. /// /// Functionally equivalent to calling `SystemGraphNode::then` multiple times. + #[inline] pub fn fork>(&self, system_group: T) -> T::Output { system_group.fork_from(self) } @@ -235,10 +237,21 @@ impl SystemGraphNode { pub trait SystemGroup { type Output; fn fork_from(self, src: &SystemGraphNode) -> Self::Output; + fn join_from(self, src: &J) -> Self::Output; } -pub trait SystemJoin { - fn join(self, next: impl IntoSystemDescriptor) -> SystemGraphNode; +pub trait SystemJoin: Sized { + /// Adds a system to the graph dependent on all of the nodes contained within the join. + fn join(&self, next: impl IntoSystemDescriptor) -> SystemGraphNode; + + /// Adds a `SystemGroup` to the graph that is dependent on all of nodes contained within + /// the join. + /// + /// Functionally equivalent to calling `join` on every node created from the group. + #[inline] + fn par_join>(&self, next: G) -> G::Output { + next.join_from(self) + } } impl> SystemGroup for Vec { @@ -246,11 +259,15 @@ impl> SystemGroup for Vec { fn fork_from(self, src: &SystemGraphNode) -> Self::Output { self.into_iter().map(|sys| src.then(sys)).collect() } + + fn join_from(self, src: &J) -> Self::Output { + self.into_iter().map(|sys| src.join(sys)).collect() + } } impl SystemJoin for Vec { - fn join(self, next: impl IntoSystemDescriptor) -> SystemGraphNode { - let mut nodes = self.into_iter().peekable(); + fn join(&self, next: impl IntoSystemDescriptor) -> SystemGraphNode { + let mut nodes = self.iter().peekable(); let output = nodes .peek() .map(|node| node.graph.create_node(next.into_descriptor())) @@ -287,12 +304,19 @@ macro_rules! impl_system_tuple { let ($($param,)*) = self; ($(src.then($param),)*) } + + #[inline] + #[allow(non_snake_case)] + fn join_from(self, src: &J) -> Self::Output { + let ($($param,)*) = self; + ($(src.join($param),)*) + } } impl SystemJoin for ($(ignore_first!($param, SystemGraphNode),)*) { #[inline] #[allow(non_snake_case)] - fn join(self, next: impl IntoSystemDescriptor) -> SystemGraphNode { + fn join(&self, next: impl IntoSystemDescriptor) -> SystemGraphNode { let output = self.0.graph.create_node(next.into_descriptor()); let ($($param,)*) = self; $( From bafe1cbfb2363427d441ec068d802b3d307588b6 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 3 Jul 2021 18:31:11 -0700 Subject: [PATCH 21/23] Review fixes: Doc comments and naming --- crates/bevy_ecs/src/schedule/system_graph.rs | 23 +++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/system_graph.rs b/crates/bevy_ecs/src/schedule/system_graph.rs index 35637585de8e8..f70f5fa8693d7 100644 --- a/crates/bevy_ecs/src/schedule/system_graph.rs +++ b/crates/bevy_ecs/src/schedule/system_graph.rs @@ -166,16 +166,18 @@ impl SystemGraph { } } - fn add_dependency(&self, src: NodeId, dst: NodeId) { - if let Some(system) = self.nodes.borrow_mut().get_mut(&dst) { + fn add_dependency(&self, origin: NodeId, dependent: NodeId) { + if let Some(system) = self.nodes.borrow_mut().get_mut(&dependent) { match system { - SystemDescriptor::Parallel(descriptor) => descriptor.after.push(src.dyn_clone()), - SystemDescriptor::Exclusive(descriptor) => descriptor.after.push(src.dyn_clone()), + SystemDescriptor::Parallel(descriptor) => descriptor.after.push(origin.dyn_clone()), + SystemDescriptor::Exclusive(descriptor) => { + descriptor.after.push(origin.dyn_clone()) + } } } else { panic!( "Attempted to add dependency for {:?}, which doesn't exist.", - dst + dependent ); } } @@ -202,6 +204,7 @@ impl From for SystemSet { } } +/// A single `SystemGraph` node that represents a system within the group. #[derive(Clone)] pub struct SystemGraphNode { id: NodeId, @@ -209,6 +212,10 @@ pub struct SystemGraphNode { } impl SystemGraphNode { + /// Gets the underlying `SystemGraph` that the node belongs to. + /// + /// `SystemGraph` is internally ref counted, so the returned value will always point to the + /// same graph even if the node itself is dropped. #[inline] pub fn graph(&self) -> SystemGraph { self.graph.clone() @@ -234,12 +241,16 @@ impl SystemGraphNode { } } +/// Represents a collection of systems. Used for grouping systems together for making +/// `SystemGraph`s. pub trait SystemGroup { type Output; fn fork_from(self, src: &SystemGraphNode) -> Self::Output; fn join_from(self, src: &J) -> Self::Output; } +/// A collection of `SystemGraphNode`s that can be joined together into one or more dependent +/// systems. pub trait SystemJoin: Sized { /// Adds a system to the graph dependent on all of the nodes contained within the join. fn join(&self, next: impl IntoSystemDescriptor) -> SystemGraphNode; @@ -249,7 +260,7 @@ pub trait SystemJoin: Sized { /// /// Functionally equivalent to calling `join` on every node created from the group. #[inline] - fn par_join>(&self, next: G) -> G::Output { + fn join_all>(&self, next: G) -> G::Output { next.join_from(self) } } From 066bceb9bdb86499704023477133cd58d543911a Mon Sep 17 00:00:00 2001 From: James Liu Date: Thu, 23 Dec 2021 12:18:18 -0500 Subject: [PATCH 22/23] Fix build/tests --- crates/bevy_app/src/app.rs | 6 +++--- crates/bevy_ecs/src/schedule/system_set.rs | 2 -- examples/ecs/system_graph.rs | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index a14baf2ae225f..d6adb34b3609b 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -331,7 +331,7 @@ impl App { /// .with_system(system_c), /// ); /// ``` - pub fn add_system_set(&mut self, system_set: SystemSet) -> &mut Self { + pub fn add_system_set(&mut self, system_set: impl Into) -> &mut Self { self.add_system_set_to_stage(CoreStage::Update, system_set) } @@ -381,7 +381,7 @@ impl App { pub fn add_system_set_to_stage( &mut self, stage_label: impl StageLabel, - system_set: SystemSet, + system_set: impl Into, ) -> &mut Self { self.schedule .add_system_set_to_stage(stage_label, system_set); @@ -491,7 +491,7 @@ impl App { pub fn add_startup_system_set_to_stage( &mut self, stage_label: impl StageLabel, - system_set: SystemSet, + system_set: impl Into, ) -> &mut Self { self.schedule .stage(CoreStage::Startup, |schedule: &mut Schedule| { diff --git a/crates/bevy_ecs/src/schedule/system_set.rs b/crates/bevy_ecs/src/schedule/system_set.rs index 09214fba91122..20dcb50d6ff4a 100644 --- a/crates/bevy_ecs/src/schedule/system_set.rs +++ b/crates/bevy_ecs/src/schedule/system_set.rs @@ -10,8 +10,6 @@ use std::{ sync::atomic::{AtomicU32, Ordering}, }; -static NEXT_SEQUENCE_ID: AtomicU32 = AtomicU32::new(0); - /// A builder for describing several systems at the same time. #[derive(Default)] pub struct SystemSet { diff --git a/examples/ecs/system_graph.rs b/examples/ecs/system_graph.rs index 1d90140868b9a..028d76727cacd 100644 --- a/examples/ecs/system_graph.rs +++ b/examples/ecs/system_graph.rs @@ -36,7 +36,7 @@ fn main() { // SystemGraph implements Into and can be used to add of the graph's systems to an // App. - App::build() + App::new() .add_system_set(sequential) .add_system_set(wide) .run(); From 913547c49dfe4d1666410e9f36ced3aa468d2d66 Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 23 Dec 2021 12:33:22 -0500 Subject: [PATCH 23/23] Remove unused use statements --- crates/bevy_ecs/src/schedule/system_set.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/system_set.rs b/crates/bevy_ecs/src/schedule/system_set.rs index 20dcb50d6ff4a..0702445c79fd0 100644 --- a/crates/bevy_ecs/src/schedule/system_set.rs +++ b/crates/bevy_ecs/src/schedule/system_set.rs @@ -4,11 +4,6 @@ use crate::schedule::{ }; use super::IntoSystemDescriptor; -use std::{ - fmt::Debug, - hash::Hash, - sync::atomic::{AtomicU32, Ordering}, -}; /// A builder for describing several systems at the same time. #[derive(Default)]