forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Use SlotMaps to store systems and system sets in Schedules (#19352)

Browse Source 
# Objective

- First step towards #279

## Solution

Makes the necessary internal data structure changes in order to allow
system removal to be added in a future PR: `Vec`s storing systems and
system sets in `ScheduleGraph` have been replaced with `SlotMap`s.

See the included migration guide for the required changes.

## Testing

Internal changes only and no new features *should* mean no new tests are
requried.
This commit is contained in:
Christian Hughes 2025-07-03 13:50:54 -05:00 committed by GitHub
parent bbf91a6964
commit ebf87f56ef
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 505 additions and 377 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Cargo.toml
View File

@ -10,7 +10,7 @@ keywords = ["game", "engine", "gamedev", "graphics", "bevy"]
license = "MIT OR Apache-2.0" license = "MIT OR Apache-2.0"
repository = "https://github.com/bevyengine/bevy" repository = "https://github.com/bevyengine/bevy"
documentation = "https://docs.rs/bevy" documentation = "https://docs.rs/bevy"
rust-version = "1.86.0" rust-version = "1.88.0"
[workspace] [workspace]
resolver = "2" resolver = "2"

1
crates/bevy_ecs/Cargo.toml
View File

@ -119,6 +119,7 @@ tracing = { version = "0.1", default-features = false, optional = true }
log = { version = "0.4", default-features = false } log = { version = "0.4", default-features = false }
bumpalo = "3" bumpalo = "3"
subsecond = { version = "0.7.0-alpha.1", optional = true } subsecond = { version = "0.7.0-alpha.1", optional = true }
slotmap = { version = "1.0.7", default-features = false }
concurrent-queue = { version = "2.5.0", default-features = false } concurrent-queue = { version = "2.5.0", default-features = false }
[target.'cfg(not(all(target_has_atomic = "8", target_has_atomic = "16", target_has_atomic = "32", target_has_atomic = "64", target_has_atomic = "ptr")))'.dependencies] [target.'cfg(not(all(target_has_atomic = "8", target_has_atomic = "16", target_has_atomic = "32", target_has_atomic = "64", target_has_atomic = "ptr")))'.dependencies]

145
crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs
View File

@ -2,8 +2,11 @@ use alloc::{boxed::Box, collections::BTreeSet, vec::Vec};
use bevy_platform::collections::HashMap; use bevy_platform::collections::HashMap;
use crate::system::IntoSystem; use crate::{
use crate::world::World; schedule::{SystemKey, SystemSetKey},
system::IntoSystem,
world::World,
};
use super::{ use super::{
is_apply_deferred, ApplyDeferred, DiGraph, Direction, NodeId, ReportCycles, ScheduleBuildError, is_apply_deferred, ApplyDeferred, DiGraph, Direction, NodeId, ReportCycles, ScheduleBuildError,
@ -36,29 +39,26 @@ impl AutoInsertApplyDeferredPass {
self.auto_sync_node_ids self.auto_sync_node_ids
.get(&distance) .get(&distance)
.copied() .copied()
.or_else(|| { .unwrap_or_else(|| {
let node_id = self.add_auto_sync(graph); let node_id = NodeId::System(self.add_auto_sync(graph));
self.auto_sync_node_ids.insert(distance, node_id); self.auto_sync_node_ids.insert(distance, node_id);
Some(node_id) node_id
}) })
.unwrap()
} }
/// add an [`ApplyDeferred`] system with no config /// add an [`ApplyDeferred`] system with no config
fn add_auto_sync(&mut self, graph: &mut ScheduleGraph) -> NodeId { fn add_auto_sync(&mut self, graph: &mut ScheduleGraph) -> SystemKey {
let id = NodeId::System(graph.systems.len()); let key = graph
graph
.systems .systems
.push(SystemNode::new(Box::new(IntoSystem::into_system( .insert(SystemNode::new(Box::new(IntoSystem::into_system(
ApplyDeferred, ApplyDeferred,
)))); ))));
graph.system_conditions.push(Vec::new()); graph.system_conditions.insert(key, Vec::new());
// ignore ambiguities with auto sync points // ignore ambiguities with auto sync points
// They aren't under user control, so no one should know or care. // They aren't under user control, so no one should know or care.
graph.ambiguous_with_all.insert(id); graph.ambiguous_with_all.insert(NodeId::System(key));
id key
} }
} }
@ -80,39 +80,45 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
let mut sync_point_graph = dependency_flattened.clone(); let mut sync_point_graph = dependency_flattened.clone();
let topo = graph.topsort_graph(dependency_flattened, ReportCycles::Dependency)?; let topo = graph.topsort_graph(dependency_flattened, ReportCycles::Dependency)?;
fn set_has_conditions(graph: &ScheduleGraph, node: NodeId) -> bool { fn set_has_conditions(graph: &ScheduleGraph, set: SystemSetKey) -> bool {
!graph.set_conditions_at(node).is_empty() !graph.set_conditions_at(set).is_empty()
|| graph || graph
.hierarchy() .hierarchy()
.graph() .graph()
.edges_directed(node, Direction::Incoming) .edges_directed(NodeId::Set(set), Direction::Incoming)
.any(|(parent, _)| set_has_conditions(graph, parent)) .any(|(parent, _)| {
parent
.as_set()
.is_some_and(|p| set_has_conditions(graph, p))
})
} }
fn system_has_conditions(graph: &ScheduleGraph, node: NodeId) -> bool { fn system_has_conditions(graph: &ScheduleGraph, key: SystemKey) -> bool {
assert!(node.is_system()); !graph.system_conditions[key].is_empty()
!graph.system_conditions[node.index()].is_empty()
|| graph || graph
.hierarchy() .hierarchy()
.graph() .graph()
.edges_directed(node, Direction::Incoming) .edges_directed(NodeId::System(key), Direction::Incoming)
.any(|(parent, _)| set_has_conditions(graph, parent)) .any(|(parent, _)| {
parent
.as_set()
.is_some_and(|p| set_has_conditions(graph, p))
})
} }
let mut system_has_conditions_cache = HashMap::<usize, bool>::default(); let mut system_has_conditions_cache = HashMap::<SystemKey, bool>::default();
let mut is_valid_explicit_sync_point = |system: NodeId| { let mut is_valid_explicit_sync_point = |key: SystemKey| {
let index = system.index(); is_apply_deferred(&graph.systems[key].get().unwrap().system)
is_apply_deferred(&graph.systems[index].get().unwrap().system)
&& !*system_has_conditions_cache && !*system_has_conditions_cache
.entry(index) .entry(key)
.or_insert_with(|| system_has_conditions(graph, system)) .or_insert_with(|| system_has_conditions(graph, key))
}; };
// Calculate the distance for each node. // Calculate the distance for each node.
// The "distance" is the number of sync points between a node and the beginning of the graph. // The "distance" is the number of sync points between a node and the beginning of the graph.
// Also store if a preceding edge would have added a sync point but was ignored to add it at // Also store if a preceding edge would have added a sync point but was ignored to add it at
// a later edge that is not ignored. // a later edge that is not ignored.
let mut distances_and_pending_sync: HashMap<usize, (u32, bool)> = let mut distances_and_pending_sync: HashMap<SystemKey, (u32, bool)> =
HashMap::with_capacity_and_hasher(topo.len(), Default::default()); HashMap::with_capacity_and_hasher(topo.len(), Default::default());
// Keep track of any explicit sync nodes for a specific distance. // Keep track of any explicit sync nodes for a specific distance.
@ -120,17 +126,21 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
// Determine the distance for every node and collect the explicit sync points. // Determine the distance for every node and collect the explicit sync points.
for node in &topo { for node in &topo {
let &NodeId::System(key) = node else {
panic!("Encountered a non-system node in the flattened dependency graph: {node:?}");
};
let (node_distance, mut node_needs_sync) = distances_and_pending_sync let (node_distance, mut node_needs_sync) = distances_and_pending_sync
.get(&node.index()) .get(&key)
.copied() .copied()
.unwrap_or_default(); .unwrap_or_default();
if is_valid_explicit_sync_point(*node) { if is_valid_explicit_sync_point(key) {
// The distance of this sync point does not change anymore as the iteration order // The distance of this sync point does not change anymore as the iteration order
// makes sure that this node is no unvisited target of another node. // makes sure that this node is no unvisited target of another node.
// Because of this, the sync point can be stored for this distance to be reused as // Because of this, the sync point can be stored for this distance to be reused as
// automatically added sync points later. // automatically added sync points later.
distance_to_explicit_sync_node.insert(node_distance, *node); distance_to_explicit_sync_node.insert(node_distance, NodeId::System(key));
// This node just did a sync, so the only reason to do another sync is if one was // This node just did a sync, so the only reason to do another sync is if one was
// explicitly scheduled afterwards. // explicitly scheduled afterwards.
@ -138,26 +148,22 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
} else if !node_needs_sync { } else if !node_needs_sync {
// No previous node has postponed sync points to add so check if the system itself // No previous node has postponed sync points to add so check if the system itself
// has deferred params that require a sync point to apply them. // has deferred params that require a sync point to apply them.
node_needs_sync = graph.systems[node.index()] node_needs_sync = graph.systems[key].get().unwrap().system.has_deferred();
.get()
.unwrap()
.system
.has_deferred();
} }
for target in dependency_flattened.neighbors_directed(*node, Direction::Outgoing) { for target in dependency_flattened.neighbors_directed(*node, Direction::Outgoing) {
let (target_distance, target_pending_sync) = distances_and_pending_sync let NodeId::System(target) = target else {
.entry(target.index()) panic!("Encountered a non-system node in the flattened dependency graph: {target:?}");
.or_default(); };
let (target_distance, target_pending_sync) =
distances_and_pending_sync.entry(target).or_default();
let mut edge_needs_sync = node_needs_sync; let mut edge_needs_sync = node_needs_sync;
if node_needs_sync if node_needs_sync
&& !graph.systems[target.index()] && !graph.systems[target].get().unwrap().system.is_exclusive()
.get() && self
.unwrap() .no_sync_edges
.system .contains(&(*node, NodeId::System(target)))
.is_exclusive()
&& self.no_sync_edges.contains(&(*node, target))
{ {
// The node has deferred params to apply, but this edge is ignoring sync points. // The node has deferred params to apply, but this edge is ignoring sync points.
// Mark the target as 'delaying' those commands to a future edge and the current // Mark the target as 'delaying' those commands to a future edge and the current
@ -182,14 +188,20 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
// Find any edges which have a different number of sync points between them and make sure // Find any edges which have a different number of sync points between them and make sure
// there is a sync point between them. // there is a sync point between them.
for node in &topo { for node in &topo {
let &NodeId::System(key) = node else {
panic!("Encountered a non-system node in the flattened dependency graph: {node:?}");
};
let (node_distance, _) = distances_and_pending_sync let (node_distance, _) = distances_and_pending_sync
.get(&node.index()) .get(&key)
.copied() .copied()
.unwrap_or_default(); .unwrap_or_default();
for target in dependency_flattened.neighbors_directed(*node, Direction::Outgoing) { for target in dependency_flattened.neighbors_directed(*node, Direction::Outgoing) {
let NodeId::System(target) = target else {
panic!("Encountered a non-system node in the flattened dependency graph: {target:?}");
};
let (target_distance, _) = distances_and_pending_sync let (target_distance, _) = distances_and_pending_sync
.get(&target.index()) .get(&target)
.copied() .copied()
.unwrap_or_default(); .unwrap_or_default();
@ -198,7 +210,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
continue; continue;
} }
if is_apply_deferred(&graph.systems[target.index()].get().unwrap().system) { if is_apply_deferred(&graph.systems[target].get().unwrap().system) {
// We don't need to insert a sync point since ApplyDeferred is a sync point // We don't need to insert a sync point since ApplyDeferred is a sync point
// already! // already!
continue; continue;
@ -210,10 +222,10 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
.unwrap_or_else(|| self.get_sync_point(graph, target_distance)); .unwrap_or_else(|| self.get_sync_point(graph, target_distance));
sync_point_graph.add_edge(*node, sync_point); sync_point_graph.add_edge(*node, sync_point);
sync_point_graph.add_edge(sync_point, target); sync_point_graph.add_edge(sync_point, NodeId::System(target));
// The edge without the sync point is now redundant. // The edge without the sync point is now redundant.
sync_point_graph.remove_edge(*node, target); sync_point_graph.remove_edge(*node, NodeId::System(target));
} }
} }
@ -223,34 +235,39 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
fn collapse_set( fn collapse_set(
&mut self, &mut self,
set: NodeId, set: SystemSetKey,
systems: &[NodeId], systems: &[SystemKey],
dependency_flattened: &DiGraph, dependency_flattened: &DiGraph,
) -> impl Iterator<Item = (NodeId, NodeId)> { ) -> impl Iterator<Item = (NodeId, NodeId)> {
if systems.is_empty() { if systems.is_empty() {
// collapse dependencies for empty sets // collapse dependencies for empty sets
for a in dependency_flattened.neighbors_directed(set, Direction::Incoming) { for a in dependency_flattened.neighbors_directed(NodeId::Set(set), Direction::Incoming)
for b in dependency_flattened.neighbors_directed(set, Direction::Outgoing) { {
if self.no_sync_edges.contains(&(a, set)) for b in
&& self.no_sync_edges.contains(&(set, b)) dependency_flattened.neighbors_directed(NodeId::Set(set), Direction::Outgoing)
{
if self.no_sync_edges.contains(&(a, NodeId::Set(set)))
&& self.no_sync_edges.contains(&(NodeId::Set(set), b))
{ {
self.no_sync_edges.insert((a, b)); self.no_sync_edges.insert((a, b));
} }
} }
} }
} else { } else {
for a in dependency_flattened.neighbors_directed(set, Direction::Incoming) { for a in dependency_flattened.neighbors_directed(NodeId::Set(set), Direction::Incoming)
{
for &sys in systems { for &sys in systems {
if self.no_sync_edges.contains(&(a, set)) { if self.no_sync_edges.contains(&(a, NodeId::Set(set))) {
self.no_sync_edges.insert((a, sys)); self.no_sync_edges.insert((a, NodeId::System(sys)));
} }
} }
} }
for b in dependency_flattened.neighbors_directed(set, Direction::Outgoing) { for b in dependency_flattened.neighbors_directed(NodeId::Set(set), Direction::Outgoing)
{
for &sys in systems { for &sys in systems {
if self.no_sync_edges.contains(&(set, b)) { if self.no_sync_edges.contains(&(NodeId::Set(set), b)) {
self.no_sync_edges.insert((sys, b)); self.no_sync_edges.insert((NodeId::System(sys), b));
} }
} }
} }

9
crates/bevy_ecs/src/schedule/executor/mod.rs
View File

@ -20,7 +20,10 @@ use crate::{
error::{BevyError, ErrorContext, Result}, error::{BevyError, ErrorContext, Result},
prelude::{IntoSystemSet, SystemSet}, prelude::{IntoSystemSet, SystemSet},
query::FilteredAccessSet, query::FilteredAccessSet,
schedule::{ConditionWithAccess, InternedSystemSet, NodeId, SystemTypeSet, SystemWithAccess}, schedule::{
ConditionWithAccess, InternedSystemSet, SystemKey, SystemSetKey, SystemTypeSet,
SystemWithAccess,
},
system::{ScheduleSystem, System, SystemIn, SystemParamValidationError, SystemStateFlags}, system::{ScheduleSystem, System, SystemIn, SystemParamValidationError, SystemStateFlags},
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
}; };
@ -73,7 +76,7 @@ pub enum ExecutorKind {
#[derive(Default)] #[derive(Default)]
pub struct SystemSchedule { pub struct SystemSchedule {
/// List of system node ids. /// List of system node ids.
pub(super) system_ids: Vec<NodeId>, pub(super) system_ids: Vec<SystemKey>,
/// Indexed by system node id. /// Indexed by system node id.
pub(super) systems: Vec<SystemWithAccess>, pub(super) systems: Vec<SystemWithAccess>,
/// Indexed by system node id. /// Indexed by system node id.
@ -96,7 +99,7 @@ pub struct SystemSchedule {
/// List of sets containing the system that have conditions /// List of sets containing the system that have conditions
pub(super) sets_with_conditions_of_systems: Vec<FixedBitSet>, pub(super) sets_with_conditions_of_systems: Vec<FixedBitSet>,
/// List of system set node ids. /// List of system set node ids.
pub(super) set_ids: Vec<NodeId>, pub(super) set_ids: Vec<SystemSetKey>,
/// Indexed by system set node id. /// Indexed by system set node id.
pub(super) set_conditions: Vec<Vec<ConditionWithAccess>>, pub(super) set_conditions: Vec<Vec<ConditionWithAccess>>,
/// Indexed by system set node id. /// Indexed by system set node id.

115
crates/bevy_ecs/src/schedule/graph/graph_map.rs
View File

@ -11,6 +11,7 @@ use core::{
hash::{BuildHasher, Hash}, hash::{BuildHasher, Hash},
}; };
use indexmap::IndexMap; use indexmap::IndexMap;
use slotmap::{Key, KeyData};
use smallvec::SmallVec; use smallvec::SmallVec;
use super::NodeId; use super::NodeId;
@ -298,7 +299,7 @@ impl Direction {
/// Compact storage of a [`NodeId`] and a [`Direction`]. /// Compact storage of a [`NodeId`] and a [`Direction`].
#[derive(Clone, Copy)] #[derive(Clone, Copy)]
struct CompactNodeIdAndDirection { struct CompactNodeIdAndDirection {
index: usize, key: KeyData,
is_system: bool, is_system: bool,
direction: Direction, direction: Direction,
} }
@ -310,27 +311,30 @@ impl fmt::Debug for CompactNodeIdAndDirection {
} }
impl CompactNodeIdAndDirection { impl CompactNodeIdAndDirection {
const fn store(node: NodeId, direction: Direction) -> Self { fn store(node: NodeId, direction: Direction) -> Self {
let index = node.index(); let key = match node {
NodeId::System(key) => key.data(),
NodeId::Set(key) => key.data(),
};
let is_system = node.is_system(); let is_system = node.is_system();
Self { Self {
index, key,
is_system, is_system,
direction, direction,
} }
} }
const fn load(self) -> (NodeId, Direction) { fn load(self) -> (NodeId, Direction) {
let Self { let Self {
index, key,
is_system, is_system,
direction, direction,
} = self; } = self;
let node = match is_system { let node = match is_system {
true => NodeId::System(index), true => NodeId::System(key.into()),
false => NodeId::Set(index), false => NodeId::Set(key.into()),
}; };
(node, direction) (node, direction)
@ -340,8 +344,8 @@ impl CompactNodeIdAndDirection {
/// Compact storage of a [`NodeId`] pair. /// Compact storage of a [`NodeId`] pair.
#[derive(Clone, Copy, Hash, PartialEq, Eq)] #[derive(Clone, Copy, Hash, PartialEq, Eq)]
struct CompactNodeIdPair { struct CompactNodeIdPair {
index_a: usize, key_a: KeyData,
index_b: usize, key_b: KeyData,
is_system_a: bool, is_system_a: bool,
is_system_b: bool, is_system_b: bool,
} }
@ -353,37 +357,43 @@ impl fmt::Debug for CompactNodeIdPair {
} }
impl CompactNodeIdPair { impl CompactNodeIdPair {
const fn store(a: NodeId, b: NodeId) -> Self { fn store(a: NodeId, b: NodeId) -> Self {
let index_a = a.index(); let key_a = match a {
NodeId::System(index) => index.data(),
NodeId::Set(index) => index.data(),
};
let is_system_a = a.is_system(); let is_system_a = a.is_system();
let index_b = b.index(); let key_b = match b {
NodeId::System(index) => index.data(),
NodeId::Set(index) => index.data(),
};
let is_system_b = b.is_system(); let is_system_b = b.is_system();
Self { Self {
index_a, key_a,
index_b, key_b,
is_system_a, is_system_a,
is_system_b, is_system_b,
} }
} }
const fn load(self) -> (NodeId, NodeId) { fn load(self) -> (NodeId, NodeId) {
let Self { let Self {
index_a, key_a,
index_b, key_b,
is_system_a, is_system_a,
is_system_b, is_system_b,
} = self; } = self;
let a = match is_system_a { let a = match is_system_a {
true => NodeId::System(index_a), true => NodeId::System(key_a.into()),
false => NodeId::Set(index_a), false => NodeId::Set(key_a.into()),
}; };
let b = match is_system_b { let b = match is_system_b {
true => NodeId::System(index_b), true => NodeId::System(key_b.into()),
false => NodeId::Set(index_b), false => NodeId::Set(key_b.into()),
}; };
(a, b) (a, b)
@ -392,8 +402,11 @@ impl CompactNodeIdPair {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use crate::schedule::SystemKey;
use super::*; use super::*;
use alloc::vec; use alloc::vec;
use slotmap::SlotMap;
/// The `Graph` type _must_ preserve the order that nodes are inserted in if /// The `Graph` type _must_ preserve the order that nodes are inserted in if
/// no removals occur. Removals are permitted to swap the latest node into the /// no removals occur. Removals are permitted to swap the latest node into the
@ -402,37 +415,43 @@ mod tests {
fn node_order_preservation() { fn node_order_preservation() {
use NodeId::System; use NodeId::System;
let mut slotmap = SlotMap::<SystemKey, ()>::with_key();
let mut graph = <DiGraph>::default(); let mut graph = <DiGraph>::default();
graph.add_node(System(1)); let sys1 = slotmap.insert(());
graph.add_node(System(2)); let sys2 = slotmap.insert(());
graph.add_node(System(3)); let sys3 = slotmap.insert(());
graph.add_node(System(4)); let sys4 = slotmap.insert(());
graph.add_node(System(sys1));
graph.add_node(System(sys2));
graph.add_node(System(sys3));
graph.add_node(System(sys4));
assert_eq!( assert_eq!(
graph.nodes().collect::<Vec<_>>(), graph.nodes().collect::<Vec<_>>(),
vec![System(1), System(2), System(3), System(4)] vec![System(sys1), System(sys2), System(sys3), System(sys4)]
); );
graph.remove_node(System(1)); graph.remove_node(System(sys1));
assert_eq!( assert_eq!(
graph.nodes().collect::<Vec<_>>(), graph.nodes().collect::<Vec<_>>(),
vec![System(4), System(2), System(3)] vec![System(sys4), System(sys2), System(sys3)]
); );
graph.remove_node(System(4)); graph.remove_node(System(sys4));
assert_eq!( assert_eq!(
graph.nodes().collect::<Vec<_>>(), graph.nodes().collect::<Vec<_>>(),
vec![System(3), System(2)] vec![System(sys3), System(sys2)]
); );
graph.remove_node(System(2)); graph.remove_node(System(sys2));
assert_eq!(graph.nodes().collect::<Vec<_>>(), vec![System(3)]); assert_eq!(graph.nodes().collect::<Vec<_>>(), vec![System(sys3)]);
graph.remove_node(System(3)); graph.remove_node(System(sys3));
assert_eq!(graph.nodes().collect::<Vec<_>>(), vec![]); assert_eq!(graph.nodes().collect::<Vec<_>>(), vec![]);
} }
@ -444,18 +463,26 @@ mod tests {
fn strongly_connected_components() { fn strongly_connected_components() {
use NodeId::System; use NodeId::System;
let mut slotmap = SlotMap::<SystemKey, ()>::with_key();
let mut graph = <DiGraph>::default(); let mut graph = <DiGraph>::default();
graph.add_edge(System(1), System(2)); let sys1 = slotmap.insert(());
graph.add_edge(System(2), System(1)); let sys2 = slotmap.insert(());
let sys3 = slotmap.insert(());
let sys4 = slotmap.insert(());
let sys5 = slotmap.insert(());
let sys6 = slotmap.insert(());
graph.add_edge(System(2), System(3)); graph.add_edge(System(sys1), System(sys2));
graph.add_edge(System(3), System(2)); graph.add_edge(System(sys2), System(sys1));
graph.add_edge(System(4), System(5)); graph.add_edge(System(sys2), System(sys3));
graph.add_edge(System(5), System(4)); graph.add_edge(System(sys3), System(sys2));
graph.add_edge(System(6), System(2)); graph.add_edge(System(sys4), System(sys5));
graph.add_edge(System(sys5), System(sys4));
graph.add_edge(System(sys6), System(sys2));
let sccs = graph let sccs = graph
.iter_sccs() .iter_sccs()
@ -465,9 +492,9 @@ mod tests {
assert_eq!( assert_eq!(
sccs, sccs,
vec![ vec![
vec![System(3), System(2), System(1)], vec![System(sys3), System(sys2), System(sys1)],
vec![System(5), System(4)], vec![System(sys5), System(sys4)],
vec![System(6)] vec![System(sys6)]
] ]
); );
} }

37
crates/bevy_ecs/src/schedule/graph/node.rs
View File

@ -1,24 +1,19 @@
use core::fmt::Debug; use core::fmt::Debug;
use crate::schedule::{SystemKey, SystemSetKey};
/// Unique identifier for a system or system set stored in a [`ScheduleGraph`]. /// Unique identifier for a system or system set stored in a [`ScheduleGraph`].
/// ///
/// [`ScheduleGraph`]: crate::schedule::ScheduleGraph /// [`ScheduleGraph`]: crate::schedule::ScheduleGraph
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub enum NodeId { pub enum NodeId {
/// Identifier for a system. /// Identifier for a system.
System(usize), System(SystemKey),
/// Identifier for a system set. /// Identifier for a system set.
Set(usize), Set(SystemSetKey),
} }
impl NodeId { impl NodeId {
/// Returns the internal integer value.
pub const fn index(&self) -> usize {
match self {
NodeId::System(index) | NodeId::Set(index) => *index,
}
}
/// Returns `true` if the identified node is a system. /// Returns `true` if the identified node is a system.
pub const fn is_system(&self) -> bool { pub const fn is_system(&self) -> bool {
matches!(self, NodeId::System(_)) matches!(self, NodeId::System(_))
@ -29,19 +24,19 @@ impl NodeId {
matches!(self, NodeId::Set(_)) matches!(self, NodeId::Set(_))
} }
/// Compare this [`NodeId`] with another. /// Returns the system key if the node is a system, otherwise `None`.
pub const fn cmp(&self, other: &Self) -> core::cmp::Ordering { pub const fn as_system(&self) -> Option<SystemKey> {
use core::cmp::Ordering::{Equal, Greater, Less}; match self {
use NodeId::{Set, System}; NodeId::System(system) => Some(*system),
NodeId::Set(_) => None,
}
}
match (self, other) { /// Returns the system set key if the node is a system set, otherwise `None`.
(System(a), System(b)) | (Set(a), Set(b)) => match a.checked_sub(*b) { pub const fn as_set(&self) -> Option<SystemSetKey> {
None => Less, match self {
Some(0) => Equal, NodeId::System(_) => None,
Some(_) => Greater, NodeId::Set(set) => Some(*set),
},
(System(_), Set(_)) => Less,
(Set(_), System(_)) => Greater,
} }
} }
} }

17
crates/bevy_ecs/src/schedule/pass.rs
View File

@ -2,7 +2,10 @@ use alloc::{boxed::Box, vec::Vec};
use core::any::{Any, TypeId}; use core::any::{Any, TypeId};
use super::{DiGraph, NodeId, ScheduleBuildError, ScheduleGraph}; use super::{DiGraph, NodeId, ScheduleBuildError, ScheduleGraph};
use crate::world::World; use crate::{
schedule::{SystemKey, SystemSetKey},
world::World,
};
use bevy_utils::TypeIdMap; use bevy_utils::TypeIdMap;
use core::fmt::Debug; use core::fmt::Debug;
@ -19,8 +22,8 @@ pub trait ScheduleBuildPass: Send + Sync + Debug + 'static {
/// Instead of modifying the graph directly, this method should return an iterator of edges to add to the graph. /// Instead of modifying the graph directly, this method should return an iterator of edges to add to the graph.
fn collapse_set( fn collapse_set(
&mut self, &mut self,
set: NodeId, set: SystemSetKey,
systems: &[NodeId], systems: &[SystemKey],
dependency_flattened: &DiGraph, dependency_flattened: &DiGraph,
) -> impl Iterator<Item = (NodeId, NodeId)>; ) -> impl Iterator<Item = (NodeId, NodeId)>;
@ -44,8 +47,8 @@ pub(super) trait ScheduleBuildPassObj: Send + Sync + Debug {
fn collapse_set( fn collapse_set(
&mut self, &mut self,
set: NodeId, set: SystemSetKey,
systems: &[NodeId], systems: &[SystemKey],
dependency_flattened: &DiGraph, dependency_flattened: &DiGraph,
dependencies_to_add: &mut Vec<(NodeId, NodeId)>, dependencies_to_add: &mut Vec<(NodeId, NodeId)>,
); );
@ -63,8 +66,8 @@ impl<T: ScheduleBuildPass> ScheduleBuildPassObj for T {
} }
fn collapse_set( fn collapse_set(
&mut self, &mut self,
set: NodeId, set: SystemSetKey,
systems: &[NodeId], systems: &[SystemKey],
dependency_flattened: &DiGraph, dependency_flattened: &DiGraph,
dependencies_to_add: &mut Vec<(NodeId, NodeId)>, dependencies_to_add: &mut Vec<(NodeId, NodeId)>,
) { ) {

479
crates/bevy_ecs/src/schedule/schedule.rs
View File

@ -19,6 +19,7 @@ use core::{
use fixedbitset::FixedBitSet; use fixedbitset::FixedBitSet;
use log::{error, info, warn}; use log::{error, info, warn};
use pass::ScheduleBuildPassObj; use pass::ScheduleBuildPassObj;
use slotmap::{new_key_type, SecondaryMap, SlotMap};
use thiserror::Error; use thiserror::Error;
#[cfg(feature = "trace")] #[cfg(feature = "trace")]
use tracing::info_span; use tracing::info_span;
@ -404,7 +405,9 @@ impl Schedule {
); );
}; };
self.graph.ambiguous_with.add_edge(a_id, b_id); self.graph
.ambiguous_with
.add_edge(NodeId::Set(a_id), NodeId::Set(b_id));
self self
} }
@ -599,7 +602,7 @@ impl Schedule {
/// schedule has never been initialized or run. /// schedule has never been initialized or run.
pub fn systems( pub fn systems(
&self, &self,
) -> Result<impl Iterator<Item = (NodeId, &ScheduleSystem)> + Sized, ScheduleNotInitialized> ) -> Result<impl Iterator<Item = (SystemKey, &ScheduleSystem)> + Sized, ScheduleNotInitialized>
{ {
if !self.executor_initialized { if !self.executor_initialized {
return Err(ScheduleNotInitialized); return Err(ScheduleNotInitialized);
@ -610,7 +613,7 @@ impl Schedule {
.system_ids .system_ids
.iter() .iter()
.zip(&self.executable.systems) .zip(&self.executable.systems)
.map(|(node_id, system)| (*node_id, &system.system)); .map(|(&node_id, system)| (node_id, &system.system));
Ok(iter) Ok(iter)
} }
@ -742,6 +745,21 @@ impl SystemNode {
} }
} }
new_key_type! {
/// A unique identifier for a system in a [`ScheduleGraph`].
pub struct SystemKey;
/// A unique identifier for a system set in a [`ScheduleGraph`].
pub struct SystemSetKey;
}
enum UninitializedId {
System(SystemKey),
Set {
key: SystemSetKey,
first_uninit_condition: usize,
},
}
/// Metadata for a [`Schedule`]. /// Metadata for a [`Schedule`].
/// ///
/// The order isn't optimized; calling `ScheduleGraph::build_schedule` will return a /// The order isn't optimized; calling `ScheduleGraph::build_schedule` will return a
@ -749,18 +767,18 @@ impl SystemNode {
#[derive(Default)] #[derive(Default)]
pub struct ScheduleGraph { pub struct ScheduleGraph {
/// List of systems in the schedule /// List of systems in the schedule
pub systems: Vec<SystemNode>, pub systems: SlotMap<SystemKey, SystemNode>,
/// List of conditions for each system, in the same order as `systems` /// List of conditions for each system, in the same order as `systems`
pub system_conditions: Vec<Vec<ConditionWithAccess>>, pub system_conditions: SecondaryMap<SystemKey, Vec<ConditionWithAccess>>,
/// List of system sets in the schedule /// List of system sets in the schedule
system_sets: Vec<SystemSetNode>, system_sets: SlotMap<SystemSetKey, SystemSetNode>,
/// List of conditions for each system set, in the same order as `system_sets` /// List of conditions for each system set, in the same order as `system_sets`
system_set_conditions: Vec<Vec<ConditionWithAccess>>, system_set_conditions: SecondaryMap<SystemSetKey, Vec<ConditionWithAccess>>,
/// Map from system set to node id /// Map from system set to node id
system_set_ids: HashMap<InternedSystemSet, NodeId>, system_set_ids: HashMap<InternedSystemSet, SystemSetKey>,
/// Systems that have not been initialized yet; for system sets, we store the index of the first uninitialized condition /// Systems that have not been initialized yet; for system sets, we store the index of the first uninitialized condition
/// (all the conditions after that index still need to be initialized) /// (all the conditions after that index still need to be initialized)
uninit: Vec<(NodeId, usize)>, uninit: Vec<UninitializedId>,
/// Directed acyclic graph of the hierarchy (which systems/sets are children of which sets) /// Directed acyclic graph of the hierarchy (which systems/sets are children of which sets)
hierarchy: Dag, hierarchy: Dag,
/// Directed acyclic graph of the dependency (which systems/sets have to run before which other systems/sets) /// Directed acyclic graph of the dependency (which systems/sets have to run before which other systems/sets)
@ -768,7 +786,7 @@ pub struct ScheduleGraph {
ambiguous_with: UnGraph, ambiguous_with: UnGraph,
/// Nodes that are allowed to have ambiguous ordering relationship with any other systems. /// Nodes that are allowed to have ambiguous ordering relationship with any other systems.
pub ambiguous_with_all: HashSet<NodeId>, pub ambiguous_with_all: HashSet<NodeId>,
conflicting_systems: Vec<(NodeId, NodeId, Vec<ComponentId>)>, conflicting_systems: Vec<(SystemKey, SystemKey, Vec<ComponentId>)>,
anonymous_sets: usize, anonymous_sets: usize,
changed: bool, changed: bool,
settings: ScheduleBuildSettings, settings: ScheduleBuildSettings,
@ -780,10 +798,10 @@ impl ScheduleGraph {
/// Creates an empty [`ScheduleGraph`] with default settings. /// Creates an empty [`ScheduleGraph`] with default settings.
pub fn new() -> Self { pub fn new() -> Self {
Self { Self {
systems: Vec::new(), systems: SlotMap::with_key(),
system_conditions: Vec::new(), system_conditions: SecondaryMap::new(),
system_sets: Vec::new(), system_sets: SlotMap::with_key(),
system_set_conditions: Vec::new(), system_set_conditions: SecondaryMap::new(),
system_set_ids: HashMap::default(), system_set_ids: HashMap::default(),
uninit: Vec::new(), uninit: Vec::new(),
hierarchy: Dag::new(), hierarchy: Dag::new(),
@ -798,14 +816,11 @@ impl ScheduleGraph {
} }
} }
/// Returns the system at the given [`NodeId`], if it exists. /// Returns the system at the given [`SystemKey`], if it exists.
pub fn get_system_at(&self, id: NodeId) -> Option<&ScheduleSystem> { pub fn get_system_at(&self, key: SystemKey) -> Option<&ScheduleSystem> {
if !id.is_system() {
return None;
}
self.systems self.systems
.get(id.index()) .get(key)
.and_then(|system| system.inner.as_ref()) .and_then(|system| system.get())
.map(|system| &system.system) .map(|system| &system.system)
} }
@ -818,74 +833,59 @@ impl ScheduleGraph {
/// ///
/// Panics if it doesn't exist. /// Panics if it doesn't exist.
#[track_caller] #[track_caller]
pub fn system_at(&self, id: NodeId) -> &ScheduleSystem { pub fn system_at(&self, key: SystemKey) -> &ScheduleSystem {
self.get_system_at(id) self.get_system_at(key)
.ok_or_else(|| format!("system with id {id:?} does not exist in this Schedule")) .unwrap_or_else(|| panic!("system with key {key:?} does not exist in this Schedule"))
.unwrap()
} }
/// Returns the set at the given [`NodeId`], if it exists. /// Returns the set at the given [`NodeId`], if it exists.
pub fn get_set_at(&self, id: NodeId) -> Option<&dyn SystemSet> { pub fn get_set_at(&self, key: SystemSetKey) -> Option<&dyn SystemSet> {
if !id.is_set() { self.system_sets.get(key).map(|set| &*set.inner)
return None;
}
self.system_sets.get(id.index()).map(|set| &*set.inner)
} }
/// Returns the set at the given [`NodeId`]. /// Returns the set at the given [`NodeId`].
/// ///
/// Panics if it doesn't exist. /// Panics if it doesn't exist.
#[track_caller] #[track_caller]
pub fn set_at(&self, id: NodeId) -> &dyn SystemSet { pub fn set_at(&self, id: SystemSetKey) -> &dyn SystemSet {
self.get_set_at(id) self.get_set_at(id)
.ok_or_else(|| format!("set with id {id:?} does not exist in this Schedule")) .unwrap_or_else(|| panic!("set with id {id:?} does not exist in this Schedule"))
.unwrap()
} }
/// Returns the conditions for the set at the given [`NodeId`], if it exists. /// Returns the conditions for the set at the given [`SystemSetKey`], if it exists.
pub fn get_set_conditions_at(&self, id: NodeId) -> Option<&[ConditionWithAccess]> { pub fn get_set_conditions_at(&self, key: SystemSetKey) -> Option<&[ConditionWithAccess]> {
if !id.is_set() { self.system_set_conditions.get(key).map(Vec::as_slice)
return None;
}
self.system_set_conditions
.get(id.index())
.map(Vec::as_slice)
} }
/// Returns the conditions for the set at the given [`NodeId`]. /// Returns the conditions for the set at the given [`SystemSetKey`].
/// ///
/// Panics if it doesn't exist. /// Panics if it doesn't exist.
#[track_caller] #[track_caller]
pub fn set_conditions_at(&self, id: NodeId) -> &[ConditionWithAccess] { pub fn set_conditions_at(&self, key: SystemSetKey) -> &[ConditionWithAccess] {
self.get_set_conditions_at(id) self.get_set_conditions_at(key)
.ok_or_else(|| format!("set with id {id:?} does not exist in this Schedule")) .unwrap_or_else(|| panic!("set with key {key:?} does not exist in this Schedule"))
.unwrap()
} }
/// Returns an iterator over all systems in this schedule, along with the conditions for each system. /// Returns an iterator over all systems in this schedule, along with the conditions for each system.
pub fn systems( pub fn systems(
&self, &self,
) -> impl Iterator<Item = (NodeId, &ScheduleSystem, &[ConditionWithAccess])> { ) -> impl Iterator<Item = (SystemKey, &ScheduleSystem, &[ConditionWithAccess])> {
self.systems self.systems.iter().filter_map(|(key, system_node)| {
.iter() let system = &system_node.inner.as_ref()?.system;
.zip(self.system_conditions.iter()) let conditions = self.system_conditions.get(key)?;
.enumerate() Some((key, system, conditions.as_slice()))
.filter_map(|(i, (system_node, condition))| { })
let system = &system_node.inner.as_ref()?.system;
Some((NodeId::System(i), system, condition.as_slice()))
})
} }
/// Returns an iterator over all system sets in this schedule, along with the conditions for each /// Returns an iterator over all system sets in this schedule, along with the conditions for each
/// system set. /// system set.
pub fn system_sets( pub fn system_sets(
&self, &self,
) -> impl Iterator<Item = (NodeId, &dyn SystemSet, &[ConditionWithAccess])> { ) -> impl Iterator<Item = (SystemSetKey, &dyn SystemSet, &[ConditionWithAccess])> {
self.system_set_ids.iter().map(|(_, &node_id)| { self.system_sets.iter().filter_map(|(key, set_node)| {
let set_node = &self.system_sets[node_id.index()];
let set = &*set_node.inner; let set = &*set_node.inner;
let conditions = self.system_set_conditions[node_id.index()].as_slice(); let conditions = self.system_set_conditions.get(key)?.as_slice();
(node_id, set, conditions) Some((key, set, conditions))
}) })
} }
@ -909,7 +909,7 @@ impl ScheduleGraph {
/// ///
/// If the `Vec<ComponentId>` is empty, the systems conflict on [`World`] access. /// If the `Vec<ComponentId>` is empty, the systems conflict on [`World`] access.
/// Must be called after [`ScheduleGraph::build_schedule`] to be non-empty. /// Must be called after [`ScheduleGraph::build_schedule`] to be non-empty.
pub fn conflicting_systems(&self) -> &[(NodeId, NodeId, Vec<ComponentId>)] { pub fn conflicting_systems(&self) -> &[(SystemKey, SystemKey, Vec<ComponentId>)] {
&self.conflicting_systems &self.conflicting_systems
} }
@ -1051,23 +1051,22 @@ impl ScheduleGraph {
&mut self, &mut self,
config: ScheduleConfig<ScheduleSystem>, config: ScheduleConfig<ScheduleSystem>,
) -> Result<NodeId, ScheduleBuildError> { ) -> Result<NodeId, ScheduleBuildError> {
let id = NodeId::System(self.systems.len()); let key = self.systems.insert(SystemNode::new(config.node));
self.system_conditions.insert(
// graph updates are immediate key,
self.update_graphs(id, config.metadata)?;
// system init has to be deferred (need `&mut World`)
self.uninit.push((id, 0));
self.systems.push(SystemNode::new(config.node));
self.system_conditions.push(
config config
.conditions .conditions
.into_iter() .into_iter()
.map(ConditionWithAccess::new) .map(ConditionWithAccess::new)
.collect(), .collect(),
); );
// system init has to be deferred (need `&mut World`)
self.uninit.push(UninitializedId::System(key));
Ok(id) // graph updates are immediate
self.update_graphs(NodeId::System(key), config.metadata)?;
Ok(NodeId::System(key))
} }
#[track_caller] #[track_caller]
@ -1086,49 +1085,30 @@ impl ScheduleGraph {
conditions, conditions,
} = set; } = set;
let id = match self.system_set_ids.get(&set) { let key = match self.system_set_ids.get(&set) {
Some(&id) => id, Some(&id) => id,
None => self.add_set(set), None => self.add_set(set),
}; };
// graph updates are immediate // graph updates are immediate
self.update_graphs(id, metadata)?; self.update_graphs(NodeId::Set(key), metadata)?;
// system init has to be deferred (need `&mut World`) // system init has to be deferred (need `&mut World`)
let system_set_conditions = &mut self.system_set_conditions[id.index()]; let system_set_conditions = self.system_set_conditions.entry(key).unwrap().or_default();
self.uninit.push((id, system_set_conditions.len())); self.uninit.push(UninitializedId::Set {
key,
first_uninit_condition: system_set_conditions.len(),
});
system_set_conditions.extend(conditions.into_iter().map(ConditionWithAccess::new)); system_set_conditions.extend(conditions.into_iter().map(ConditionWithAccess::new));
Ok(id) Ok(NodeId::Set(key))
} }
fn add_set(&mut self, set: InternedSystemSet) -> NodeId { fn add_set(&mut self, set: InternedSystemSet) -> SystemSetKey {
let id = NodeId::Set(self.system_sets.len()); let key = self.system_sets.insert(SystemSetNode::new(set));
self.system_sets.push(SystemSetNode::new(set)); self.system_set_conditions.insert(key, Vec::new());
self.system_set_conditions.push(Vec::new()); self.system_set_ids.insert(set, key);
self.system_set_ids.insert(set, id); key
id
}
/// Checks that a system set isn't included in itself.
/// If not present, add the set to the graph.
fn check_hierarchy_set(
&mut self,
id: &NodeId,
set: InternedSystemSet,
) -> Result<(), ScheduleBuildError> {
match self.system_set_ids.get(&set) {
Some(set_id) => {
if id == set_id {
return Err(ScheduleBuildError::HierarchyLoop(self.get_node_name(id)));
}
}
None => {
self.add_set(set);
}
}
Ok(())
} }
fn create_anonymous_set(&mut self) -> AnonymousSet { fn create_anonymous_set(&mut self) -> AnonymousSet {
@ -1141,11 +1121,24 @@ impl ScheduleGraph {
/// Add all the sets from the [`GraphInfo`]'s hierarchy to the graph. /// Add all the sets from the [`GraphInfo`]'s hierarchy to the graph.
fn check_hierarchy_sets( fn check_hierarchy_sets(
&mut self, &mut self,
id: &NodeId, id: NodeId,
graph_info: &GraphInfo, graph_info: &GraphInfo,
) -> Result<(), ScheduleBuildError> { ) -> Result<(), ScheduleBuildError> {
for &set in &graph_info.hierarchy { for &set in &graph_info.hierarchy {
self.check_hierarchy_set(id, set)?; if let Some(&set_id) = self.system_set_ids.get(&set) {
if let NodeId::Set(key) = id
&& set_id == key
{
{
return Err(ScheduleBuildError::HierarchyLoop(
self.get_node_name(&NodeId::Set(key)),
));
}
}
} else {
// If the set is not in the graph, we add it
self.add_set(set);
}
} }
Ok(()) Ok(())
@ -1155,22 +1148,29 @@ impl ScheduleGraph {
/// Add all the sets from the [`GraphInfo`]'s dependencies to the graph. /// Add all the sets from the [`GraphInfo`]'s dependencies to the graph.
fn check_edges( fn check_edges(
&mut self, &mut self,
id: &NodeId, id: NodeId,
graph_info: &GraphInfo, graph_info: &GraphInfo,
) -> Result<(), ScheduleBuildError> { ) -> Result<(), ScheduleBuildError> {
for Dependency { set, .. } in &graph_info.dependencies { for Dependency { set, .. } in &graph_info.dependencies {
match self.system_set_ids.get(set) { if let Some(&set_id) = self.system_set_ids.get(set) {
Some(set_id) => { if let NodeId::Set(key) = id
if id == set_id { && set_id == key
return Err(ScheduleBuildError::DependencyLoop(self.get_node_name(id))); {
} return Err(ScheduleBuildError::DependencyLoop(
} self.get_node_name(&NodeId::Set(key)),
None => { ));
self.add_set(*set);
} }
} else {
// If the set is not in the graph, we add it
self.add_set(*set);
} }
} }
Ok(())
}
/// Add all the sets from the [`GraphInfo`]'s ambiguity to the graph.
fn add_ambiguities(&mut self, graph_info: &GraphInfo) {
if let Ambiguity::IgnoreWithSet(ambiguous_with) = &graph_info.ambiguous_with { if let Ambiguity::IgnoreWithSet(ambiguous_with) = &graph_info.ambiguous_with {
for set in ambiguous_with { for set in ambiguous_with {
if !self.system_set_ids.contains_key(set) { if !self.system_set_ids.contains_key(set) {
@ -1178,8 +1178,6 @@ impl ScheduleGraph {
} }
} }
} }
Ok(())
} }
/// Update the internal graphs (hierarchy, dependency, ambiguity) by adding a single [`GraphInfo`] /// Update the internal graphs (hierarchy, dependency, ambiguity) by adding a single [`GraphInfo`]
@ -1188,8 +1186,9 @@ impl ScheduleGraph {
id: NodeId, id: NodeId,
graph_info: GraphInfo, graph_info: GraphInfo,
) -> Result<(), ScheduleBuildError> { ) -> Result<(), ScheduleBuildError> {
self.check_hierarchy_sets(&id, &graph_info)?; self.check_hierarchy_sets(id, &graph_info)?;
self.check_edges(&id, &graph_info)?; self.check_edges(id, &graph_info)?;
self.add_ambiguities(&graph_info);
self.changed = true; self.changed = true;
let GraphInfo { let GraphInfo {
@ -1202,20 +1201,20 @@ impl ScheduleGraph {
self.hierarchy.graph.add_node(id); self.hierarchy.graph.add_node(id);
self.dependency.graph.add_node(id); self.dependency.graph.add_node(id);
for set in sets.into_iter().map(|set| self.system_set_ids[&set]) { for key in sets.into_iter().map(|set| self.system_set_ids[&set]) {
self.hierarchy.graph.add_edge(set, id); self.hierarchy.graph.add_edge(NodeId::Set(key), id);
// ensure set also appears in dependency graph // ensure set also appears in dependency graph
self.dependency.graph.add_node(set); self.dependency.graph.add_node(NodeId::Set(key));
} }
for (kind, set, options) in dependencies for (kind, key, options) in dependencies
.into_iter() .into_iter()
.map(|Dependency { kind, set, options }| (kind, self.system_set_ids[&set], options)) .map(|Dependency { kind, set, options }| (kind, self.system_set_ids[&set], options))
{ {
let (lhs, rhs) = match kind { let (lhs, rhs) = match kind {
DependencyKind::Before => (id, set), DependencyKind::Before => (id, NodeId::Set(key)),
DependencyKind::After => (set, id), DependencyKind::After => (NodeId::Set(key), id),
}; };
self.dependency.graph.add_edge(lhs, rhs); self.dependency.graph.add_edge(lhs, rhs);
for pass in self.passes.values_mut() { for pass in self.passes.values_mut() {
@ -1223,17 +1222,17 @@ impl ScheduleGraph {
} }
// ensure set also appears in hierarchy graph // ensure set also appears in hierarchy graph
self.hierarchy.graph.add_node(set); self.hierarchy.graph.add_node(NodeId::Set(key));
} }
match ambiguous_with { match ambiguous_with {
Ambiguity::Check => (), Ambiguity::Check => (),
Ambiguity::IgnoreWithSet(ambiguous_with) => { Ambiguity::IgnoreWithSet(ambiguous_with) => {
for set in ambiguous_with for key in ambiguous_with
.into_iter() .into_iter()
.map(|set| self.system_set_ids[&set]) .map(|set| self.system_set_ids[&set])
{ {
self.ambiguous_with.add_edge(id, set); self.ambiguous_with.add_edge(id, NodeId::Set(key));
} }
} }
Ambiguity::IgnoreAll => { Ambiguity::IgnoreAll => {
@ -1246,17 +1245,23 @@ impl ScheduleGraph {
/// Initializes any newly-added systems and conditions by calling [`System::initialize`](crate::system::System) /// Initializes any newly-added systems and conditions by calling [`System::initialize`](crate::system::System)
pub fn initialize(&mut self, world: &mut World) { pub fn initialize(&mut self, world: &mut World) {
for (id, i) in self.uninit.drain(..) { for id in self.uninit.drain(..) {
match id { match id {
NodeId::System(index) => { UninitializedId::System(key) => {
let system = self.systems[index].get_mut().unwrap(); let system = self.systems[key].get_mut().unwrap();
system.access = system.system.initialize(world); system.access = system.system.initialize(world);
for condition in &mut self.system_conditions[index] { for condition in &mut self.system_conditions[key] {
condition.access = condition.condition.initialize(world); condition.access = condition.condition.initialize(world);
} }
} }
NodeId::Set(index) => { UninitializedId::Set {
for condition in self.system_set_conditions[index].iter_mut().skip(i) { key,
first_uninit_condition,
} => {
for condition in self.system_set_conditions[key]
.iter_mut()
.skip(first_uninit_condition)
{
condition.access = condition.condition.initialize(world); condition.access = condition.condition.initialize(world);
} }
} }
@ -1348,41 +1353,47 @@ impl ScheduleGraph {
&self, &self,
hierarchy_topsort: &[NodeId], hierarchy_topsort: &[NodeId],
hierarchy_graph: &DiGraph, hierarchy_graph: &DiGraph,
) -> (HashMap<NodeId, Vec<NodeId>>, HashMap<NodeId, FixedBitSet>) { ) -> (
let mut set_systems: HashMap<NodeId, Vec<NodeId>> = HashMap<SystemSetKey, Vec<SystemKey>>,
HashMap<SystemSetKey, HashSet<SystemKey>>,
) {
let mut set_systems: HashMap<SystemSetKey, Vec<SystemKey>> =
HashMap::with_capacity_and_hasher(self.system_sets.len(), Default::default()); HashMap::with_capacity_and_hasher(self.system_sets.len(), Default::default());
let mut set_system_bitsets = let mut set_system_sets: HashMap<SystemSetKey, HashSet<SystemKey>> =
HashMap::with_capacity_and_hasher(self.system_sets.len(), Default::default()); HashMap::with_capacity_and_hasher(self.system_sets.len(), Default::default());
for &id in hierarchy_topsort.iter().rev() { for &id in hierarchy_topsort.iter().rev() {
if id.is_system() { let NodeId::Set(set_key) = id else {
continue; continue;
} };
let mut systems = Vec::new(); let mut systems = Vec::new();
let mut system_bitset = FixedBitSet::with_capacity(self.systems.len()); let mut system_set = HashSet::with_capacity(self.systems.len());
for child in hierarchy_graph.neighbors_directed(id, Outgoing) { for child in hierarchy_graph.neighbors_directed(id, Outgoing) {
match child { match child {
NodeId::System(_) => { NodeId::System(key) => {
systems.push(child); systems.push(key);
system_bitset.insert(child.index()); system_set.insert(key);
} }
NodeId::Set(_) => { NodeId::Set(key) => {
let child_systems = set_systems.get(&child).unwrap(); let child_systems = set_systems.get(&key).unwrap();
let child_system_bitset = set_system_bitsets.get(&child).unwrap(); let child_system_set = set_system_sets.get(&key).unwrap();
systems.extend_from_slice(child_systems); systems.extend_from_slice(child_systems);
system_bitset.union_with(child_system_bitset); system_set.extend(child_system_set.iter());
} }
} }
} }
set_systems.insert(id, systems); set_systems.insert(set_key, systems);
set_system_bitsets.insert(id, system_bitset); set_system_sets.insert(set_key, system_set);
} }
(set_systems, set_system_bitsets) (set_systems, set_system_sets)
} }
fn get_dependency_flattened(&mut self, set_systems: &HashMap<NodeId, Vec<NodeId>>) -> DiGraph { fn get_dependency_flattened(
&mut self,
set_systems: &HashMap<SystemSetKey, Vec<SystemKey>>,
) -> DiGraph {
// flatten: combine `in_set` with `before` and `after` information // flatten: combine `in_set` with `before` and `after` information
// have to do it like this to preserve transitivity // have to do it like this to preserve transitivity
let mut dependency_flattened = self.dependency.graph.clone(); let mut dependency_flattened = self.dependency.graph.clone();
@ -1393,26 +1404,26 @@ impl ScheduleGraph {
} }
if systems.is_empty() { if systems.is_empty() {
// collapse dependencies for empty sets // collapse dependencies for empty sets
for a in dependency_flattened.neighbors_directed(set, Incoming) { for a in dependency_flattened.neighbors_directed(NodeId::Set(set), Incoming) {
for b in dependency_flattened.neighbors_directed(set, Outgoing) { for b in dependency_flattened.neighbors_directed(NodeId::Set(set), Outgoing) {
temp.push((a, b)); temp.push((a, b));
} }
} }
} else { } else {
for a in dependency_flattened.neighbors_directed(set, Incoming) { for a in dependency_flattened.neighbors_directed(NodeId::Set(set), Incoming) {
for &sys in systems { for &sys in systems {
temp.push((a, sys)); temp.push((a, NodeId::System(sys)));
} }
} }
for b in dependency_flattened.neighbors_directed(set, Outgoing) { for b in dependency_flattened.neighbors_directed(NodeId::Set(set), Outgoing) {
for &sys in systems { for &sys in systems {
temp.push((sys, b)); temp.push((NodeId::System(sys), b));
} }
} }
} }
dependency_flattened.remove_node(set); dependency_flattened.remove_node(NodeId::Set(set));
for (a, b) in temp.drain(..) { for (a, b) in temp.drain(..) {
dependency_flattened.add_edge(a, b); dependency_flattened.add_edge(a, b);
} }
@ -1421,27 +1432,31 @@ impl ScheduleGraph {
dependency_flattened dependency_flattened
} }
fn get_ambiguous_with_flattened(&self, set_systems: &HashMap<NodeId, Vec<NodeId>>) -> UnGraph { fn get_ambiguous_with_flattened(
&self,
set_systems: &HashMap<SystemSetKey, Vec<SystemKey>>,
) -> UnGraph {
let mut ambiguous_with_flattened = UnGraph::default(); let mut ambiguous_with_flattened = UnGraph::default();
for (lhs, rhs) in self.ambiguous_with.all_edges() { for (lhs, rhs) in self.ambiguous_with.all_edges() {
match (lhs, rhs) { match (lhs, rhs) {
(NodeId::System(_), NodeId::System(_)) => { (NodeId::System(_), NodeId::System(_)) => {
ambiguous_with_flattened.add_edge(lhs, rhs); ambiguous_with_flattened.add_edge(lhs, rhs);
} }
(NodeId::Set(_), NodeId::System(_)) => { (NodeId::Set(lhs), NodeId::System(_)) => {
for &lhs_ in set_systems.get(&lhs).unwrap_or(&Vec::new()) { for &lhs_ in set_systems.get(&lhs).unwrap_or(&Vec::new()) {
ambiguous_with_flattened.add_edge(lhs_, rhs); ambiguous_with_flattened.add_edge(NodeId::System(lhs_), rhs);
} }
} }
(NodeId::System(_), NodeId::Set(_)) => { (NodeId::System(_), NodeId::Set(rhs)) => {
for &rhs_ in set_systems.get(&rhs).unwrap_or(&Vec::new()) { for &rhs_ in set_systems.get(&rhs).unwrap_or(&Vec::new()) {
ambiguous_with_flattened.add_edge(lhs, rhs_); ambiguous_with_flattened.add_edge(lhs, NodeId::System(rhs_));
} }
} }
(NodeId::Set(_), NodeId::Set(_)) => { (NodeId::Set(lhs), NodeId::Set(rhs)) => {
for &lhs_ in set_systems.get(&lhs).unwrap_or(&Vec::new()) { for &lhs_ in set_systems.get(&lhs).unwrap_or(&Vec::new()) {
for &rhs_ in set_systems.get(&rhs).unwrap_or(&vec![]) { for &rhs_ in set_systems.get(&rhs).unwrap_or(&vec![]) {
ambiguous_with_flattened.add_edge(lhs_, rhs_); ambiguous_with_flattened
.add_edge(NodeId::System(lhs_), NodeId::System(rhs_));
} }
} }
} }
@ -1456,7 +1471,7 @@ impl ScheduleGraph {
flat_results_disconnected: &Vec<(NodeId, NodeId)>, flat_results_disconnected: &Vec<(NodeId, NodeId)>,
ambiguous_with_flattened: &UnGraph, ambiguous_with_flattened: &UnGraph,
ignored_ambiguities: &BTreeSet<ComponentId>, ignored_ambiguities: &BTreeSet<ComponentId>,
) -> Vec<(NodeId, NodeId, Vec<ComponentId>)> { ) -> Vec<(SystemKey, SystemKey, Vec<ComponentId>)> {
let mut conflicting_systems = Vec::new(); let mut conflicting_systems = Vec::new();
for &(a, b) in flat_results_disconnected { for &(a, b) in flat_results_disconnected {
if ambiguous_with_flattened.contains_edge(a, b) if ambiguous_with_flattened.contains_edge(a, b)
@ -1466,8 +1481,18 @@ impl ScheduleGraph {
continue; continue;
} }
let system_a = self.systems[a.index()].get().unwrap(); let NodeId::System(a) = a else {
let system_b = self.systems[b.index()].get().unwrap(); panic!(
"Encountered a non-system node in the flattened disconnected results: {a:?}"
);
};
let NodeId::System(b) = b else {
panic!(
"Encountered a non-system node in the flattened disconnected results: {b:?}"
);
};
let system_a = self.systems[a].get().unwrap();
let system_b = self.systems[b].get().unwrap();
if system_a.system.is_exclusive() || system_b.system.is_exclusive() { if system_a.system.is_exclusive() || system_b.system.is_exclusive() {
conflicting_systems.push((a, b, Vec::new())); conflicting_systems.push((a, b, Vec::new()));
} else { } else {
@ -1503,7 +1528,11 @@ impl ScheduleGraph {
dependency_flattened_dag: Dag, dependency_flattened_dag: Dag,
hier_results_reachable: FixedBitSet, hier_results_reachable: FixedBitSet,
) -> SystemSchedule { ) -> SystemSchedule {
let dg_system_ids = dependency_flattened_dag.topsort.clone(); let dg_system_ids = dependency_flattened_dag
.topsort
.iter()
.filter_map(NodeId::as_system)
.collect::<Vec<_>>();
let dg_system_idx_map = dg_system_ids let dg_system_idx_map = dg_system_ids
.iter() .iter()
.cloned() .cloned()
@ -1517,7 +1546,7 @@ impl ScheduleGraph {
.iter() .iter()
.cloned() .cloned()
.enumerate() .enumerate()
.filter(|&(_i, id)| id.is_system()) .filter_map(|(i, id)| Some((i, id.as_system()?)))
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let (hg_set_with_conditions_idxs, hg_set_ids): (Vec<_>, Vec<_>) = self let (hg_set_with_conditions_idxs, hg_set_ids): (Vec<_>, Vec<_>) = self
@ -1526,10 +1555,11 @@ impl ScheduleGraph {
.iter() .iter()
.cloned() .cloned()
.enumerate() .enumerate()
.filter(|&(_i, id)| { .filter_map(|(i, id)| {
// ignore system sets that have no conditions // ignore system sets that have no conditions
// ignore system type sets (already covered, they don't have conditions) // ignore system type sets (already covered, they don't have conditions)
id.is_set() && !self.system_set_conditions[id.index()].is_empty() let key = id.as_set()?;
(!self.system_set_conditions[key].is_empty()).then_some((i, key))
}) })
.unzip(); .unzip();
@ -1541,16 +1571,19 @@ impl ScheduleGraph {
// (needed by multi_threaded executor to run systems in the correct order) // (needed by multi_threaded executor to run systems in the correct order)
let mut system_dependencies = Vec::with_capacity(sys_count); let mut system_dependencies = Vec::with_capacity(sys_count);
let mut system_dependents = Vec::with_capacity(sys_count); let mut system_dependents = Vec::with_capacity(sys_count);
for &sys_id in &dg_system_ids { for &sys_key in &dg_system_ids {
let num_dependencies = dependency_flattened_dag let num_dependencies = dependency_flattened_dag
.graph .graph
.neighbors_directed(sys_id, Incoming) .neighbors_directed(NodeId::System(sys_key), Incoming)
.count(); .count();
let dependents = dependency_flattened_dag let dependents = dependency_flattened_dag
.graph .graph
.neighbors_directed(sys_id, Outgoing) .neighbors_directed(NodeId::System(sys_key), Outgoing)
.map(|dep_id| dg_system_idx_map[&dep_id]) .filter_map(|dep_id| {
let dep_key = dep_id.as_system()?;
Some(dg_system_idx_map[&dep_key])
})
.collect::<Vec<_>>(); .collect::<Vec<_>>();
system_dependencies.push(num_dependencies); system_dependencies.push(num_dependencies);
@ -1563,8 +1596,8 @@ impl ScheduleGraph {
vec![FixedBitSet::with_capacity(sys_count); set_with_conditions_count]; vec![FixedBitSet::with_capacity(sys_count); set_with_conditions_count];
for (i, &row) in hg_set_with_conditions_idxs.iter().enumerate() { for (i, &row) in hg_set_with_conditions_idxs.iter().enumerate() {
let bitset = &mut systems_in_sets_with_conditions[i]; let bitset = &mut systems_in_sets_with_conditions[i];
for &(col, sys_id) in &hg_systems { for &(col, sys_key) in &hg_systems {
let idx = dg_system_idx_map[&sys_id]; let idx = dg_system_idx_map[&sys_key];
let is_descendant = hier_results_reachable[index(row, col, hg_node_count)]; let is_descendant = hier_results_reachable[index(row, col, hg_node_count)];
bitset.set(idx, is_descendant); bitset.set(idx, is_descendant);
} }
@ -1572,8 +1605,8 @@ impl ScheduleGraph {
let mut sets_with_conditions_of_systems = let mut sets_with_conditions_of_systems =
vec![FixedBitSet::with_capacity(set_with_conditions_count); sys_count]; vec![FixedBitSet::with_capacity(set_with_conditions_count); sys_count];
for &(col, sys_id) in &hg_systems { for &(col, sys_key) in &hg_systems {
let i = dg_system_idx_map[&sys_id]; let i = dg_system_idx_map[&sys_key];
let bitset = &mut sets_with_conditions_of_systems[i]; let bitset = &mut sets_with_conditions_of_systems[i];
for (idx, &row) in hg_set_with_conditions_idxs for (idx, &row) in hg_set_with_conditions_idxs
.iter() .iter()
@ -1611,36 +1644,36 @@ impl ScheduleGraph {
} }
// move systems out of old schedule // move systems out of old schedule
for ((id, system), conditions) in schedule for ((key, system), conditions) in schedule
.system_ids .system_ids
.drain(..) .drain(..)
.zip(schedule.systems.drain(..)) .zip(schedule.systems.drain(..))
.zip(schedule.system_conditions.drain(..)) .zip(schedule.system_conditions.drain(..))
{ {
self.systems[id.index()].inner = Some(system); self.systems[key].inner = Some(system);
self.system_conditions[id.index()] = conditions; self.system_conditions[key] = conditions;
} }
for (id, conditions) in schedule for (key, conditions) in schedule
.set_ids .set_ids
.drain(..) .drain(..)
.zip(schedule.set_conditions.drain(..)) .zip(schedule.set_conditions.drain(..))
{ {
self.system_set_conditions[id.index()] = conditions; self.system_set_conditions[key] = conditions;
} }
*schedule = self.build_schedule(world, schedule_label, ignored_ambiguities)?; *schedule = self.build_schedule(world, schedule_label, ignored_ambiguities)?;
// move systems into new schedule // move systems into new schedule
for &id in &schedule.system_ids { for &key in &schedule.system_ids {
let system = self.systems[id.index()].inner.take().unwrap(); let system = self.systems[key].inner.take().unwrap();
let conditions = core::mem::take(&mut self.system_conditions[id.index()]); let conditions = core::mem::take(&mut self.system_conditions[key]);
schedule.systems.push(system); schedule.systems.push(system);
schedule.system_conditions.push(conditions); schedule.system_conditions.push(conditions);
} }
for &id in &schedule.set_ids { for &key in &schedule.set_ids {
let conditions = core::mem::take(&mut self.system_set_conditions[id.index()]); let conditions = core::mem::take(&mut self.system_set_conditions[key]);
schedule.set_conditions.push(conditions); schedule.set_conditions.push(conditions);
} }
@ -1693,9 +1726,9 @@ impl ScheduleGraph {
#[inline] #[inline]
fn get_node_name_inner(&self, id: &NodeId, report_sets: bool) -> String { fn get_node_name_inner(&self, id: &NodeId, report_sets: bool) -> String {
match id { match *id {
NodeId::System(_) => { NodeId::System(key) => {
let name = self.systems[id.index()].get().unwrap().system.name(); let name = self.systems[key].get().unwrap().system.name();
let name = if self.settings.use_shortnames { let name = if self.settings.use_shortnames {
name.shortname().to_string() name.shortname().to_string()
} else { } else {
@ -1714,8 +1747,8 @@ impl ScheduleGraph {
name name
} }
} }
NodeId::Set(_) => { NodeId::Set(key) => {
let set = &self.system_sets[id.index()]; let set = &self.system_sets[key];
if set.is_anonymous() { if set.is_anonymous() {
self.anonymous_set_name(id) self.anonymous_set_name(id)
} else { } else {
@ -1903,16 +1936,16 @@ impl ScheduleGraph {
fn check_order_but_intersect( fn check_order_but_intersect(
&self, &self,
dep_results_connected: &HashSet<(NodeId, NodeId)>, dep_results_connected: &HashSet<(NodeId, NodeId)>,
set_system_bitsets: &HashMap<NodeId, FixedBitSet>, set_system_sets: &HashMap<SystemSetKey, HashSet<SystemKey>>,
) -> Result<(), ScheduleBuildError> { ) -> Result<(), ScheduleBuildError> {
// check that there is no ordering between system sets that intersect // check that there is no ordering between system sets that intersect
for (a, b) in dep_results_connected { for (a, b) in dep_results_connected {
if !(a.is_set() && b.is_set()) { let (NodeId::Set(a_key), NodeId::Set(b_key)) = (a, b) else {
continue; continue;
} };
let a_systems = set_system_bitsets.get(a).unwrap(); let a_systems = set_system_sets.get(a_key).unwrap();
let b_systems = set_system_bitsets.get(b).unwrap(); let b_systems = set_system_sets.get(b_key).unwrap();
if !a_systems.is_disjoint(b_systems) { if !a_systems.is_disjoint(b_systems) {
return Err(ScheduleBuildError::SetsHaveOrderButIntersect( return Err(ScheduleBuildError::SetsHaveOrderButIntersect(
@ -1927,19 +1960,25 @@ impl ScheduleGraph {
fn check_system_type_set_ambiguity( fn check_system_type_set_ambiguity(
&self, &self,
set_systems: &HashMap<NodeId, Vec<NodeId>>, set_systems: &HashMap<SystemSetKey, Vec<SystemKey>>,
) -> Result<(), ScheduleBuildError> { ) -> Result<(), ScheduleBuildError> {
for (&id, systems) in set_systems { for (&key, systems) in set_systems {
let set = &self.system_sets[id.index()]; let set = &self.system_sets[key];
if set.is_system_type() { if set.is_system_type() {
let instances = systems.len(); let instances = systems.len();
let ambiguous_with = self.ambiguous_with.edges(id); let ambiguous_with = self.ambiguous_with.edges(NodeId::Set(key));
let before = self.dependency.graph.edges_directed(id, Incoming); let before = self
let after = self.dependency.graph.edges_directed(id, Outgoing); .dependency
.graph
.edges_directed(NodeId::Set(key), Incoming);
let after = self
.dependency
.graph
.edges_directed(NodeId::Set(key), Outgoing);
let relations = before.count() + after.count() + ambiguous_with.count(); let relations = before.count() + after.count() + ambiguous_with.count();
if instances > 1 && relations > 0 { if instances > 1 && relations > 0 {
return Err(ScheduleBuildError::SystemTypeSetAmbiguity( return Err(ScheduleBuildError::SystemTypeSetAmbiguity(
self.get_node_name(&id), self.get_node_name(&NodeId::Set(key)),
)); ));
} }
} }
@ -1950,7 +1989,7 @@ impl ScheduleGraph {
/// if [`ScheduleBuildSettings::ambiguity_detection`] is [`LogLevel::Ignore`], this check is skipped /// if [`ScheduleBuildSettings::ambiguity_detection`] is [`LogLevel::Ignore`], this check is skipped
fn optionally_check_conflicts( fn optionally_check_conflicts(
&self, &self,
conflicts: &[(NodeId, NodeId, Vec<ComponentId>)], conflicts: &[(SystemKey, SystemKey, Vec<ComponentId>)],
components: &Components, components: &Components,
schedule_label: InternedScheduleLabel, schedule_label: InternedScheduleLabel,
) -> Result<(), ScheduleBuildError> { ) -> Result<(), ScheduleBuildError> {
@ -1971,7 +2010,7 @@ impl ScheduleGraph {
fn get_conflicts_error_message( fn get_conflicts_error_message(
&self, &self,
ambiguities: &[(NodeId, NodeId, Vec<ComponentId>)], ambiguities: &[(SystemKey, SystemKey, Vec<ComponentId>)],
components: &Components, components: &Components,
) -> String { ) -> String {
let n_ambiguities = ambiguities.len(); let n_ambiguities = ambiguities.len();
@ -1999,17 +2038,14 @@ impl ScheduleGraph {
/// convert conflicts to human readable format /// convert conflicts to human readable format
pub fn conflicts_to_string<'a>( pub fn conflicts_to_string<'a>(
&'a self, &'a self,
ambiguities: &'a [(NodeId, NodeId, Vec<ComponentId>)], ambiguities: &'a [(SystemKey, SystemKey, Vec<ComponentId>)],
components: &'a Components, components: &'a Components,
) -> impl Iterator<Item = (String, String, Vec<DebugName>)> + 'a { ) -> impl Iterator<Item = (String, String, Vec<DebugName>)> + 'a {
ambiguities ambiguities
.iter() .iter()
.map(move |(system_a, system_b, conflicts)| { .map(move |(system_a, system_b, conflicts)| {
let name_a = self.get_node_name(system_a); let name_a = self.get_node_name(&NodeId::System(*system_a));
let name_b = self.get_node_name(system_b); let name_b = self.get_node_name(&NodeId::System(*system_b));
debug_assert!(system_a.is_system(), "{name_a} is not a system.");
debug_assert!(system_b.is_system(), "{name_b} is not a system.");
let conflict_names: Vec<_> = conflicts let conflict_names: Vec<_> = conflicts
.iter() .iter()
@ -2020,22 +2056,25 @@ impl ScheduleGraph {
}) })
} }
fn traverse_sets_containing_node(&self, id: NodeId, f: &mut impl FnMut(NodeId) -> bool) { fn traverse_sets_containing_node(&self, id: NodeId, f: &mut impl FnMut(SystemSetKey) -> bool) {
for (set_id, _) in self.hierarchy.graph.edges_directed(id, Incoming) { for (set_id, _) in self.hierarchy.graph.edges_directed(id, Incoming) {
if f(set_id) { let NodeId::Set(set_key) = set_id else {
self.traverse_sets_containing_node(set_id, f); continue;
};
if f(set_key) {
self.traverse_sets_containing_node(NodeId::Set(set_key), f);
} }
} }
} }
fn names_of_sets_containing_node(&self, id: &NodeId) -> Vec<String> { fn names_of_sets_containing_node(&self, id: &NodeId) -> Vec<String> {
let mut sets = <HashSet<_>>::default(); let mut sets = <HashSet<_>>::default();
self.traverse_sets_containing_node(*id, &mut |set_id| { self.traverse_sets_containing_node(*id, &mut |key| {
!self.system_sets[set_id.index()].is_system_type() && sets.insert(set_id) !self.system_sets[key].is_system_type() && sets.insert(key)
}); });
let mut sets: Vec<_> = sets let mut sets: Vec<_> = sets
.into_iter() .into_iter()
.map(|set_id| self.get_node_name(&set_id)) .map(|key| self.get_node_name(&NodeId::Set(key)))
.collect(); .collect();
sets.sort(); sets.sort();
sets sets

35
crates/bevy_ecs/src/schedule/stepping.rs
View File

@ -1,6 +1,6 @@
use crate::{ use crate::{
resource::Resource, resource::Resource,
schedule::{InternedScheduleLabel, NodeId, Schedule, ScheduleLabel}, schedule::{InternedScheduleLabel, NodeId, Schedule, ScheduleLabel, SystemKey},
system::{IntoSystem, ResMut}, system::{IntoSystem, ResMut},
}; };
use alloc::vec::Vec; use alloc::vec::Vec;
@ -173,7 +173,7 @@ impl Stepping {
state state
.node_ids .node_ids
.get(self.cursor.system) .get(self.cursor.system)
.map(|node_id| (*label, *node_id)) .map(|node_id| (*label, NodeId::System(*node_id)))
} }
/// Enable stepping for the provided schedule /// Enable stepping for the provided schedule
@ -606,7 +606,7 @@ struct ScheduleState {
/// This is a cached copy of `SystemExecutable::system_ids`. We need it /// This is a cached copy of `SystemExecutable::system_ids`. We need it
/// available here to be accessed by [`Stepping::cursor()`] so we can return /// available here to be accessed by [`Stepping::cursor()`] so we can return
/// [`NodeId`]s to the caller. /// [`NodeId`]s to the caller.
node_ids: Vec<NodeId>, node_ids: Vec<SystemKey>,
/// changes to system behavior that should be applied the next time /// changes to system behavior that should be applied the next time
/// [`ScheduleState::skipped_systems()`] is called /// [`ScheduleState::skipped_systems()`] is called
@ -662,15 +662,15 @@ impl ScheduleState {
// updates for the system TypeId. // updates for the system TypeId.
// PERF: If we add a way to efficiently query schedule systems by their TypeId, we could remove the full // PERF: If we add a way to efficiently query schedule systems by their TypeId, we could remove the full
// system scan here // system scan here
for (node_id, system) in schedule.systems().unwrap() { for (key, system) in schedule.systems().unwrap() {
let behavior = self.behavior_updates.get(&system.type_id()); let behavior = self.behavior_updates.get(&system.type_id());
match behavior { match behavior {
None => continue, None => continue,
Some(None) => { Some(None) => {
self.behaviors.remove(&node_id); self.behaviors.remove(&NodeId::System(key));
} }
Some(Some(behavior)) => { Some(Some(behavior)) => {
self.behaviors.insert(node_id, *behavior); self.behaviors.insert(NodeId::System(key), *behavior);
} }
} }
} }
@ -703,8 +703,8 @@ impl ScheduleState {
// if we don't have a first system set, set it now // if we don't have a first system set, set it now
if self.first.is_none() { if self.first.is_none() {
for (i, (node_id, _)) in schedule.systems().unwrap().enumerate() { for (i, (key, _)) in schedule.systems().unwrap().enumerate() {
match self.behaviors.get(&node_id) { match self.behaviors.get(&NodeId::System(key)) {
Some(SystemBehavior::AlwaysRun | SystemBehavior::NeverRun) => continue, Some(SystemBehavior::AlwaysRun | SystemBehavior::NeverRun) => continue,
Some(_) | None => { Some(_) | None => {
self.first = Some(i); self.first = Some(i);
@ -717,10 +717,10 @@ impl ScheduleState {
let mut skip = FixedBitSet::with_capacity(schedule.systems_len()); let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
let mut pos = start; let mut pos = start;
for (i, (node_id, _system)) in schedule.systems().unwrap().enumerate() { for (i, (key, _system)) in schedule.systems().unwrap().enumerate() {
let behavior = self let behavior = self
.behaviors .behaviors
.get(&node_id) .get(&NodeId::System(key))
.unwrap_or(&SystemBehavior::Continue); .unwrap_or(&SystemBehavior::Continue);
#[cfg(test)] #[cfg(test)]
@ -825,6 +825,7 @@ mod tests {
use super::*; use super::*;
use crate::{prelude::*, schedule::ScheduleLabel}; use crate::{prelude::*, schedule::ScheduleLabel};
use alloc::{format, vec}; use alloc::{format, vec};
use slotmap::SlotMap;
use std::println; use std::println;
#[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)] #[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)]
@ -1467,10 +1468,11 @@ mod tests {
// helper to build a cursor tuple for the supplied schedule // helper to build a cursor tuple for the supplied schedule
fn cursor(schedule: &Schedule, index: usize) -> (InternedScheduleLabel, NodeId) { fn cursor(schedule: &Schedule, index: usize) -> (InternedScheduleLabel, NodeId) {
let node_id = schedule.executable().system_ids[index]; let node_id = schedule.executable().system_ids[index];
(schedule.label(), node_id) (schedule.label(), NodeId::System(node_id))
} }
let mut world = World::new(); let mut world = World::new();
let mut slotmap = SlotMap::<SystemKey, ()>::with_key();
// create two schedules with a number of systems in them // create two schedules with a number of systems in them
let mut schedule_a = Schedule::new(TestScheduleA); let mut schedule_a = Schedule::new(TestScheduleA);
@ -1517,6 +1519,11 @@ mod tests {
] ]
); );
let sys0 = slotmap.insert(());
let sys1 = slotmap.insert(());
let _sys2 = slotmap.insert(());
let sys3 = slotmap.insert(());
// reset our cursor (disable/enable), and update stepping to test if the // reset our cursor (disable/enable), and update stepping to test if the
// cursor properly skips over AlwaysRun & NeverRun systems. Also set // cursor properly skips over AlwaysRun & NeverRun systems. Also set
// a Break system to ensure that shows properly in the cursor // a Break system to ensure that shows properly in the cursor
@ -1524,9 +1531,9 @@ mod tests {
// disable/enable to reset cursor // disable/enable to reset cursor
.disable() .disable()
.enable() .enable()
.set_breakpoint_node(TestScheduleA, NodeId::System(1)) .set_breakpoint_node(TestScheduleA, NodeId::System(sys1))
.always_run_node(TestScheduleA, NodeId::System(3)) .always_run_node(TestScheduleA, NodeId::System(sys3))
.never_run_node(TestScheduleB, NodeId::System(0)); .never_run_node(TestScheduleB, NodeId::System(sys0));
let mut cursors = Vec::new(); let mut cursors = Vec::new();
for _ in 0..9 { for _ in 0..9 {

8
examples/games/stepping.rs
View File

@ -132,18 +132,20 @@ fn build_ui(
return; return;
}; };
for (node_id, system) in systems { for (key, system) in systems {
// skip bevy default systems; we don't want to step those // skip bevy default systems; we don't want to step those
#[cfg(feature = "debug")] #[cfg(feature = "debug")]
if system.name().as_string().starts_with("bevy") { if system.name().as_string().starts_with("bevy") {
always_run.push((*label, node_id)); always_run.push((*label, NodeId::System(key)));
continue; continue;
} }
// Add an entry to our systems list so we can find where to draw // Add an entry to our systems list so we can find where to draw
// the cursor when the stepping cursor is at this system // the cursor when the stepping cursor is at this system
// we add plus 1 to account for the empty root span // we add plus 1 to account for the empty root span
state.systems.push((*label, node_id, text_spans.len() + 1)); state
.systems
.push((*label, NodeId::System(key), text_spans.len() + 1));
// Add a text section for displaying the cursor for this system // Add a text section for displaying the cursor for this system
text_spans.push(( text_spans.push((

34
release-content/migration-guides/schedule_slotmaps.md Normal file
View File

@ -0,0 +1,34 @@
---
title: Schedule SlotMaps
pull_requests: [19352]
---
In order to support removing systems from schedules, `Vec`s storing `System`s and
`SystemSet`s have been replaced with `SlotMap`s which allow safely removing and
reusing indices. The maps are respectively keyed by `SystemKey`s and `SystemSetKey`s.
The following signatures were changed:
- `NodeId::System`: Now stores a `SystemKey` instead of a plain `usize`
- `NodeId::Set`: Now stores a `SystemSetKey` instead of a plain `usize`
- `ScheduleBuildPass::collapse_set`: Now takes the type-specific keys. Wrap them back into a `NodeId` if necessary.
- The following functions now return the type-specific keys. Wrap them back into a `NodeId` if necessary.
- `Schedule::systems`
- `ScheduleGraph::systems`
- `ScheduleGraph::system_sets`
- `ScheduleGraph::conflicting_systems`
- Use the appropriate key types to index these structures rather than bare `usize`s:
- `ScheduleGraph::systems` field
- `ScheduleGraph::system_conditions`
- The following functions now take the type-specific keys. Use pattern matching to extract them from `NodeId`s, if necessary:
- `ScheduleGraph::get_system_at`
- `ScheduleGraph::system_at`
- `ScheduleGraph::get_set_at`
- `ScheduleGraph::set_at`
- `ScheduleGraph::get_set_conditions_at`
- `ScheduleGraph::set_conditions_at`
The following functions were removed:
- `NodeId::index`: You should match on and use the `SystemKey` and `SystemSetKey` instead.
- `NodeId::cmp`: Use the `PartialOrd` and `Ord` traits instead.