Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand Down
15 changes: 7 additions & 8 deletions crates/bevy_ecs/src/schedule/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};
Expand Down Expand Up @@ -164,7 +163,7 @@ impl<T: Schedulable<Metadata = GraphInfo, GroupMetadata = Chain>> 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 {
Expand All @@ -180,7 +179,7 @@ impl<T: Schedulable<Metadata = GraphInfo, GroupMetadata = Chain>> 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 {
Expand All @@ -196,7 +195,7 @@ impl<T: Schedulable<Metadata = GraphInfo, GroupMetadata = Chain>> 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 {
Expand All @@ -212,7 +211,7 @@ impl<T: Schedulable<Metadata = GraphInfo, GroupMetadata = Chain>> 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 {
Expand Down Expand Up @@ -293,7 +292,7 @@ impl<T: Schedulable<Metadata = GraphInfo, GroupMetadata = Chain>> ScheduleConfig
match &mut self {
Self::ScheduleConfig(_) => { /* no op */ }
Self::Configs { metadata, .. } => {
metadata.set_chained_with_config(IgnoreDeferred);
metadata.set_chained_ignore_deferred();
}
}
self
Expand All @@ -303,7 +302,7 @@ impl<T: Schedulable<Metadata = GraphInfo, GroupMetadata = Chain>> ScheduleConfig
match &mut self {
Self::ScheduleConfig(_) => { /* no op */ }
Self::Configs { metadata, .. } => {
metadata.set_chained_with_config(Weak);
metadata.set_chained_weak();
}
}
self
Expand Down
31 changes: 20 additions & 11 deletions crates/bevy_ecs/src/schedule/graph/mod.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -28,19 +23,33 @@ pub(crate) enum DependencyKind {
pub(crate) struct Dependency {
pub(crate) kind: DependencyKind,
pub(crate) set: InternedSystemSet,
pub(crate) options: TypeIdHashMap<Box<dyn Any>>,
pub(crate) is_weak: bool,
pub(crate) ignore_deferred: bool,
}

impl Dependency {
pub fn new(kind: DependencyKind, set: InternedSystemSet) -> Self {
Self {
kind,
set,
options: Default::default(),
is_weak: false,
ignore_deferred: false,
}
}
pub fn add_config<T: 'static>(mut self, option: T) -> Self {
self.options.insert(TypeId::of::<T>(), 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
}
}
Expand Down
33 changes: 6 additions & 27 deletions crates/bevy_ecs/src/schedule/pass.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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.
Expand Down Expand Up @@ -127,12 +119,7 @@ pub(super) trait ScheduleBuildPassObj: Send + Sync + Debug {
dependency_flattening: &DiGraph<NodeId>,
dependencies_to_add: &mut Vec<(NodeId, NodeId)>,
);
fn add_dependency(
&mut self,
from: NodeId,
to: NodeId,
all_options: &TypeIdHashMap<Box<dyn Any>>,
);
fn add_dependency(&mut self, from: NodeId, to: NodeId, is_weak: bool, ignore_deferred: bool);
}

impl<T: ScheduleBuildPass> ScheduleBuildPassObj for T {
Expand All @@ -154,15 +141,7 @@ impl<T: ScheduleBuildPass> 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<Box<dyn Any>>,
) {
let option = all_options
.get(&TypeId::of::<T::EdgeOptions>())
.and_then(|x| x.downcast_ref::<T::EdgeOptions>());
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);
}
}
109 changes: 74 additions & 35 deletions crates/bevy_ecs/src/schedule/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -296,22 +290,48 @@ pub enum Chain {
Unchained,
/// Systems are chained. `before -> after` ordering constraints
/// will be added between the successive elements.
Chained(TypeIdHashMap<Box<dyn Any>>),
Chained {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically a breaking change but eh...

/// 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<T: 'static>(&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::<T>(), 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!()
};
Expand Down Expand Up @@ -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::<Weak>())
);
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();

Expand All @@ -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 {
&current_result.nodes[..1]
Expand All @@ -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
Expand All @@ -950,7 +980,8 @@ impl ScheduleGraph {
pass.add_dependency(
*previous_node,
*current_node,
chain_options,
*is_weak,
*ignore_deferred,
);
}
}
Expand Down Expand Up @@ -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::<Weak>()) {
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
Expand Down Expand Up @@ -2938,12 +2977,12 @@ mod tests {
struct Pass<const N: usize>;

impl<const N: usize> ScheduleBuildPass for Pass<N> {
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(
Expand Down