diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index 28987f1413449..e969e2db15167 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -124,6 +124,7 @@ variadics_please = { version = "1.1", default-features = false } tracing = { version = "0.1", default-features = false, optional = true } log = { version = "0.4", default-features = false } bumpalo = "3" +slotmap = { version = "1.0.7", 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] 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 dda6d604a74b2..d9e4fe4654867 100644 --- a/crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs +++ b/crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs @@ -2,8 +2,9 @@ use alloc::{boxed::Box, collections::BTreeSet, vec::Vec}; use bevy_platform::collections::HashMap; -use crate::system::IntoSystem; +use crate::schedule::SystemSetKey; use crate::world::World; +use crate::{schedule::SystemKey, system::IntoSystem}; use super::{ is_apply_deferred, ApplyDeferred, DiGraph, Direction, NodeId, ReportCycles, ScheduleBuildError, @@ -37,28 +38,26 @@ impl AutoInsertApplyDeferredPass { .get(&distance) .copied() .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); Some(node_id) }) .unwrap() } /// add an [`ApplyDeferred`] system with no config - fn add_auto_sync(&mut self, graph: &mut ScheduleGraph) -> NodeId { - let id = NodeId::System(graph.systems.len()); - - graph + fn add_auto_sync(&mut self, graph: &mut ScheduleGraph) -> SystemKey { + let key = graph .systems - .push(SystemNode::new(Box::new(IntoSystem::into_system( + .insert(SystemNode::new(Box::new(IntoSystem::into_system( ApplyDeferred, )))); - graph.system_conditions.push(Vec::new()); + graph.system_conditions.insert(key, Vec::new()); // ignore ambiguities with auto sync points // 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 +79,45 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass { let mut sync_point_graph = dependency_flattened.clone(); let topo = graph.topsort_graph(dependency_flattened, ReportCycles::Dependency)?; - fn set_has_conditions(graph: &ScheduleGraph, node: NodeId) -> bool { - !graph.set_conditions_at(node).is_empty() + fn set_has_conditions(graph: &ScheduleGraph, set: SystemSetKey) -> bool { + !graph.set_conditions_at(set).is_empty() || graph .hierarchy() .graph() - .edges_directed(node, Direction::Incoming) - .any(|(parent, _)| set_has_conditions(graph, parent)) + .edges_directed(NodeId::Set(set), Direction::Incoming) + .any(|(parent, _)| { + parent + .as_set() + .is_some_and(|p| set_has_conditions(graph, p)) + }) } - fn system_has_conditions(graph: &ScheduleGraph, node: NodeId) -> bool { - assert!(node.is_system()); - !graph.system_conditions[node.index()].is_empty() + fn system_has_conditions(graph: &ScheduleGraph, key: SystemKey) -> bool { + !graph.system_conditions[key].is_empty() || graph .hierarchy() .graph() - .edges_directed(node, Direction::Incoming) - .any(|(parent, _)| set_has_conditions(graph, parent)) + .edges_directed(NodeId::System(key), Direction::Incoming) + .any(|(parent, _)| { + parent + .as_set() + .is_some_and(|p| set_has_conditions(graph, p)) + }) } - let mut system_has_conditions_cache = HashMap::::default(); - let mut is_valid_explicit_sync_point = |system: NodeId| { - let index = system.index(); - is_apply_deferred(graph.systems[index].get().unwrap()) + let mut system_has_conditions_cache = HashMap::::default(); + let mut is_valid_explicit_sync_point = |key: SystemKey| { + is_apply_deferred(graph.systems[key].get().unwrap()) && !*system_has_conditions_cache - .entry(index) - .or_insert_with(|| system_has_conditions(graph, system)) + .entry(key) + .or_insert_with(|| system_has_conditions(graph, key)) }; // Calculate the distance for each node. // 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 // a later edge that is not ignored. - let mut distances_and_pending_sync: HashMap = + let mut distances_and_pending_sync: HashMap = HashMap::with_capacity_and_hasher(topo.len(), Default::default()); // Keep track of any explicit sync nodes for a specific distance. @@ -120,17 +125,21 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass { // Determine the distance for every node and collect the explicit sync points. for node in &topo { + let &NodeId::System(key) = node else { + continue; + }; + let (node_distance, mut node_needs_sync) = distances_and_pending_sync - .get(&node.index()) + .get(&key) .copied() .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 // 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 // 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 // explicitly scheduled afterwards. @@ -138,18 +147,22 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass { } else if !node_needs_sync { // 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. - node_needs_sync = graph.systems[node.index()].get().unwrap().has_deferred(); + node_needs_sync = graph.systems[key].get().unwrap().has_deferred(); } for target in dependency_flattened.neighbors_directed(*node, Direction::Outgoing) { - let (target_distance, target_pending_sync) = distances_and_pending_sync - .entry(target.index()) - .or_default(); + let NodeId::System(target) = target else { + continue; + }; + let (target_distance, target_pending_sync) = + distances_and_pending_sync.entry(target).or_default(); let mut edge_needs_sync = node_needs_sync; if node_needs_sync - && !graph.systems[target.index()].get().unwrap().is_exclusive() - && self.no_sync_edges.contains(&(*node, target)) + && !graph.systems[target].get().unwrap().is_exclusive() + && self + .no_sync_edges + .contains(&(*node, NodeId::System(target))) { // 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 @@ -174,14 +187,20 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass { // Find any edges which have a different number of sync points between them and make sure // there is a sync point between them. for node in &topo { + let &NodeId::System(key) = node else { + continue; + }; let (node_distance, _) = distances_and_pending_sync - .get(&node.index()) + .get(&key) .copied() .unwrap_or_default(); for target in dependency_flattened.neighbors_directed(*node, Direction::Outgoing) { + let NodeId::System(target) = target else { + continue; + }; let (target_distance, _) = distances_and_pending_sync - .get(&target.index()) + .get(&target) .copied() .unwrap_or_default(); @@ -190,7 +209,7 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass { continue; } - if is_apply_deferred(graph.systems[target.index()].get().unwrap()) { + if is_apply_deferred(graph.systems[target].get().unwrap()) { // We don't need to insert a sync point since ApplyDeferred is a sync point // already! continue; @@ -202,10 +221,10 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass { .unwrap_or_else(|| self.get_sync_point(graph, target_distance)); 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. - sync_point_graph.remove_edge(*node, target); + sync_point_graph.remove_edge(*node, NodeId::System(target)); } } @@ -215,34 +234,39 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass { fn collapse_set( &mut self, - set: NodeId, - systems: &[NodeId], + set: SystemSetKey, + systems: &[SystemKey], dependency_flattened: &DiGraph, ) -> impl Iterator { if systems.is_empty() { // collapse dependencies for empty sets - for a in dependency_flattened.neighbors_directed(set, Direction::Incoming) { - for b in dependency_flattened.neighbors_directed(set, Direction::Outgoing) { - if self.no_sync_edges.contains(&(a, set)) - && self.no_sync_edges.contains(&(set, b)) + for a in dependency_flattened.neighbors_directed(NodeId::Set(set), Direction::Incoming) + { + for b in + 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)); } } } } 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 { - if self.no_sync_edges.contains(&(a, set)) { - self.no_sync_edges.insert((a, sys)); + if self.no_sync_edges.contains(&(a, NodeId::Set(set))) { + 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 { - if self.no_sync_edges.contains(&(set, b)) { - self.no_sync_edges.insert((sys, b)); + if self.no_sync_edges.contains(&(NodeId::Set(set), b)) { + self.no_sync_edges.insert((NodeId::System(sys), b)); } } } diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index a601284fb005a..acfc424666563 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -20,7 +20,7 @@ use crate::{ error::{BevyError, ErrorContext, Result}, prelude::{IntoSystemSet, SystemSet}, query::{Access, FilteredAccessSet}, - schedule::{BoxedCondition, InternedSystemSet, NodeId, SystemTypeSet}, + schedule::{BoxedCondition, InternedSystemSet, SystemKey, SystemSetKey, SystemTypeSet}, system::{ScheduleSystem, System, SystemIn, SystemParamValidationError}, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, }; @@ -73,7 +73,7 @@ pub enum ExecutorKind { #[derive(Default)] pub struct SystemSchedule { /// List of system node ids. - pub(super) system_ids: Vec, + pub(super) system_ids: Vec, /// Indexed by system node id. pub(super) systems: Vec, /// Indexed by system node id. @@ -96,7 +96,7 @@ pub struct SystemSchedule { /// List of sets containing the system that have conditions pub(super) sets_with_conditions_of_systems: Vec, /// List of system set node ids. - pub(super) set_ids: Vec, + pub(super) set_ids: Vec, /// Indexed by system set node id. pub(super) set_conditions: Vec>, /// Indexed by system set node id. diff --git a/crates/bevy_ecs/src/schedule/graph/graph_map.rs b/crates/bevy_ecs/src/schedule/graph/graph_map.rs index d2fbde9995071..9224efe276560 100644 --- a/crates/bevy_ecs/src/schedule/graph/graph_map.rs +++ b/crates/bevy_ecs/src/schedule/graph/graph_map.rs @@ -11,6 +11,7 @@ use core::{ hash::{BuildHasher, Hash}, }; use indexmap::IndexMap; +use slotmap::{Key, KeyData}; use smallvec::SmallVec; use super::NodeId; @@ -298,7 +299,7 @@ impl Direction { /// Compact storage of a [`NodeId`] and a [`Direction`]. #[derive(Clone, Copy)] struct CompactNodeIdAndDirection { - index: usize, + key: KeyData, is_system: bool, direction: Direction, } @@ -310,27 +311,30 @@ impl fmt::Debug for CompactNodeIdAndDirection { } impl CompactNodeIdAndDirection { - const fn store(node: NodeId, direction: Direction) -> Self { - let index = node.index(); + fn store(node: NodeId, direction: Direction) -> Self { + let key = match node { + NodeId::System(key) => key.data(), + NodeId::Set(key) => key.data(), + }; let is_system = node.is_system(); Self { - index, + key, is_system, direction, } } - const fn load(self) -> (NodeId, Direction) { + fn load(self) -> (NodeId, Direction) { let Self { - index, + key, is_system, direction, } = self; let node = match is_system { - true => NodeId::System(index), - false => NodeId::Set(index), + true => NodeId::System(key.into()), + false => NodeId::Set(key.into()), }; (node, direction) @@ -340,8 +344,8 @@ impl CompactNodeIdAndDirection { /// Compact storage of a [`NodeId`] pair. #[derive(Clone, Copy, Hash, PartialEq, Eq)] struct CompactNodeIdPair { - index_a: usize, - index_b: usize, + key_a: KeyData, + key_b: KeyData, is_system_a: bool, is_system_b: bool, } @@ -353,37 +357,43 @@ impl fmt::Debug for CompactNodeIdPair { } impl CompactNodeIdPair { - const fn store(a: NodeId, b: NodeId) -> Self { - let index_a = a.index(); + fn store(a: NodeId, b: NodeId) -> Self { + let key_a = match a { + NodeId::System(index) => index.data(), + NodeId::Set(index) => index.data(), + }; 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(); Self { - index_a, - index_b, + key_a, + key_b, is_system_a, is_system_b, } } - const fn load(self) -> (NodeId, NodeId) { + fn load(self) -> (NodeId, NodeId) { let Self { - index_a, - index_b, + key_a, + key_b, is_system_a, is_system_b, } = self; let a = match is_system_a { - true => NodeId::System(index_a), - false => NodeId::Set(index_a), + true => NodeId::System(key_a.into()), + false => NodeId::Set(key_a.into()), }; let b = match is_system_b { - true => NodeId::System(index_b), - false => NodeId::Set(index_b), + true => NodeId::System(key_b.into()), + false => NodeId::Set(key_b.into()), }; (a, b) @@ -392,8 +402,11 @@ impl CompactNodeIdPair { #[cfg(test)] mod tests { + use crate::schedule::SystemKey; + use super::*; use alloc::vec; + use slotmap::SlotMap; /// 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 @@ -402,37 +415,43 @@ mod tests { fn node_order_preservation() { use NodeId::System; + let mut slotmap = SlotMap::::with_key(); let mut graph = ::default(); - graph.add_node(System(1)); - graph.add_node(System(2)); - graph.add_node(System(3)); - graph.add_node(System(4)); + let sys1 = slotmap.insert(()); + let sys2 = slotmap.insert(()); + let sys3 = slotmap.insert(()); + 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!( graph.nodes().collect::>(), - 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!( graph.nodes().collect::>(), - 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!( graph.nodes().collect::>(), - 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![System(3)]); + assert_eq!(graph.nodes().collect::>(), vec![System(sys3)]); - graph.remove_node(System(3)); + graph.remove_node(System(sys3)); assert_eq!(graph.nodes().collect::>(), vec![]); } @@ -444,18 +463,26 @@ mod tests { fn strongly_connected_components() { use NodeId::System; + let mut slotmap = SlotMap::::with_key(); let mut graph = ::default(); - graph.add_edge(System(1), System(2)); - graph.add_edge(System(2), System(1)); + let sys1 = slotmap.insert(()); + let sys2 = slotmap.insert(()); + let sys3 = slotmap.insert(()); + let sys4 = slotmap.insert(()); + let sys5 = slotmap.insert(()); + let sys6 = slotmap.insert(()); + + graph.add_edge(System(sys1), System(sys2)); + graph.add_edge(System(sys2), System(sys1)); - graph.add_edge(System(2), System(3)); - graph.add_edge(System(3), System(2)); + graph.add_edge(System(sys2), System(sys3)); + graph.add_edge(System(sys3), System(sys2)); - graph.add_edge(System(4), System(5)); - graph.add_edge(System(5), System(4)); + graph.add_edge(System(sys4), System(sys5)); + graph.add_edge(System(sys5), System(sys4)); - graph.add_edge(System(6), System(2)); + graph.add_edge(System(sys6), System(sys2)); let sccs = graph .iter_sccs() @@ -465,9 +492,9 @@ mod tests { assert_eq!( sccs, vec![ - vec![System(3), System(2), System(1)], - vec![System(5), System(4)], - vec![System(6)] + vec![System(sys3), System(sys2), System(sys1)], + vec![System(sys5), System(sys4)], + vec![System(sys6)] ] ); } diff --git a/crates/bevy_ecs/src/schedule/graph/node.rs b/crates/bevy_ecs/src/schedule/graph/node.rs index e4af143fc7a78..40f4f53988853 100644 --- a/crates/bevy_ecs/src/schedule/graph/node.rs +++ b/crates/bevy_ecs/src/schedule/graph/node.rs @@ -1,24 +1,19 @@ use core::fmt::Debug; +use crate::schedule::{SystemKey, SystemSetKey}; + /// Unique identifier for a system or system set stored in a [`ScheduleGraph`]. /// /// [`ScheduleGraph`]: crate::schedule::ScheduleGraph #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] pub enum NodeId { /// Identifier for a system. - System(usize), + System(SystemKey), /// Identifier for a system set. - Set(usize), + Set(SystemSetKey), } 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. pub const fn is_system(&self) -> bool { matches!(self, NodeId::System(_)) @@ -29,19 +24,19 @@ impl NodeId { matches!(self, NodeId::Set(_)) } - /// Compare this [`NodeId`] with another. - pub const fn cmp(&self, other: &Self) -> core::cmp::Ordering { - use core::cmp::Ordering::{Equal, Greater, Less}; - use NodeId::{Set, System}; + /// Returns the system key if the node is a system, otherwise `None`. + pub const fn as_system(&self) -> Option { + match self { + NodeId::System(system) => Some(*system), + NodeId::Set(_) => None, + } + } - match (self, other) { - (System(a), System(b)) | (Set(a), Set(b)) => match a.checked_sub(*b) { - None => Less, - Some(0) => Equal, - Some(_) => Greater, - }, - (System(_), Set(_)) => Less, - (Set(_), System(_)) => Greater, + /// Returns the system set key if the node is a system set, otherwise `None`. + pub const fn as_set(&self) -> Option { + match self { + NodeId::System(_) => None, + NodeId::Set(set) => Some(*set), } } } diff --git a/crates/bevy_ecs/src/schedule/pass.rs b/crates/bevy_ecs/src/schedule/pass.rs index 20680e04e032c..9f49cc4e220ab 100644 --- a/crates/bevy_ecs/src/schedule/pass.rs +++ b/crates/bevy_ecs/src/schedule/pass.rs @@ -2,7 +2,10 @@ use alloc::{boxed::Box, vec::Vec}; use core::any::{Any, TypeId}; use super::{DiGraph, NodeId, ScheduleBuildError, ScheduleGraph}; -use crate::world::World; +use crate::{ + schedule::{SystemKey, SystemSetKey}, + world::World, +}; use bevy_utils::TypeIdMap; 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. fn collapse_set( &mut self, - set: NodeId, - systems: &[NodeId], + set: SystemSetKey, + systems: &[SystemKey], dependency_flattened: &DiGraph, ) -> impl Iterator; @@ -44,8 +47,8 @@ pub(super) trait ScheduleBuildPassObj: Send + Sync + Debug { fn collapse_set( &mut self, - set: NodeId, - systems: &[NodeId], + set: SystemSetKey, + systems: &[SystemKey], dependency_flattened: &DiGraph, dependencies_to_add: &mut Vec<(NodeId, NodeId)>, ); @@ -62,8 +65,8 @@ impl ScheduleBuildPassObj for T { } fn collapse_set( &mut self, - set: NodeId, - systems: &[NodeId], + set: SystemSetKey, + systems: &[SystemKey], dependency_flattened: &DiGraph, dependencies_to_add: &mut Vec<(NodeId, NodeId)>, ) { diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 667464ea8fae1..00ff396e0fd2e 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -21,6 +21,7 @@ use disqualified::ShortName; use fixedbitset::FixedBitSet; use log::{error, info, warn}; use pass::ScheduleBuildPassObj; +use slotmap::{new_key_type, SecondaryMap, SlotMap}; use thiserror::Error; #[cfg(feature = "trace")] use tracing::info_span; @@ -403,7 +404,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 } @@ -586,7 +589,7 @@ impl Schedule { /// schedule has never been initialized or run. pub fn systems( &self, - ) -> Result + Sized, ScheduleNotInitialized> + ) -> Result + Sized, ScheduleNotInitialized> { if !self.executor_initialized { return Err(ScheduleNotInitialized); @@ -597,7 +600,7 @@ impl Schedule { .system_ids .iter() .zip(&self.executable.systems) - .map(|(node_id, system)| (*node_id, system)); + .map(|(&node_id, system)| (node_id, system)); Ok(iter) } @@ -689,6 +692,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`]. /// /// The order isn't optimized; calling `ScheduleGraph::build_schedule` will return a @@ -696,18 +714,18 @@ impl SystemNode { #[derive(Default)] pub struct ScheduleGraph { /// List of systems in the schedule - pub systems: Vec, + pub systems: SlotMap, /// List of conditions for each system, in the same order as `systems` - pub system_conditions: Vec>, + pub system_conditions: SecondaryMap>, /// List of system sets in the schedule - system_sets: Vec, + system_sets: SlotMap, /// List of conditions for each system set, in the same order as `system_sets` - system_set_conditions: Vec>, + system_set_conditions: SecondaryMap>, /// Map from system set to node id - system_set_ids: HashMap, + system_set_ids: HashMap, /// 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) - uninit: Vec<(NodeId, usize)>, + uninit: Vec, /// Directed acyclic graph of the hierarchy (which systems/sets are children of which sets) hierarchy: Dag, /// Directed acyclic graph of the dependency (which systems/sets have to run before which other systems/sets) @@ -715,7 +733,7 @@ pub struct ScheduleGraph { ambiguous_with: UnGraph, /// Nodes that are allowed to have ambiguous ordering relationship with any other systems. pub ambiguous_with_all: HashSet, - conflicting_systems: Vec<(NodeId, NodeId, Vec)>, + conflicting_systems: Vec<(SystemKey, SystemKey, Vec)>, anonymous_sets: usize, changed: bool, settings: ScheduleBuildSettings, @@ -727,10 +745,10 @@ impl ScheduleGraph { /// Creates an empty [`ScheduleGraph`] with default settings. pub fn new() -> Self { Self { - systems: Vec::new(), - system_conditions: Vec::new(), - system_sets: Vec::new(), - system_set_conditions: Vec::new(), + systems: SlotMap::with_key(), + system_conditions: SecondaryMap::new(), + system_sets: SlotMap::with_key(), + system_set_conditions: SecondaryMap::new(), system_set_ids: HashMap::default(), uninit: Vec::new(), hierarchy: Dag::new(), @@ -745,14 +763,9 @@ impl ScheduleGraph { } } - /// Returns the system at the given [`NodeId`], if it exists. - pub fn get_system_at(&self, id: NodeId) -> Option<&ScheduleSystem> { - if !id.is_system() { - return None; - } - self.systems - .get(id.index()) - .and_then(|system| system.inner.as_ref()) + /// Returns the system at the given [`SystemKey`], if it exists. + pub fn get_system_at(&self, key: SystemKey) -> Option<&ScheduleSystem> { + self.systems.get(key).and_then(|system| system.get()) } /// Returns `true` if the given system set is part of the graph. Otherwise, returns `false`. @@ -764,70 +777,58 @@ impl ScheduleGraph { /// /// Panics if it doesn't exist. #[track_caller] - pub fn system_at(&self, id: NodeId) -> &ScheduleSystem { - self.get_system_at(id) - .ok_or_else(|| format!("system with id {id:?} does not exist in this Schedule")) - .unwrap() + pub fn system_at(&self, key: SystemKey) -> &ScheduleSystem { + self.get_system_at(key) + .unwrap_or_else(|| panic!("system with key {key:?} does not exist in this Schedule")) } /// Returns the set at the given [`NodeId`], if it exists. - pub fn get_set_at(&self, id: NodeId) -> Option<&dyn SystemSet> { - if !id.is_set() { - return None; - } - self.system_sets.get(id.index()).map(|set| &*set.inner) + pub fn get_set_at(&self, key: SystemSetKey) -> Option<&dyn SystemSet> { + self.system_sets.get(key).map(|set| &*set.inner) } /// Returns the set at the given [`NodeId`]. /// /// Panics if it doesn't exist. #[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) - .ok_or_else(|| format!("set with id {id:?} does not exist in this Schedule")) - .unwrap() + .unwrap_or_else(|| panic!("set with id {id:?} does not exist in this Schedule")) } - /// Returns the conditions for the set at the given [`NodeId`], if it exists. - pub fn get_set_conditions_at(&self, id: NodeId) -> Option<&[BoxedCondition]> { - if !id.is_set() { - return None; - } - self.system_set_conditions - .get(id.index()) - .map(Vec::as_slice) + /// Returns the conditions for the set at the given [`SystemSetKey`], if it exists. + pub fn get_set_conditions_at(&self, key: SystemSetKey) -> Option<&[BoxedCondition]> { + self.system_set_conditions.get(key).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. #[track_caller] - pub fn set_conditions_at(&self, id: NodeId) -> &[BoxedCondition] { - self.get_set_conditions_at(id) - .ok_or_else(|| format!("set with id {id:?} does not exist in this Schedule")) - .unwrap() + pub fn set_conditions_at(&self, key: SystemSetKey) -> &[BoxedCondition] { + self.get_set_conditions_at(key) + .unwrap_or_else(|| panic!("set with key {key:?} does not exist in this Schedule")) } /// Returns an iterator over all systems in this schedule, along with the conditions for each system. - pub fn systems(&self) -> impl Iterator { - self.systems - .iter() - .zip(self.system_conditions.iter()) - .enumerate() - .filter_map(|(i, (system_node, condition))| { - let system = system_node.inner.as_ref()?; - Some((NodeId::System(i), system, condition.as_slice())) - }) + pub fn systems(&self) -> impl Iterator { + self.systems.iter().filter_map(|(key, system_node)| { + let system = system_node.inner.as_ref()?; + let conditions = self.system_conditions.get(key)?; + Some((key, system, conditions.as_slice())) + }) } /// Returns an iterator over all system sets in this schedule, along with the conditions for each /// system set. - pub fn system_sets(&self) -> impl Iterator { - self.system_set_ids.iter().map(|(_, &node_id)| { - let set_node = &self.system_sets[node_id.index()]; + pub fn system_sets( + &self, + ) -> impl Iterator { + self.system_set_ids.iter().filter_map(|(_, &key)| { + let set_node = self.system_sets.get(key)?; let set = &*set_node.inner; - let conditions = self.system_set_conditions[node_id.index()].as_slice(); - (node_id, set, conditions) + let conditions = self.system_set_conditions.get(key)?.as_slice(); + Some((key, set, conditions)) }) } @@ -851,7 +852,7 @@ impl ScheduleGraph { /// /// If the `Vec` is empty, the systems conflict on [`World`] access. /// Must be called after [`ScheduleGraph::build_schedule`] to be non-empty. - pub fn conflicting_systems(&self) -> &[(NodeId, NodeId, Vec)] { + pub fn conflicting_systems(&self) -> &[(SystemKey, SystemKey, Vec)] { &self.conflicting_systems } @@ -993,17 +994,15 @@ impl ScheduleGraph { &mut self, config: ScheduleConfig, ) -> Result { - let id = NodeId::System(self.systems.len()); + let key = self.systems.insert(SystemNode::new(config.node)); + self.system_conditions.insert(key, config.conditions); + // system init has to be deferred (need `&mut World`) + self.uninit.push(UninitializedId::System(key)); // graph updates are immediate - 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.conditions); + self.update_graphs(NodeId::System(key), config.metadata)?; - Ok(id) + Ok(NodeId::System(key)) } #[track_caller] @@ -1022,49 +1021,30 @@ impl ScheduleGraph { mut conditions, } = set; - let id = match self.system_set_ids.get(&set) { + let key = match self.system_set_ids.get(&set) { Some(&id) => id, None => self.add_set(set), }; // 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`) - let system_set_conditions = &mut self.system_set_conditions[id.index()]; - self.uninit.push((id, system_set_conditions.len())); + let system_set_conditions = self.system_set_conditions.entry(key).unwrap().or_default(); + self.uninit.push(UninitializedId::Set { + key, + first_uninit_condition: system_set_conditions.len(), + }); system_set_conditions.append(&mut conditions); - Ok(id) + Ok(NodeId::Set(key)) } - fn add_set(&mut self, set: InternedSystemSet) -> NodeId { - let id = NodeId::Set(self.system_sets.len()); - self.system_sets.push(SystemSetNode::new(set)); - self.system_set_conditions.push(Vec::new()); - self.system_set_ids.insert(set, id); - 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 add_set(&mut self, set: InternedSystemSet) -> SystemSetKey { + let key = self.system_sets.insert(SystemSetNode::new(set)); + self.system_set_conditions.insert(key, Vec::new()); + self.system_set_ids.insert(set, key); + key } fn create_anonymous_set(&mut self) -> AnonymousSet { @@ -1077,11 +1057,18 @@ impl ScheduleGraph { /// Add all the sets from the [`GraphInfo`]'s hierarchy to the graph. fn check_hierarchy_sets( &mut self, - id: &NodeId, + id: NodeId, graph_info: &GraphInfo, ) -> Result<(), ScheduleBuildError> { for &set in &graph_info.hierarchy { - self.check_hierarchy_set(id, set)?; + if let NodeId::Set(key) = id { + if self.system_set_ids.get(&set).is_some_and(|set| *set == key) { + return Err(ScheduleBuildError::HierarchyLoop( + self.get_node_name(&NodeId::Set(key)), + )); + } + } + self.add_set(set); } Ok(()) @@ -1091,22 +1078,25 @@ impl ScheduleGraph { /// Add all the sets from the [`GraphInfo`]'s dependencies to the graph. fn check_edges( &mut self, - id: &NodeId, + id: NodeId, graph_info: &GraphInfo, ) -> Result<(), ScheduleBuildError> { for Dependency { set, .. } in &graph_info.dependencies { - match self.system_set_ids.get(set) { - Some(set_id) => { - if id == set_id { - return Err(ScheduleBuildError::DependencyLoop(self.get_node_name(id))); - } - } - None => { - self.add_set(*set); + if let NodeId::Set(key) = id { + if self.system_set_ids.get(set).is_some_and(|set| *set == key) { + return Err(ScheduleBuildError::DependencyLoop( + self.get_node_name(&NodeId::Set(key)), + )); } } + 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 { for set in ambiguous_with { if !self.system_set_ids.contains_key(set) { @@ -1114,8 +1104,6 @@ impl ScheduleGraph { } } } - - Ok(()) } /// Update the internal graphs (hierarchy, dependency, ambiguity) by adding a single [`GraphInfo`] @@ -1124,8 +1112,9 @@ impl ScheduleGraph { id: NodeId, graph_info: GraphInfo, ) -> Result<(), ScheduleBuildError> { - self.check_hierarchy_sets(&id, &graph_info)?; - self.check_edges(&id, &graph_info)?; + self.check_hierarchy_sets(id, &graph_info)?; + self.check_edges(id, &graph_info)?; + self.add_ambiguities(&graph_info); self.changed = true; let GraphInfo { @@ -1138,20 +1127,20 @@ impl ScheduleGraph { self.hierarchy.graph.add_node(id); self.dependency.graph.add_node(id); - for set in sets.into_iter().map(|set| self.system_set_ids[&set]) { - self.hierarchy.graph.add_edge(set, id); + for key in sets.into_iter().map(|set| self.system_set_ids[&set]) { + self.hierarchy.graph.add_edge(NodeId::Set(key), id); // 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() .map(|Dependency { kind, set, options }| (kind, self.system_set_ids[&set], options)) { let (lhs, rhs) = match kind { - DependencyKind::Before => (id, set), - DependencyKind::After => (set, id), + DependencyKind::Before => (id, NodeId::Set(key)), + DependencyKind::After => (NodeId::Set(key), id), }; self.dependency.graph.add_edge(lhs, rhs); for pass in self.passes.values_mut() { @@ -1159,17 +1148,17 @@ impl ScheduleGraph { } // ensure set also appears in hierarchy graph - self.hierarchy.graph.add_node(set); + self.hierarchy.graph.add_node(NodeId::Set(key)); } match ambiguous_with { Ambiguity::Check => (), Ambiguity::IgnoreWithSet(ambiguous_with) => { - for set in ambiguous_with + for key in ambiguous_with .into_iter() .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 => { @@ -1182,16 +1171,22 @@ impl ScheduleGraph { /// Initializes any newly-added systems and conditions by calling [`System::initialize`](crate::system::System) pub fn initialize(&mut self, world: &mut World) { - for (id, i) in self.uninit.drain(..) { + for id in self.uninit.drain(..) { match id { - NodeId::System(index) => { - self.systems[index].get_mut().unwrap().initialize(world); - for condition in &mut self.system_conditions[index] { + UninitializedId::System(key) => { + self.systems[key].get_mut().unwrap().initialize(world); + for condition in &mut self.system_conditions[key] { condition.initialize(world); } } - NodeId::Set(index) => { - for condition in self.system_set_conditions[index].iter_mut().skip(i) { + UninitializedId::Set { + key, + first_uninit_condition, + } => { + for condition in self.system_set_conditions[key] + .iter_mut() + .skip(first_uninit_condition) + { condition.initialize(world); } } @@ -1283,41 +1278,47 @@ impl ScheduleGraph { &self, hierarchy_topsort: &[NodeId], hierarchy_graph: &DiGraph, - ) -> (HashMap>, HashMap) { - let mut set_systems: HashMap> = + ) -> ( + HashMap>, + HashMap>, + ) { + let mut set_systems: HashMap> = HashMap::with_capacity_and_hasher(self.system_sets.len(), Default::default()); - let mut set_system_bitsets = + let mut set_system_sets: HashMap> = HashMap::with_capacity_and_hasher(self.system_sets.len(), Default::default()); for &id in hierarchy_topsort.iter().rev() { - if id.is_system() { + let NodeId::Set(set_key) = id else { continue; - } + }; 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) { match child { - NodeId::System(_) => { - systems.push(child); - system_bitset.insert(child.index()); + NodeId::System(key) => { + systems.push(key); + system_set.insert(key); } - NodeId::Set(_) => { - let child_systems = set_systems.get(&child).unwrap(); - let child_system_bitset = set_system_bitsets.get(&child).unwrap(); + NodeId::Set(key) => { + let child_systems = set_systems.get(&key).unwrap(); + let child_system_set = set_system_sets.get(&key).unwrap(); 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_system_bitsets.insert(id, system_bitset); + set_systems.insert(set_key, systems); + 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>) -> DiGraph { + fn get_dependency_flattened( + &mut self, + set_systems: &HashMap>, + ) -> DiGraph { // flatten: combine `in_set` with `before` and `after` information // have to do it like this to preserve transitivity let mut dependency_flattened = self.dependency.graph.clone(); @@ -1328,26 +1329,26 @@ impl ScheduleGraph { } if systems.is_empty() { // collapse dependencies for empty sets - for a in dependency_flattened.neighbors_directed(set, Incoming) { - for b in dependency_flattened.neighbors_directed(set, Outgoing) { + for a in dependency_flattened.neighbors_directed(NodeId::Set(set), Incoming) { + for b in dependency_flattened.neighbors_directed(NodeId::Set(set), Outgoing) { temp.push((a, b)); } } } 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 { - 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 { - 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(..) { dependency_flattened.add_edge(a, b); } @@ -1356,27 +1357,31 @@ impl ScheduleGraph { dependency_flattened } - fn get_ambiguous_with_flattened(&self, set_systems: &HashMap>) -> UnGraph { + fn get_ambiguous_with_flattened( + &self, + set_systems: &HashMap>, + ) -> UnGraph { let mut ambiguous_with_flattened = UnGraph::default(); for (lhs, rhs) in self.ambiguous_with.all_edges() { match (lhs, rhs) { (NodeId::System(_), NodeId::System(_)) => { 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()) { - 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()) { - 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 &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_)); } } } @@ -1391,7 +1396,7 @@ impl ScheduleGraph { flat_results_disconnected: &Vec<(NodeId, NodeId)>, ambiguous_with_flattened: &UnGraph, ignored_ambiguities: &BTreeSet, - ) -> Vec<(NodeId, NodeId, Vec)> { + ) -> Vec<(SystemKey, SystemKey, Vec)> { let mut conflicting_systems = Vec::new(); for &(a, b) in flat_results_disconnected { if ambiguous_with_flattened.contains_edge(a, b) @@ -1401,8 +1406,14 @@ impl ScheduleGraph { continue; } - let system_a = self.systems[a.index()].get().unwrap(); - let system_b = self.systems[b.index()].get().unwrap(); + let NodeId::System(a) = a else { + continue; + }; + let NodeId::System(b) = b else { + continue; + }; + let system_a = self.systems[a].get().unwrap(); + let system_b = self.systems[b].get().unwrap(); if system_a.is_exclusive() || system_b.is_exclusive() { conflicting_systems.push((a, b, Vec::new())); } else { @@ -1438,7 +1449,11 @@ impl ScheduleGraph { dependency_flattened_dag: Dag, hier_results_reachable: FixedBitSet, ) -> 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::>(); let dg_system_idx_map = dg_system_ids .iter() .cloned() @@ -1461,10 +1476,11 @@ impl ScheduleGraph { .iter() .cloned() .enumerate() - .filter(|&(_i, id)| { + .filter_map(|(i, id)| { // ignore system sets that have no 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(); @@ -1476,16 +1492,19 @@ impl ScheduleGraph { // (needed by multi_threaded executor to run systems in the correct order) let mut system_dependencies = 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 .graph - .neighbors_directed(sys_id, Incoming) + .neighbors_directed(NodeId::System(sys_key), Incoming) .count(); let dependents = dependency_flattened_dag .graph - .neighbors_directed(sys_id, Outgoing) - .map(|dep_id| dg_system_idx_map[&dep_id]) + .neighbors_directed(NodeId::System(sys_key), Outgoing) + .filter_map(|dep_id| { + let dep_key = dep_id.as_system()?; + Some(dg_system_idx_map[&dep_key]) + }) .collect::>(); system_dependencies.push(num_dependencies); @@ -1499,7 +1518,10 @@ impl ScheduleGraph { for (i, &row) in hg_set_with_conditions_idxs.iter().enumerate() { let bitset = &mut systems_in_sets_with_conditions[i]; for &(col, sys_id) in &hg_systems { - let idx = dg_system_idx_map[&sys_id]; + let NodeId::System(sys_key) = sys_id else { + continue; + }; + let idx = dg_system_idx_map[&sys_key]; let is_descendant = hier_results_reachable[index(row, col, hg_node_count)]; bitset.set(idx, is_descendant); } @@ -1508,7 +1530,10 @@ impl ScheduleGraph { let mut sets_with_conditions_of_systems = vec![FixedBitSet::with_capacity(set_with_conditions_count); sys_count]; for &(col, sys_id) in &hg_systems { - let i = dg_system_idx_map[&sys_id]; + let NodeId::System(sys_key) = sys_id else { + continue; + }; + let i = dg_system_idx_map[&sys_key]; let bitset = &mut sets_with_conditions_of_systems[i]; for (idx, &row) in hg_set_with_conditions_idxs .iter() @@ -1546,36 +1571,36 @@ impl ScheduleGraph { } // move systems out of old schedule - for ((id, system), conditions) in schedule + for ((key, system), conditions) in schedule .system_ids .drain(..) .zip(schedule.systems.drain(..)) .zip(schedule.system_conditions.drain(..)) { - self.systems[id.index()].inner = Some(system); - self.system_conditions[id.index()] = conditions; + self.systems[key].inner = Some(system); + self.system_conditions[key] = conditions; } - for (id, conditions) in schedule + for (key, conditions) in schedule .set_ids .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)?; // move systems into new schedule - for &id in &schedule.system_ids { - let system = self.systems[id.index()].inner.take().unwrap(); - let conditions = core::mem::take(&mut self.system_conditions[id.index()]); + for &key in &schedule.system_ids { + let system = self.systems[key].inner.take().unwrap(); + let conditions = core::mem::take(&mut self.system_conditions[key]); schedule.systems.push(system); schedule.system_conditions.push(conditions); } - for &id in &schedule.set_ids { - let conditions = core::mem::take(&mut self.system_set_conditions[id.index()]); + for &key in &schedule.set_ids { + let conditions = core::mem::take(&mut self.system_set_conditions[key]); schedule.set_conditions.push(conditions); } @@ -1628,9 +1653,9 @@ impl ScheduleGraph { #[inline] fn get_node_name_inner(&self, id: &NodeId, report_sets: bool) -> String { - let name = match id { - NodeId::System(_) => { - let name = self.systems[id.index()].get().unwrap().name().to_string(); + let name = match *id { + NodeId::System(key) => { + let name = self.systems[key].get().unwrap().name().to_string(); if report_sets { let sets = self.names_of_sets_containing_node(id); if sets.is_empty() { @@ -1644,8 +1669,8 @@ impl ScheduleGraph { name } } - NodeId::Set(_) => { - let set = &self.system_sets[id.index()]; + NodeId::Set(key) => { + let set = &self.system_sets[key]; if set.is_anonymous() { self.anonymous_set_name(id) } else { @@ -1841,16 +1866,16 @@ impl ScheduleGraph { fn check_order_but_intersect( &self, dep_results_connected: &HashSet<(NodeId, NodeId)>, - set_system_bitsets: &HashMap, + set_system_sets: &HashMap>, ) -> Result<(), ScheduleBuildError> { // check that there is no ordering between system sets that intersect 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; - } + }; - let a_systems = set_system_bitsets.get(a).unwrap(); - let b_systems = set_system_bitsets.get(b).unwrap(); + let a_systems = set_system_sets.get(a_key).unwrap(); + let b_systems = set_system_sets.get(b_key).unwrap(); if !a_systems.is_disjoint(b_systems) { return Err(ScheduleBuildError::SetsHaveOrderButIntersect( @@ -1865,19 +1890,25 @@ impl ScheduleGraph { fn check_system_type_set_ambiguity( &self, - set_systems: &HashMap>, + set_systems: &HashMap>, ) -> Result<(), ScheduleBuildError> { - for (&id, systems) in set_systems { - let set = &self.system_sets[id.index()]; + for (&key, systems) in set_systems { + let set = &self.system_sets[key]; if set.is_system_type() { let instances = systems.len(); - let ambiguous_with = self.ambiguous_with.edges(id); - let before = self.dependency.graph.edges_directed(id, Incoming); - let after = self.dependency.graph.edges_directed(id, Outgoing); + let ambiguous_with = self.ambiguous_with.edges(NodeId::Set(key)); + let before = self + .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(); if instances > 1 && relations > 0 { return Err(ScheduleBuildError::SystemTypeSetAmbiguity( - self.get_node_name(&id), + self.get_node_name(&NodeId::Set(key)), )); } } @@ -1888,7 +1919,7 @@ impl ScheduleGraph { /// if [`ScheduleBuildSettings::ambiguity_detection`] is [`LogLevel::Ignore`], this check is skipped fn optionally_check_conflicts( &self, - conflicts: &[(NodeId, NodeId, Vec)], + conflicts: &[(SystemKey, SystemKey, Vec)], components: &Components, schedule_label: InternedScheduleLabel, ) -> Result<(), ScheduleBuildError> { @@ -1909,7 +1940,7 @@ impl ScheduleGraph { fn get_conflicts_error_message( &self, - ambiguities: &[(NodeId, NodeId, Vec)], + ambiguities: &[(SystemKey, SystemKey, Vec)], components: &Components, ) -> String { let n_ambiguities = ambiguities.len(); @@ -1937,17 +1968,14 @@ impl ScheduleGraph { /// convert conflicts to human readable format pub fn conflicts_to_string<'a>( &'a self, - ambiguities: &'a [(NodeId, NodeId, Vec)], + ambiguities: &'a [(SystemKey, SystemKey, Vec)], components: &'a Components, ) -> impl Iterator>)> + 'a { ambiguities .iter() .map(move |(system_a, system_b, conflicts)| { - let name_a = self.get_node_name(system_a); - let name_b = self.get_node_name(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 name_a = self.get_node_name(&NodeId::System(*system_a)); + let name_b = self.get_node_name(&NodeId::System(*system_b)); let conflict_names: Vec<_> = conflicts .iter() @@ -1958,22 +1986,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) { - if f(set_id) { - self.traverse_sets_containing_node(set_id, f); + let NodeId::Set(set_key) = set_id else { + 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 { let mut sets = >::default(); - self.traverse_sets_containing_node(*id, &mut |set_id| { - !self.system_sets[set_id.index()].is_system_type() && sets.insert(set_id) + self.traverse_sets_containing_node(*id, &mut |key| { + !self.system_sets[key].is_system_type() && sets.insert(key) }); let mut sets: Vec<_> = sets .into_iter() - .map(|set_id| self.get_node_name(&set_id)) + .map(|key| self.get_node_name(&NodeId::Set(key))) .collect(); sets.sort(); sets diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs index b5df8555e2115..4adf7f5759fc5 100644 --- a/crates/bevy_ecs/src/schedule/stepping.rs +++ b/crates/bevy_ecs/src/schedule/stepping.rs @@ -1,6 +1,6 @@ use crate::{ resource::Resource, - schedule::{InternedScheduleLabel, NodeId, Schedule, ScheduleLabel}, + schedule::{InternedScheduleLabel, NodeId, Schedule, ScheduleLabel, SystemKey}, system::{IntoSystem, ResMut}, }; use alloc::vec::Vec; @@ -173,7 +173,7 @@ impl Stepping { state .node_ids .get(self.cursor.system) - .map(|node_id| (*label, *node_id)) + .map(|node_id| (*label, NodeId::System(*node_id))) } /// Enable stepping for the provided schedule @@ -609,7 +609,7 @@ struct ScheduleState { /// This is a cached copy of `SystemExecutable::system_ids`. We need it /// available here to be accessed by [`Stepping::cursor()`] so we can return /// [`NodeId`]s to the caller. - node_ids: Vec, + node_ids: Vec, /// changes to system behavior that should be applied the next time /// [`ScheduleState::skipped_systems()`] is called @@ -665,15 +665,15 @@ impl ScheduleState { // updates for the system TypeId. // PERF: If we add a way to efficiently query schedule systems by their TypeId, we could remove the full // 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()); match behavior { None => continue, Some(None) => { - self.behaviors.remove(&node_id); + self.behaviors.remove(&NodeId::System(key)); } Some(Some(behavior)) => { - self.behaviors.insert(node_id, *behavior); + self.behaviors.insert(NodeId::System(key), *behavior); } } } @@ -706,8 +706,8 @@ impl ScheduleState { // if we don't have a first system set, set it now if self.first.is_none() { - for (i, (node_id, _)) in schedule.systems().unwrap().enumerate() { - match self.behaviors.get(&node_id) { + for (i, (key, _)) in schedule.systems().unwrap().enumerate() { + match self.behaviors.get(&NodeId::System(key)) { Some(SystemBehavior::AlwaysRun | SystemBehavior::NeverRun) => continue, Some(_) | None => { self.first = Some(i); @@ -720,10 +720,10 @@ impl ScheduleState { let mut skip = FixedBitSet::with_capacity(schedule.systems_len()); 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 .behaviors - .get(&node_id) + .get(&NodeId::System(key)) .unwrap_or(&SystemBehavior::Continue); #[cfg(test)] @@ -828,6 +828,7 @@ mod tests { use super::*; use crate::{prelude::*, schedule::ScheduleLabel}; use alloc::{format, vec}; + use slotmap::SlotMap; use std::println; #[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)] @@ -1470,10 +1471,11 @@ mod tests { // helper to build a cursor tuple for the supplied schedule fn cursor(schedule: &Schedule, index: usize) -> (InternedScheduleLabel, NodeId) { 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 slotmap = SlotMap::::with_key(); // create two schedules with a number of systems in them let mut schedule_a = Schedule::new(TestScheduleA); @@ -1520,6 +1522,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 // cursor properly skips over AlwaysRun & NeverRun systems. Also set // a Break system to ensure that shows properly in the cursor @@ -1527,9 +1534,9 @@ mod tests { // disable/enable to reset cursor .disable() .enable() - .set_breakpoint_node(TestScheduleA, NodeId::System(1)) - .always_run_node(TestScheduleA, NodeId::System(3)) - .never_run_node(TestScheduleB, NodeId::System(0)); + .set_breakpoint_node(TestScheduleA, NodeId::System(sys1)) + .always_run_node(TestScheduleA, NodeId::System(sys3)) + .never_run_node(TestScheduleB, NodeId::System(sys0)); let mut cursors = Vec::new(); for _ in 0..9 { diff --git a/examples/games/stepping.rs b/examples/games/stepping.rs index 653b7a12ff8f3..50f0a3a3237ef 100644 --- a/examples/games/stepping.rs +++ b/examples/games/stepping.rs @@ -139,7 +139,9 @@ fn build_ui( // Add an entry to our systems list so we can find where to draw // the cursor when the stepping cursor is at this system // 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(node_id), text_spans.len() + 1)); // Add a text section for displaying the cursor for this system text_spans.push(( @@ -158,7 +160,7 @@ fn build_ui( } for (label, node) in always_run.drain(..) { - stepping.always_run_node(label, node); + stepping.always_run_node(label, NodeId::System(node)); } commands.spawn(( diff --git a/release-content/migration-guides/schedule_slotmaps.md b/release-content/migration-guides/schedule_slotmaps.md new file mode 100644 index 0000000000000..8be72a23fab08 --- /dev/null +++ b/release-content/migration-guides/schedule_slotmaps.md @@ -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.