diff --git a/crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs b/crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs index 0c45f81c27bea..6d2662ba40d04 100644 --- a/crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs +++ b/crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs @@ -61,10 +61,8 @@ impl AutoInsertApplyDeferredPass { } impl ScheduleBuildPass for AutoInsertApplyDeferredPass { - type EdgeOptions = IgnoreDeferred; - - fn add_dependency(&mut self, from: NodeId, to: NodeId, options: Option<&Self::EdgeOptions>) { - if options.is_some() { + fn add_dependency(&mut self, from: NodeId, to: NodeId, _: bool, ignore_deferred: bool) { + if ignore_deferred { self.no_sync_edges.insert((from, to)); } } diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index d153bbb11afb6..2562b2c4e0c55 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -3,11 +3,10 @@ use variadics_please::all_tuples; use crate::{ schedule::{ - auto_insert_apply_deferred::IgnoreDeferred, condition::{BoxedCondition, SystemCondition}, graph::{Ambiguity, Dependency, DependencyKind, GraphInfo}, set::{InternedSystemSet, IntoSystemSet, SystemSet}, - Chain, Weak, + Chain, }, system::{BoxedSystem, IntoSystem, ScheduleSystem, System}, }; @@ -164,7 +163,7 @@ impl> ScheduleConfig config .metadata .dependencies - .push(Dependency::new(DependencyKind::Before, set).add_config(Weak)); + .push(Dependency::new(DependencyKind::Before, set).set_weak()); } Self::Configs { configs, .. } => { for config in configs { @@ -180,7 +179,7 @@ impl> ScheduleConfig config .metadata .dependencies - .push(Dependency::new(DependencyKind::After, set).add_config(Weak)); + .push(Dependency::new(DependencyKind::After, set).set_weak()); } Self::Configs { configs, .. } => { for config in configs { @@ -196,7 +195,7 @@ impl> ScheduleConfig config .metadata .dependencies - .push(Dependency::new(DependencyKind::Before, set).add_config(IgnoreDeferred)); + .push(Dependency::new(DependencyKind::Before, set).ignore_deferred()); } Self::Configs { configs, .. } => { for config in configs { @@ -212,7 +211,7 @@ impl> ScheduleConfig config .metadata .dependencies - .push(Dependency::new(DependencyKind::After, set).add_config(IgnoreDeferred)); + .push(Dependency::new(DependencyKind::After, set).ignore_deferred()); } Self::Configs { configs, .. } => { for config in configs { @@ -293,7 +292,7 @@ impl> ScheduleConfig match &mut self { Self::ScheduleConfig(_) => { /* no op */ } Self::Configs { metadata, .. } => { - metadata.set_chained_with_config(IgnoreDeferred); + metadata.set_chained_ignore_deferred(); } } self @@ -303,7 +302,7 @@ impl> ScheduleConfig match &mut self { Self::ScheduleConfig(_) => { /* no op */ } Self::Configs { metadata, .. } => { - metadata.set_chained_with_config(Weak); + metadata.set_chained_weak(); } } self diff --git a/crates/bevy_ecs/src/schedule/graph/mod.rs b/crates/bevy_ecs/src/schedule/graph/mod.rs index 911ab6626ef66..ea11d3e8bf66c 100644 --- a/crates/bevy_ecs/src/schedule/graph/mod.rs +++ b/crates/bevy_ecs/src/schedule/graph/mod.rs @@ -1,10 +1,5 @@ -use alloc::{boxed::Box, vec::Vec}; -use core::{ - any::{Any, TypeId}, - fmt::Debug, -}; - -use bevy_utils::TypeIdHashMap; +use alloc::vec::Vec; +use core::fmt::Debug; use crate::schedule::InternedSystemSet; @@ -28,7 +23,8 @@ pub(crate) enum DependencyKind { pub(crate) struct Dependency { pub(crate) kind: DependencyKind, pub(crate) set: InternedSystemSet, - pub(crate) options: TypeIdHashMap>, + pub(crate) is_weak: bool, + pub(crate) ignore_deferred: bool, } impl Dependency { @@ -36,11 +32,24 @@ impl Dependency { Self { kind, set, - options: Default::default(), + is_weak: false, + ignore_deferred: false, } } - pub fn add_config(mut self, option: T) -> Self { - self.options.insert(TypeId::of::(), Box::new(option)); + + /// Marks the dependency as weak. + /// A weak dependency allows systems to run in parallel if they do not conflict. + pub fn set_weak(mut self) -> Self { + self.is_weak = true; + self + } + + /// Marks the dependency to ignore deferred commands between systems. + /// This tells the [`AutoInsertApplyDeferredPass`] to ignore this dependency when considering sync points. + /// + /// [`AutoInsertApplyDeferredPass`]: crate::schedule::passes::AutoInsertApplyDeferredPass + pub fn ignore_deferred(mut self) -> Self { + self.ignore_deferred = true; self } } diff --git a/crates/bevy_ecs/src/schedule/pass.rs b/crates/bevy_ecs/src/schedule/pass.rs index 7f4bd402b62ef..f13289e22240e 100644 --- a/crates/bevy_ecs/src/schedule/pass.rs +++ b/crates/bevy_ecs/src/schedule/pass.rs @@ -1,12 +1,7 @@ -use alloc::{boxed::Box, vec::Vec}; -use core::{ - any::{Any, TypeId}, - fmt::Debug, - ops::Deref, -}; +use alloc::vec::Vec; +use core::{fmt::Debug, ops::Deref}; use bevy_platform::{collections::HashSet, hash::FixedHasher}; -use bevy_utils::TypeIdHashMap; use indexmap::IndexSet; use super::{DiGraph, NodeId, ScheduleBuildError, ScheduleGraph}; @@ -20,11 +15,8 @@ use crate::{ /// A pass for modular modification of the dependency graph. pub trait ScheduleBuildPass: Send + Sync + Debug + 'static { - /// Custom options for dependencies between sets or systems. - type EdgeOptions: 'static; - /// Called when a dependency between sets or systems was explicitly added to the graph. - fn add_dependency(&mut self, from: NodeId, to: NodeId, options: Option<&Self::EdgeOptions>); + fn add_dependency(&mut self, from: NodeId, to: NodeId, is_weak: bool, is_deferred: bool); /// Called while flattening the dependency graph. For each `set`, this method is called /// with the `systems` associated with the set as well as an immutable reference to the current graph. @@ -127,12 +119,7 @@ pub(super) trait ScheduleBuildPassObj: Send + Sync + Debug { dependency_flattening: &DiGraph, dependencies_to_add: &mut Vec<(NodeId, NodeId)>, ); - fn add_dependency( - &mut self, - from: NodeId, - to: NodeId, - all_options: &TypeIdHashMap>, - ); + fn add_dependency(&mut self, from: NodeId, to: NodeId, is_weak: bool, ignore_deferred: bool); } impl ScheduleBuildPassObj for T { @@ -154,15 +141,7 @@ impl ScheduleBuildPassObj for T { let iter = self.collapse_set(set, systems, dependency_flattening); dependencies_to_add.extend(iter); } - fn add_dependency( - &mut self, - from: NodeId, - to: NodeId, - all_options: &TypeIdHashMap>, - ) { - let option = all_options - .get(&TypeId::of::()) - .and_then(|x| x.downcast_ref::()); - self.add_dependency(from, to, option); + fn add_dependency(&mut self, from: NodeId, to: NodeId, is_weak: bool, ignore_deferred: bool) { + self.add_dependency(from, to, is_weak, ignore_deferred); } } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 52195a876cf82..74ceb32755439 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -15,9 +15,9 @@ use bevy_platform::{ collections::{HashMap, HashSet}, hash::FixedHasher, }; -use bevy_utils::{default, TypeIdHashMap}; +use bevy_utils::default; use core::{ - any::{Any, TypeId}, + any::TypeId, fmt::{Debug, Write}, }; use fixedbitset::FixedBitSet; @@ -282,12 +282,6 @@ impl Schedules { } } -/// Marker stored in a [`Chain`]'s options by -/// [`chain_weak`](crate::schedule::IntoScheduleConfigs::chain_weak) to tag its edges as weak, -/// meaning the ordering is only kept between systems that actually conflict (access the same data in a way that is incompatible with the borrow checker). See `chain_weak` -/// for the semantics. -pub(crate) struct Weak; - /// Chain systems into dependencies #[derive(Default)] pub enum Chain { @@ -296,22 +290,48 @@ pub enum Chain { Unchained, /// Systems are chained. `before -> after` ordering constraints /// will be added between the successive elements. - Chained(TypeIdHashMap>), + Chained { + /// Specifies if the links between the chained systems are weak + is_weak: bool, + /// Whether or not to insert sync points between systems in this chain + ignore_deferred: bool, + }, } impl Chain { /// Specify that the systems must be chained. pub fn set_chained(&mut self) { if matches!(self, Chain::Unchained) { - *self = Self::Chained(Default::default()); + *self = Self::Chained { + is_weak: false, + ignore_deferred: false, + }; }; } - /// Specify that the systems must be chained, and add the specified configuration for - /// all dependencies created between these systems. - pub fn set_chained_with_config(&mut self, config: T) { + + /// Specify that the systems must be chained, and the links are weak + pub fn set_chained_weak(&mut self) { self.set_chained(); - if let Chain::Chained(config_map) = self { - config_map.insert(TypeId::of::(), Box::new(config)); + if let Chain::Chained { + is_weak, + ignore_deferred: _, + } = self + { + *is_weak = true; + } else { + unreachable!() + }; + } + + /// Specify that the systems must be chained, and the links ignore deferred operations + pub fn set_chained_ignore_deferred(&mut self) { + self.set_chained(); + if let Chain::Chained { + is_weak: _, + ignore_deferred, + } = self + { + *ignore_deferred = true; } else { unreachable!() }; @@ -893,16 +913,22 @@ impl ScheduleGraph { } => { self.apply_collective_conditions(&mut configs, collective_conditions); - let is_chained = matches!(metadata, Chain::Chained(_)); - let is_weak = matches!( - &metadata, - Chain::Chained(options) if options.contains_key(&TypeId::of::()) - ); + let mut is_chained = false; + let mut weak_link = false; + + if let Chain::Chained { + is_weak, + ignore_deferred: _, + } = metadata + { + weak_link = is_weak; + is_chained = true; + } // Densely chained if // * a non-weak chain whose configs are all densely chained, or // * a single densely chained config - let mut densely_chained = (is_chained && !is_weak) || configs.len() == 1; + let mut densely_chained = (is_chained && !weak_link) || configs.len() == 1; let mut configs = configs.into_iter(); let mut nodes = Vec::new(); @@ -919,7 +945,11 @@ impl ScheduleGraph { let current_result = self.process_configs(current, collect_nodes || is_chained); densely_chained &= current_result.densely_chained; - if let Chain::Chained(chain_options) = &metadata { + if let Chain::Chained { + is_weak, + ignore_deferred, + } = &metadata + { // if the current result is densely chained, we only need to chain the first node let current_nodes = if current_result.densely_chained { ¤t_result.nodes[..1] @@ -939,7 +969,7 @@ impl ScheduleGraph { for current_node in current_nodes { self.dependency.add_edge(*previous_node, *current_node); - if is_weak { + if weak_link { self.weak_node_edges.insert((*previous_node, *current_node)); } else { self.strict_node_edges @@ -950,7 +980,8 @@ impl ScheduleGraph { pass.add_dependency( *previous_node, *current_node, - chain_options, + *is_weak, + *ignore_deferred, ); } } @@ -1166,25 +1197,33 @@ impl ScheduleGraph { self.dependency.add_node(NodeId::Set(key)); } - for (kind, key, options) in - dependencies - .into_iter() - .map(|Dependency { kind, set, options }| { - (kind, self.system_sets.get_key_or_insert(set), options) - }) - { + for (kind, key, is_weak, ignore_deferred) in dependencies.into_iter().map( + |Dependency { + kind, + set, + is_weak, + ignore_deferred, + }| { + ( + kind, + self.system_sets.get_key_or_insert(set), + is_weak, + ignore_deferred, + ) + }, + ) { let (lhs, rhs) = match kind { DependencyKind::Before => (id, NodeId::Set(key)), DependencyKind::After => (NodeId::Set(key), id), }; self.dependency.add_edge(lhs, rhs); - if options.contains_key(&TypeId::of::()) { + if is_weak { self.weak_node_edges.insert((lhs, rhs)); } else { self.strict_node_edges.insert((lhs, rhs)); } for pass in self.passes.values_mut() { - pass.add_dependency(lhs, rhs, &options); + pass.add_dependency(lhs, rhs, is_weak, ignore_deferred); } // ensure set also appears in hierarchy graph @@ -2938,12 +2977,12 @@ mod tests { struct Pass; impl ScheduleBuildPass for Pass { - type EdgeOptions = (); fn add_dependency( &mut self, _from: crate::schedule::NodeId, _to: crate::schedule::NodeId, - _options: Option<&Self::EdgeOptions>, + _is_weak: bool, + _ignore_deferred: bool, ) { } fn build(