diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index be80709fb55c2..222788ec817cf 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -27,7 +27,6 @@ bevy_tasks = { path = "../bevy_tasks", version = "0.15.0-dev" } bevy_utils = { path = "../bevy_utils", version = "0.15.0-dev" } bevy_ecs_macros = { path = "macros", version = "0.15.0-dev" } -petgraph = "0.6" bitflags = "2.3" concurrent-queue = "2.5.0" disqualified = "1.0" @@ -43,6 +42,7 @@ derive_more = { version = "1", default-features = false, features = [ nonmax = "0.5" arrayvec = { version = "0.7.4", optional = true } smallvec = { version = "1", features = ["union"] } +indexmap = { version = "2.5.0", default-features = false, features = ["std"] } variadics_please = "1.0" [dev-dependencies] diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index 0b4fdad860f69..3faac82edb456 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -3,7 +3,7 @@ use variadics_please::all_tuples; use crate::{ schedule::{ condition::{BoxedCondition, Condition}, - graph_utils::{Ambiguity, Dependency, DependencyKind, GraphInfo}, + graph::{Ambiguity, Dependency, DependencyKind, GraphInfo}, set::{InternedSystemSet, IntoSystemSet, SystemSet}, Chain, }, diff --git a/crates/bevy_ecs/src/schedule/graph/graph_map.rs b/crates/bevy_ecs/src/schedule/graph/graph_map.rs new file mode 100644 index 0000000000000..680774bce5f4d --- /dev/null +++ b/crates/bevy_ecs/src/schedule/graph/graph_map.rs @@ -0,0 +1,480 @@ +//! `Graph` is a graph datastructure where node values are mapping +//! keys. +//! Based on the `GraphMap` datastructure from [`petgraph`]. +//! +//! [`petgraph`]: https://docs.rs/petgraph/0.6.5/petgraph/ + +use bevy_utils::{hashbrown::HashSet, AHasher}; +use core::{ + fmt, + hash::{BuildHasher, BuildHasherDefault, Hash}, +}; +use indexmap::IndexMap; +use smallvec::SmallVec; + +use super::NodeId; + +use Direction::{Incoming, Outgoing}; + +/// A `Graph` with undirected edges. +/// +/// For example, an edge between *1* and *2* is equivalent to an edge between +/// *2* and *1*. +pub type UnGraph> = Graph; + +/// A `Graph` with directed edges. +/// +/// For example, an edge from *1* to *2* is distinct from an edge from *2* to +/// *1*. +pub type DiGraph> = Graph; + +/// `Graph` is a graph datastructure using an associative array +/// of its node weights `NodeId`. +/// +/// It uses an combined adjacency list and sparse adjacency matrix +/// representation, using **O(|N| + |E|)** space, and allows testing for edge +/// existence in constant time. +/// +/// `Graph` is parameterized over: +/// +/// - Constant generic bool `DIRECTED` determines whether the graph edges are directed or +/// undirected. +/// - The `BuildHasher` `S`. +/// +/// You can use the type aliases `UnGraph` and `DiGraph` for convenience. +/// +/// `Graph` does not allow parallel edges, but self loops are allowed. +#[derive(Clone)] +pub struct Graph> +where + S: BuildHasher, +{ + nodes: IndexMap, S>, + edges: HashSet, +} + +impl fmt::Debug for Graph { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.nodes.fmt(f) + } +} + +impl Graph +where + S: BuildHasher, +{ + /// Create a new `Graph` + pub(crate) fn new() -> Self + where + S: Default, + { + Self::default() + } + + /// Create a new `Graph` with estimated capacity. + pub(crate) fn with_capacity(nodes: usize, edges: usize) -> Self + where + S: Default, + { + Self { + nodes: IndexMap::with_capacity_and_hasher(nodes, S::default()), + edges: HashSet::with_capacity_and_hasher(edges, S::default()), + } + } + + /// Use their natural order to map the node pair (a, b) to a canonical edge id. + #[inline] + fn edge_key(a: NodeId, b: NodeId) -> CompactNodeIdPair { + let (a, b) = if DIRECTED || a <= b { (a, b) } else { (b, a) }; + + CompactNodeIdPair::store(a, b) + } + + /// Return the number of nodes in the graph. + pub fn node_count(&self) -> usize { + self.nodes.len() + } + + /// Add node `n` to the graph. + pub(crate) fn add_node(&mut self, n: NodeId) { + self.nodes.entry(n).or_default(); + } + + /// Remove a node `n` from the graph. + /// + /// Computes in **O(N)** time, due to the removal of edges with other nodes. + pub(crate) fn remove_node(&mut self, n: NodeId) { + let Some(links) = self.nodes.swap_remove(&n) else { + return; + }; + + let links = links.into_iter().map(CompactNodeIdAndDirection::load); + + for (succ, dir) in links { + let edge = if dir == Outgoing { + Self::edge_key(n, succ) + } else { + Self::edge_key(succ, n) + }; + // remove all successor links + self.remove_single_edge(succ, n, dir.opposite()); + // Remove all edge values + self.edges.remove(&edge); + } + } + + /// Return `true` if the node is contained in the graph. + pub fn contains_node(&self, n: NodeId) -> bool { + self.nodes.contains_key(&n) + } + + /// Add an edge connecting `a` and `b` to the graph. + /// For a directed graph, the edge is directed from `a` to `b`. + /// + /// Inserts nodes `a` and/or `b` if they aren't already part of the graph. + pub(crate) fn add_edge(&mut self, a: NodeId, b: NodeId) { + if self.edges.insert(Self::edge_key(a, b)) { + // insert in the adjacency list if it's a new edge + self.nodes + .entry(a) + .or_insert_with(|| Vec::with_capacity(1)) + .push(CompactNodeIdAndDirection::store(b, Outgoing)); + if a != b { + // self loops don't have the Incoming entry + self.nodes + .entry(b) + .or_insert_with(|| Vec::with_capacity(1)) + .push(CompactNodeIdAndDirection::store(a, Incoming)); + } + } + } + + /// Remove edge relation from a to b + /// + /// Return `true` if it did exist. + fn remove_single_edge(&mut self, a: NodeId, b: NodeId, dir: Direction) -> bool { + let Some(sus) = self.nodes.get_mut(&a) else { + return false; + }; + + let Some(index) = sus + .iter() + .copied() + .map(CompactNodeIdAndDirection::load) + .position(|elt| (DIRECTED && elt == (b, dir)) || (!DIRECTED && elt.0 == b)) + else { + return false; + }; + + sus.swap_remove(index); + true + } + + /// Remove edge from `a` to `b` from the graph. + /// + /// Return `false` if the edge didn't exist. + pub(crate) fn remove_edge(&mut self, a: NodeId, b: NodeId) -> bool { + let exist1 = self.remove_single_edge(a, b, Outgoing); + let exist2 = if a != b { + self.remove_single_edge(b, a, Incoming) + } else { + exist1 + }; + let weight = self.edges.remove(&Self::edge_key(a, b)); + debug_assert!(exist1 == exist2 && exist1 == weight); + weight + } + + /// Return `true` if the edge connecting `a` with `b` is contained in the graph. + pub fn contains_edge(&self, a: NodeId, b: NodeId) -> bool { + self.edges.contains(&Self::edge_key(a, b)) + } + + /// Return an iterator over the nodes of the graph. + pub fn nodes( + &self, + ) -> impl DoubleEndedIterator + ExactSizeIterator + '_ { + self.nodes.keys().copied() + } + + /// Return an iterator of all nodes with an edge starting from `a`. + pub fn neighbors(&self, a: NodeId) -> impl DoubleEndedIterator + '_ { + let iter = match self.nodes.get(&a) { + Some(neigh) => neigh.iter(), + None => [].iter(), + }; + + iter.copied() + .map(CompactNodeIdAndDirection::load) + .filter_map(|(n, dir)| (!DIRECTED || dir == Outgoing).then_some(n)) + } + + /// Return an iterator of all neighbors that have an edge between them and + /// `a`, in the specified direction. + /// If the graph's edges are undirected, this is equivalent to *.neighbors(a)*. + pub fn neighbors_directed( + &self, + a: NodeId, + dir: Direction, + ) -> impl DoubleEndedIterator + '_ { + let iter = match self.nodes.get(&a) { + Some(neigh) => neigh.iter(), + None => [].iter(), + }; + + iter.copied() + .map(CompactNodeIdAndDirection::load) + .filter_map(move |(n, d)| (!DIRECTED || d == dir || n == a).then_some(n)) + } + + /// Return an iterator of target nodes with an edge starting from `a`, + /// paired with their respective edge weights. + pub fn edges(&self, a: NodeId) -> impl DoubleEndedIterator + '_ { + self.neighbors(a) + .map(move |b| match self.edges.get(&Self::edge_key(a, b)) { + None => unreachable!(), + Some(_) => (a, b), + }) + } + + /// Return an iterator of target nodes with an edge starting from `a`, + /// paired with their respective edge weights. + pub fn edges_directed( + &self, + a: NodeId, + dir: Direction, + ) -> impl DoubleEndedIterator + '_ { + self.neighbors_directed(a, dir).map(move |b| { + let (a, b) = if dir == Incoming { (b, a) } else { (a, b) }; + + match self.edges.get(&Self::edge_key(a, b)) { + None => unreachable!(), + Some(_) => (a, b), + } + }) + } + + /// Return an iterator over all edges of the graph with their weight in arbitrary order. + pub fn all_edges(&self) -> impl ExactSizeIterator + '_ { + self.edges.iter().copied().map(CompactNodeIdPair::load) + } + + pub(crate) fn to_index(&self, ix: NodeId) -> usize { + self.nodes.get_index_of(&ix).unwrap() + } +} + +/// Create a new empty `Graph`. +impl Default for Graph +where + S: BuildHasher + Default, +{ + fn default() -> Self { + Self::with_capacity(0, 0) + } +} + +impl Graph { + /// Iterate over all *Strongly Connected Components* in this graph. + pub(crate) fn iter_sccs(&self) -> impl Iterator> + '_ { + super::tarjan_scc::new_tarjan_scc(self) + } +} + +/// Edge direction. +#[derive(Clone, Copy, Debug, PartialEq, PartialOrd, Ord, Eq, Hash)] +#[repr(u8)] +pub enum Direction { + /// An `Outgoing` edge is an outward edge *from* the current node. + Outgoing = 0, + /// An `Incoming` edge is an inbound edge *to* the current node. + Incoming = 1, +} + +impl Direction { + /// Return the opposite `Direction`. + #[inline] + pub fn opposite(self) -> Self { + match self { + Self::Outgoing => Self::Incoming, + Self::Incoming => Self::Outgoing, + } + } +} + +/// Compact storage of a [`NodeId`] and a [`Direction`]. +#[derive(Clone, Copy)] +struct CompactNodeIdAndDirection { + index: usize, + is_system: bool, + direction: Direction, +} + +impl fmt::Debug for CompactNodeIdAndDirection { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.load().fmt(f) + } +} + +impl CompactNodeIdAndDirection { + const fn store(node: NodeId, direction: Direction) -> Self { + let index = node.index(); + let is_system = node.is_system(); + + Self { + index, + is_system, + direction, + } + } + + const fn load(self) -> (NodeId, Direction) { + let Self { + index, + is_system, + direction, + } = self; + + let node = match is_system { + true => NodeId::System(index), + false => NodeId::Set(index), + }; + + (node, direction) + } +} + +/// Compact storage of a [`NodeId`] pair. +#[derive(Clone, Copy, Hash, PartialEq, Eq)] +struct CompactNodeIdPair { + index_a: usize, + index_b: usize, + is_system_a: bool, + is_system_b: bool, +} + +impl fmt::Debug for CompactNodeIdPair { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.load().fmt(f) + } +} + +impl CompactNodeIdPair { + const fn store(a: NodeId, b: NodeId) -> Self { + let index_a = a.index(); + let is_system_a = a.is_system(); + + let index_b = b.index(); + let is_system_b = b.is_system(); + + Self { + index_a, + index_b, + is_system_a, + is_system_b, + } + } + + const fn load(self) -> (NodeId, NodeId) { + let Self { + index_a, + index_b, + is_system_a, + is_system_b, + } = self; + + let a = match is_system_a { + true => NodeId::System(index_a), + false => NodeId::Set(index_a), + }; + + let b = match is_system_b { + true => NodeId::System(index_b), + false => NodeId::Set(index_b), + }; + + (a, b) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// 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 + /// location of the removed node. + #[test] + fn node_order_preservation() { + use NodeId::System; + + let mut graph = Graph::::new(); + + graph.add_node(System(1)); + graph.add_node(System(2)); + graph.add_node(System(3)); + graph.add_node(System(4)); + + assert_eq!( + graph.nodes().collect::>(), + vec![System(1), System(2), System(3), System(4)] + ); + + graph.remove_node(System(1)); + + assert_eq!( + graph.nodes().collect::>(), + vec![System(4), System(2), System(3)] + ); + + graph.remove_node(System(4)); + + assert_eq!( + graph.nodes().collect::>(), + vec![System(3), System(2)] + ); + + graph.remove_node(System(2)); + + assert_eq!(graph.nodes().collect::>(), vec![System(3)]); + + graph.remove_node(System(3)); + + assert_eq!(graph.nodes().collect::>(), vec![]); + } + + /// Nodes that have bidrectional edges (or any edge in the case of undirected graphs) are + /// considered strongly connected. A strongly connected component is a collection of + /// nodes where there exists a path from any node to any other node in the collection. + #[test] + fn strongly_connected_components() { + use NodeId::System; + + let mut graph = Graph::::new(); + + graph.add_edge(System(1), System(2)); + graph.add_edge(System(2), System(1)); + + graph.add_edge(System(2), System(3)); + graph.add_edge(System(3), System(2)); + + graph.add_edge(System(4), System(5)); + graph.add_edge(System(5), System(4)); + + graph.add_edge(System(6), System(2)); + + let sccs = graph + .iter_sccs() + .map(|scc| scc.to_vec()) + .collect::>(); + + assert_eq!( + sccs, + vec![ + vec![System(3), System(2), System(1)], + vec![System(5), System(4)], + vec![System(6)] + ] + ); + } +} diff --git a/crates/bevy_ecs/src/schedule/graph_utils.rs b/crates/bevy_ecs/src/schedule/graph/mod.rs similarity index 78% rename from crates/bevy_ecs/src/schedule/graph_utils.rs rename to crates/bevy_ecs/src/schedule/graph/mod.rs index a92579ca58293..08c4951d201cf 100644 --- a/crates/bevy_ecs/src/schedule/graph_utils.rs +++ b/crates/bevy_ecs/src/schedule/graph/mod.rs @@ -1,40 +1,20 @@ +use alloc::vec; +use alloc::vec::Vec; use core::fmt::Debug; +use core::hash::BuildHasherDefault; +use smallvec::SmallVec; -use bevy_utils::{HashMap, HashSet}; +use bevy_utils::{AHasher, HashMap, HashSet}; use fixedbitset::FixedBitSet; -use petgraph::{algo::TarjanScc, graphmap::NodeTrait, prelude::*}; use crate::schedule::set::*; -/// Unique identifier for a system or system set stored in a [`ScheduleGraph`]. -/// -/// [`ScheduleGraph`]: super::ScheduleGraph -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub enum NodeId { - /// Identifier for a system. - System(usize), - /// Identifier for a system set. - Set(usize), -} - -impl NodeId { - /// Returns the internal integer value. - pub(crate) fn index(&self) -> usize { - match self { - NodeId::System(index) | NodeId::Set(index) => *index, - } - } +mod graph_map; +mod node; +mod tarjan_scc; - /// Returns `true` if the identified node is a system. - pub const fn is_system(&self) -> bool { - matches!(self, NodeId::System(_)) - } - - /// Returns `true` if the identified node is a system set. - pub const fn is_set(&self) -> bool { - matches!(self, NodeId::Set(_)) - } -} +pub use graph_map::{DiGraph, Direction, UnGraph}; +pub use node::NodeId; /// Specifies what kind of edge should be added to the dependency graph. #[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Hash)] @@ -95,32 +75,32 @@ pub(crate) fn row_col(index: usize, num_cols: usize) -> (usize, usize) { } /// Stores the results of the graph analysis. -pub(crate) struct CheckGraphResults { +pub(crate) struct CheckGraphResults { /// Boolean reachability matrix for the graph. pub(crate) reachable: FixedBitSet, /// Pairs of nodes that have a path connecting them. - pub(crate) connected: HashSet<(V, V)>, + pub(crate) connected: HashSet<(NodeId, NodeId)>, /// Pairs of nodes that don't have a path connecting them. - pub(crate) disconnected: Vec<(V, V)>, + pub(crate) disconnected: Vec<(NodeId, NodeId)>, /// Edges that are redundant because a longer path exists. - pub(crate) transitive_edges: Vec<(V, V)>, + pub(crate) transitive_edges: Vec<(NodeId, NodeId)>, /// Variant of the graph with no transitive edges. - pub(crate) transitive_reduction: DiGraphMap, + pub(crate) transitive_reduction: DiGraph, /// Variant of the graph with all possible transitive edges. // TODO: this will very likely be used by "if-needed" ordering #[allow(dead_code)] - pub(crate) transitive_closure: DiGraphMap, + pub(crate) transitive_closure: DiGraph, } -impl Default for CheckGraphResults { +impl Default for CheckGraphResults { fn default() -> Self { Self { reachable: FixedBitSet::new(), connected: HashSet::new(), disconnected: Vec::new(), transitive_edges: Vec::new(), - transitive_reduction: DiGraphMap::new(), - transitive_closure: DiGraphMap::new(), + transitive_reduction: DiGraph::new(), + transitive_closure: DiGraph::new(), } } } @@ -136,13 +116,7 @@ impl Default for CheckGraphResults { /// ["On the calculation of transitive reduction-closure of orders"][1] by Habib, Morvan and Rampon. /// /// [1]: https://doi.org/10.1016/0012-365X(93)90164-O -pub(crate) fn check_graph( - graph: &DiGraphMap, - topological_order: &[V], -) -> CheckGraphResults -where - V: NodeTrait + Debug, -{ +pub(crate) fn check_graph(graph: &DiGraph, topological_order: &[NodeId]) -> CheckGraphResults { if graph.node_count() == 0 { return CheckGraphResults::default(); } @@ -151,14 +125,14 @@ where // build a copy of the graph where the nodes and edges appear in topsorted order let mut map = HashMap::with_capacity(n); - let mut topsorted = DiGraphMap::::new(); + let mut topsorted = DiGraph::>::new(); // iterate nodes in topological order for (i, &node) in topological_order.iter().enumerate() { map.insert(node, i); topsorted.add_node(node); // insert nodes as successors to their predecessors - for pred in graph.neighbors_directed(node, Incoming) { - topsorted.add_edge(pred, node, ()); + for pred in graph.neighbors_directed(node, Direction::Incoming) { + topsorted.add_edge(pred, node); } } @@ -167,8 +141,8 @@ where let mut disconnected = Vec::new(); let mut transitive_edges = Vec::new(); - let mut transitive_reduction = DiGraphMap::::new(); - let mut transitive_closure = DiGraphMap::::new(); + let mut transitive_reduction = DiGraph::new(); + let mut transitive_closure = DiGraph::new(); let mut visited = FixedBitSet::with_capacity(n); @@ -182,24 +156,24 @@ where for a in topsorted.nodes().rev() { let index_a = *map.get(&a).unwrap(); // iterate their successors in topological order - for b in topsorted.neighbors_directed(a, Outgoing) { + for b in topsorted.neighbors_directed(a, Direction::Outgoing) { let index_b = *map.get(&b).unwrap(); debug_assert!(index_a < index_b); if !visited[index_b] { // edge is not redundant - transitive_reduction.add_edge(a, b, ()); - transitive_closure.add_edge(a, b, ()); + transitive_reduction.add_edge(a, b); + transitive_closure.add_edge(a, b); reachable.insert(index(index_a, index_b, n)); let successors = transitive_closure - .neighbors_directed(b, Outgoing) + .neighbors_directed(b, Direction::Outgoing) .collect::>(); for c in successors { let index_c = *map.get(&c).unwrap(); debug_assert!(index_b < index_c); if !visited[index_c] { visited.insert(index_c); - transitive_closure.add_edge(a, c, ()); + transitive_closure.add_edge(a, c); reachable.insert(index(index_a, index_c, n)); } } @@ -247,16 +221,13 @@ where /// ["Finding all the elementary circuits of a directed graph"][1] by D. B. Johnson. /// /// [1]: https://doi.org/10.1137/0204007 -pub fn simple_cycles_in_component(graph: &DiGraphMap, scc: &[N]) -> Vec> -where - N: NodeTrait + Debug, -{ +pub fn simple_cycles_in_component(graph: &DiGraph, scc: &[NodeId]) -> Vec> { let mut cycles = vec![]; - let mut sccs = vec![scc.to_vec()]; + let mut sccs = vec![SmallVec::from_slice(scc)]; while let Some(mut scc) = sccs.pop() { // only look at nodes and edges in this strongly-connected component - let mut subgraph = DiGraphMap::new(); + let mut subgraph = DiGraph::>::new(); for &node in &scc { subgraph.add_node(node); } @@ -264,7 +235,7 @@ where for &node in &scc { for successor in graph.neighbors(node) { if subgraph.contains_node(successor) { - subgraph.add_edge(node, successor, ()); + subgraph.add_edge(node, successor); } } } @@ -275,12 +246,13 @@ where let mut blocked = HashSet::with_capacity(subgraph.node_count()); // connects nodes along path segments that can't be part of a cycle (given current root) // those nodes can be unblocked at the same time - let mut unblock_together: HashMap> = + let mut unblock_together: HashMap> = HashMap::with_capacity(subgraph.node_count()); // stack for unblocking nodes let mut unblock_stack = Vec::with_capacity(subgraph.node_count()); // nodes can be involved in multiple cycles - let mut maybe_in_more_cycles: HashSet = HashSet::with_capacity(subgraph.node_count()); + let mut maybe_in_more_cycles: HashSet = + HashSet::with_capacity(subgraph.node_count()); // stack for DFS let mut stack = Vec::with_capacity(subgraph.node_count()); @@ -338,16 +310,13 @@ where } } + drop(stack); + // remove node from subgraph subgraph.remove_node(root); // divide remainder into smaller SCCs - let mut tarjan_scc = TarjanScc::new(); - tarjan_scc.run(&subgraph, |scc| { - if scc.len() > 1 { - sccs.push(scc.to_vec()); - } - }); + sccs.extend(subgraph.iter_sccs().filter(|scc| scc.len() > 1)); } cycles diff --git a/crates/bevy_ecs/src/schedule/graph/node.rs b/crates/bevy_ecs/src/schedule/graph/node.rs new file mode 100644 index 0000000000000..3ee2c97824fb4 --- /dev/null +++ b/crates/bevy_ecs/src/schedule/graph/node.rs @@ -0,0 +1,59 @@ +use core::fmt::Debug; + +/// Unique identifier for a system or system set stored in a [`ScheduleGraph`]. +/// +/// [`ScheduleGraph`]: crate::schedule::ScheduleGraph +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum NodeId { + /// Identifier for a system. + System(usize), + /// Identifier for a system set. + Set(usize), +} + +impl PartialOrd for NodeId { + fn partial_cmp(&self, other: &Self) -> Option { + Some(Ord::cmp(self, other)) + } +} + +impl Ord for NodeId { + fn cmp(&self, other: &Self) -> core::cmp::Ordering { + self.cmp(other) + } +} + +impl NodeId { + /// Returns the internal integer value. + pub(crate) 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(_)) + } + + /// Returns `true` if the identified node is a system set. + pub const fn is_set(&self) -> bool { + 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}; + + 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, + } + } +} diff --git a/crates/bevy_ecs/src/schedule/graph/tarjan_scc.rs b/crates/bevy_ecs/src/schedule/graph/tarjan_scc.rs new file mode 100644 index 0000000000000..5718dd2cfb83b --- /dev/null +++ b/crates/bevy_ecs/src/schedule/graph/tarjan_scc.rs @@ -0,0 +1,223 @@ +use super::DiGraph; +use super::NodeId; +use alloc::vec::Vec; +use core::hash::BuildHasher; +use core::num::NonZeroUsize; +use smallvec::SmallVec; + +/// Create an iterator over *strongly connected components* using Algorithm 3 in +/// [A Space-Efficient Algorithm for Finding Strongly Connected Components][1] by David J. Pierce, +/// which is a memory-efficient variation of [Tarjan's algorithm][2]. +/// +/// +/// [1]: https://homepages.ecs.vuw.ac.nz/~djp/files/P05.pdf +/// [2]: https://en.wikipedia.org/wiki/Tarjan%27s_strongly_connected_components_algorithm +/// +/// Returns each strongly strongly connected component (scc). +/// The order of node ids within each scc is arbitrary, but the order of +/// the sccs is their postorder (reverse topological sort). +pub(crate) fn new_tarjan_scc( + graph: &DiGraph, +) -> impl Iterator> + '_ { + // Create a list of all nodes we need to visit. + let unchecked_nodes = graph.nodes(); + + // For each node we need to visit, we also need to visit its neighbors. + // Storing the iterator for each set of neighbors allows this list to be computed without + // an additional allocation. + let nodes = graph + .nodes() + .map(|node| NodeData { + root_index: None, + neighbors: graph.neighbors(node), + }) + .collect::>(); + + TarjanScc { + graph, + unchecked_nodes, + index: 1, // Invariant: index < component_count at all times. + component_count: usize::MAX, // Will hold if component_count is initialized to number of nodes - 1 or higher. + nodes, + stack: Vec::new(), + visitation_stack: Vec::new(), + start: None, + index_adjustment: None, + } +} + +struct NodeData> { + root_index: Option, + neighbors: N, +} + +/// A state for computing the *strongly connected components* using [Tarjan's algorithm][1]. +/// +/// This is based on [`TarjanScc`] from [`petgraph`]. +/// +/// [1]: https://en.wikipedia.org/wiki/Tarjan%27s_strongly_connected_components_algorithm +/// [`petgraph`]: https://docs.rs/petgraph/0.6.5/petgraph/ +/// [`TarjanScc`]: https://docs.rs/petgraph/0.6.5/petgraph/algo/struct.TarjanScc.html +struct TarjanScc<'graph, Hasher, AllNodes, Neighbors> +where + Hasher: BuildHasher, + AllNodes: Iterator, + Neighbors: Iterator, +{ + /// Source of truth [`DiGraph`] + graph: &'graph DiGraph, + /// An [`Iterator`] of [`NodeId`]s from the `graph` which may not have been visited yet. + unchecked_nodes: AllNodes, + /// The index of the next SCC + index: usize, + /// A count of potentially remaining SCCs + component_count: usize, + /// Information about each [`NodeId`], including a possible SCC index and an + /// [`Iterator`] of possibly unvisited neighbors. + nodes: Vec>, + /// A stack of [`NodeId`]s where a SCC will be found starting at the top of the stack. + stack: Vec, + /// A stack of [`NodeId`]s which need to be visited to determine which SCC they belong to. + visitation_stack: Vec<(NodeId, bool)>, + /// An index into the `stack` indicating the starting point of a SCC. + start: Option, + /// An adjustment to the `index` which will be applied once the current SCC is found. + index_adjustment: Option, +} + +impl<'graph, S: BuildHasher, A: Iterator, N: Iterator> + TarjanScc<'graph, S, A, N> +{ + /// Compute the next *strongly connected component* using Algorithm 3 in + /// [A Space-Efficient Algorithm for Finding Strongly Connected Components][1] by David J. Pierce, + /// which is a memory-efficient variation of [Tarjan's algorithm][2]. + /// + /// + /// [1]: https://homepages.ecs.vuw.ac.nz/~djp/files/P05.pdf + /// [2]: https://en.wikipedia.org/wiki/Tarjan%27s_strongly_connected_components_algorithm + /// + /// Returns `Some` for each strongly strongly connected component (scc). + /// The order of node ids within each scc is arbitrary, but the order of + /// the sccs is their postorder (reverse topological sort). + fn next_scc(&mut self) -> Option<&[NodeId]> { + // Cleanup from possible previous iteration + if let (Some(start), Some(index_adjustment)) = + (self.start.take(), self.index_adjustment.take()) + { + self.stack.truncate(start); + self.index -= index_adjustment; // Backtrack index back to where it was before we ever encountered the component. + self.component_count -= 1; + } + + loop { + // If there are items on the visitation stack, then we haven't finished visiting + // the node at the bottom of the stack yet. + // Must visit all nodes in the stack from top to bottom before visiting the next node. + while let Some((v, v_is_local_root)) = self.visitation_stack.pop() { + // If this visitation finds a complete SCC, return it immediately. + if let Some(start) = self.visit_once(v, v_is_local_root) { + return Some(&self.stack[start..]); + }; + } + + // Get the next node to check, otherwise we're done and can return None. + let Some(node) = self.unchecked_nodes.next() else { + break None; + }; + + let visited = self.nodes[self.graph.to_index(node)].root_index.is_some(); + + // If this node hasn't already been visited (e.g., it was the neighbor of a previously checked node) + // add it to the visitation stack. + if !visited { + self.visitation_stack.push((node, true)); + } + } + } + + /// Attempt to find the starting point on the stack for a new SCC without visiting neighbors. + /// If a visitation is required, this will return `None` and mark the required neighbor and the + /// current node as in need of visitation again. + /// If no SCC can be found in the current visitation stack, returns `None`. + fn visit_once(&mut self, v: NodeId, mut v_is_local_root: bool) -> Option { + let node_v = &mut self.nodes[self.graph.to_index(v)]; + + if node_v.root_index.is_none() { + let v_index = self.index; + node_v.root_index = NonZeroUsize::new(v_index); + self.index += 1; + } + + while let Some(w) = self.nodes[self.graph.to_index(v)].neighbors.next() { + // If a neighbor hasn't been visited yet... + if self.nodes[self.graph.to_index(w)].root_index.is_none() { + // Push the current node and the neighbor back onto the visitation stack. + // On the next execution of `visit_once`, the neighbor will be visited. + self.visitation_stack.push((v, v_is_local_root)); + self.visitation_stack.push((w, true)); + + return None; + } + + if self.nodes[self.graph.to_index(w)].root_index + < self.nodes[self.graph.to_index(v)].root_index + { + self.nodes[self.graph.to_index(v)].root_index = + self.nodes[self.graph.to_index(w)].root_index; + v_is_local_root = false; + } + } + + if !v_is_local_root { + self.stack.push(v); // Stack is filled up when backtracking, unlike in Tarjans original algorithm. + return None; + } + + // Pop the stack and generate an SCC. + let mut index_adjustment = 1; + let c = NonZeroUsize::new(self.component_count); + let nodes = &mut self.nodes; + let start = self + .stack + .iter() + .rposition(|&w| { + if nodes[self.graph.to_index(v)].root_index + > nodes[self.graph.to_index(w)].root_index + { + true + } else { + nodes[self.graph.to_index(w)].root_index = c; + index_adjustment += 1; + false + } + }) + .map(|x| x + 1) + .unwrap_or_default(); + nodes[self.graph.to_index(v)].root_index = c; + self.stack.push(v); // Pushing the component root to the back right before getting rid of it is somewhat ugly, but it lets it be included in f. + + self.start = Some(start); + self.index_adjustment = Some(index_adjustment); + + Some(start) + } +} + +impl<'graph, S: BuildHasher, A: Iterator, N: Iterator> Iterator + for TarjanScc<'graph, S, A, N> +{ + // It is expected that the `DiGraph` is sparse, and as such wont have many large SCCs. + // Returning a `SmallVec` allows this iterator to skip allocation in cases where that + // assumption holds. + type Item = SmallVec<[NodeId; 4]>; + + fn next(&mut self) -> Option { + let next = SmallVec::from_slice(self.next_scc()?); + Some(next) + } + + fn size_hint(&self) -> (usize, Option) { + // There can be no more than the number of nodes in a graph worth of SCCs + (0, Some(self.nodes.len())) + } +} diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 73689d57ce57b..dc2a907949bd1 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -3,16 +3,16 @@ mod condition; mod config; mod executor; -mod graph_utils; +mod graph; #[allow(clippy::module_inception)] mod schedule; mod set; mod stepping; -use self::graph_utils::*; +use self::graph::*; pub use self::{condition::*, config::*, executor::*, schedule::*, set::*}; -pub use self::graph_utils::NodeId; +pub use self::graph::NodeId; #[cfg(test)] mod tests { diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index b00304ae1462d..6ce385bade128 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -1,17 +1,17 @@ use alloc::collections::BTreeSet; use core::fmt::{Debug, Write}; +use core::hash::BuildHasherDefault; #[cfg(feature = "trace")] use bevy_utils::tracing::info_span; use bevy_utils::{ default, tracing::{error, info, warn}, - HashMap, HashSet, + AHasher, HashMap, HashSet, }; use derive_more::derive::{Display, Error}; use disqualified::ShortName; use fixedbitset::FixedBitSet; -use petgraph::{algo::TarjanScc, prelude::*}; use crate::{ self as bevy_ecs, @@ -24,6 +24,7 @@ use crate::{ use crate::{query::AccessConflicts, storage::SparseSetIndex}; pub use stepping::Stepping; +use Direction::{Incoming, Outgoing}; /// Resource that stores [`Schedule`]s mapped to [`ScheduleLabel`]s excluding the current running [`Schedule`]. #[derive(Default, Resource)] @@ -327,7 +328,7 @@ impl Schedule { ); }; - self.graph.ambiguous_with.add_edge(a_id, b_id, ()); + self.graph.ambiguous_with.add_edge(a_id, b_id); self } @@ -513,7 +514,7 @@ impl Schedule { #[derive(Default)] pub struct Dag { /// A directed graph. - graph: DiGraphMap, + graph: DiGraph>, /// A cached topological ordering of the graph. topsort: Vec, } @@ -521,13 +522,13 @@ pub struct Dag { impl Dag { fn new() -> Self { Self { - graph: DiGraphMap::new(), + graph: DiGraph::new(), topsort: Vec::new(), } } /// The directed graph of the stored systems, connected by their ordering dependencies. - pub fn graph(&self) -> &DiGraphMap { + pub fn graph(&self) -> &DiGraph { &self.graph } @@ -606,7 +607,7 @@ pub struct ScheduleGraph { hierarchy: Dag, /// Directed acyclic graph of the dependency (which systems/sets have to run before which other systems/sets) dependency: Dag, - ambiguous_with: UnGraphMap, + ambiguous_with: UnGraph>, ambiguous_with_all: HashSet, conflicting_systems: Vec<(NodeId, NodeId, Vec)>, anonymous_sets: usize, @@ -629,7 +630,7 @@ impl ScheduleGraph { uninit: Vec::new(), hierarchy: Dag::new(), dependency: Dag::new(), - ambiguous_with: UnGraphMap::new(), + ambiguous_with: UnGraph::new(), ambiguous_with_all: HashSet::new(), conflicting_systems: Vec::new(), anonymous_sets: 0, @@ -832,7 +833,7 @@ impl ScheduleGraph { for current_node in current_nodes { self.dependency .graph - .add_edge(*previous_node, *current_node, ()); + .add_edge(*previous_node, *current_node); if ignore_deferred { self.no_sync_edges.insert((*previous_node, *current_node)); @@ -1003,7 +1004,7 @@ impl ScheduleGraph { 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, ()); + self.hierarchy.graph.add_edge(set, id); // ensure set also appears in dependency graph self.dependency.graph.add_node(set); @@ -1025,7 +1026,7 @@ impl ScheduleGraph { (set, id) } }; - self.dependency.graph.add_edge(lhs, rhs, ()); + self.dependency.graph.add_edge(lhs, rhs); // ensure set also appears in hierarchy graph self.hierarchy.graph.add_node(set); @@ -1038,7 +1039,7 @@ impl ScheduleGraph { .into_iter() .map(|set| self.system_set_ids[&set]) { - self.ambiguous_with.add_edge(id, set, ()); + self.ambiguous_with.add_edge(id, set); } } Ambiguity::IgnoreAll => { @@ -1146,8 +1147,8 @@ impl ScheduleGraph { // modify the graph to have sync nodes for any dependents after a system with deferred system params fn auto_insert_apply_deferred( &mut self, - dependency_flattened: &mut GraphMap, - ) -> Result, ScheduleBuildError> { + dependency_flattened: &mut DiGraph, + ) -> Result { let mut sync_point_graph = dependency_flattened.clone(); let topo = self.topsort_graph(dependency_flattened, ReportCycles::Dependency)?; @@ -1178,8 +1179,8 @@ impl ScheduleGraph { if add_sync_on_edge { let sync_point = self.get_sync_point(distances[&target.index()].unwrap()); - sync_point_graph.add_edge(*node, sync_point, ()); - sync_point_graph.add_edge(sync_point, target, ()); + sync_point_graph.add_edge(*node, sync_point); + sync_point_graph.add_edge(sync_point, target); // edge is now redundant sync_point_graph.remove_edge(*node, target); @@ -1227,7 +1228,7 @@ impl ScheduleGraph { fn map_sets_to_systems( &self, hierarchy_topsort: &[NodeId], - hierarchy_graph: &GraphMap, + hierarchy_graph: &DiGraph, ) -> (HashMap>, HashMap) { let mut set_systems: HashMap> = HashMap::with_capacity(self.system_sets.len()); @@ -1261,10 +1262,7 @@ impl ScheduleGraph { (set_systems, set_system_bitsets) } - fn get_dependency_flattened( - &mut self, - set_systems: &HashMap>, - ) -> GraphMap { + 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(); @@ -1305,37 +1303,34 @@ impl ScheduleGraph { dependency_flattened.remove_node(set); for (a, b) in temp.drain(..) { - dependency_flattened.add_edge(a, b, ()); + dependency_flattened.add_edge(a, b); } } dependency_flattened } - fn get_ambiguous_with_flattened( - &self, - set_systems: &HashMap>, - ) -> GraphMap { - let mut ambiguous_with_flattened = UnGraphMap::new(); - for (lhs, rhs, _) in self.ambiguous_with.all_edges() { + fn get_ambiguous_with_flattened(&self, set_systems: &HashMap>) -> UnGraph { + let mut ambiguous_with_flattened = UnGraph::new(); + for (lhs, rhs) in self.ambiguous_with.all_edges() { match (lhs, rhs) { (NodeId::System(_), NodeId::System(_)) => { - ambiguous_with_flattened.add_edge(lhs, rhs, ()); + ambiguous_with_flattened.add_edge(lhs, rhs); } (NodeId::Set(_), 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(lhs_, rhs); } } (NodeId::System(_), NodeId::Set(_)) => { for &rhs_ in set_systems.get(&rhs).unwrap_or(&Vec::new()) { - ambiguous_with_flattened.add_edge(lhs, rhs_, ()); + ambiguous_with_flattened.add_edge(lhs, rhs_); } } (NodeId::Set(_), NodeId::Set(_)) => { 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(lhs_, rhs_); } } } @@ -1348,7 +1343,7 @@ impl ScheduleGraph { fn get_conflicting_systems( &self, flat_results_disconnected: &Vec<(NodeId, NodeId)>, - ambiguous_with_flattened: &GraphMap, + ambiguous_with_flattened: &UnGraph, ignored_ambiguities: &BTreeSet, ) -> Vec<(NodeId, NodeId, Vec)> { let mut conflicting_systems = Vec::new(); @@ -1624,7 +1619,7 @@ impl ScheduleGraph { .graph .edges_directed(*id, Outgoing) // never get the sets of the members or this will infinite recurse when the report_sets setting is on. - .map(|(_, member_id, _)| self.get_node_name_inner(&member_id, false)) + .map(|(_, member_id)| self.get_node_name_inner(&member_id, false)) .reduce(|a, b| format!("{a}, {b}")) .unwrap_or_default() ) @@ -1692,23 +1687,22 @@ impl ScheduleGraph { /// If the graph contain cycles, then an error is returned. fn topsort_graph( &self, - graph: &DiGraphMap, + graph: &DiGraph, report: ReportCycles, ) -> Result, ScheduleBuildError> { // Tarjan's SCC algorithm returns elements in *reverse* topological order. - let mut tarjan_scc = TarjanScc::new(); let mut top_sorted_nodes = Vec::with_capacity(graph.node_count()); let mut sccs_with_cycles = Vec::new(); - tarjan_scc.run(graph, |scc| { + for scc in graph.iter_sccs() { // A strongly-connected component is a group of nodes who can all reach each other // through one or more paths. If an SCC contains more than one node, there must be // at least one cycle within them. + top_sorted_nodes.extend_from_slice(&scc); if scc.len() > 1 { - sccs_with_cycles.push(scc.to_vec()); + sccs_with_cycles.push(scc); } - top_sorted_nodes.extend_from_slice(scc); - }); + } if sccs_with_cycles.is_empty() { // reverse to get topological order @@ -1781,7 +1775,7 @@ impl ScheduleGraph { fn check_for_cross_dependencies( &self, - dep_results: &CheckGraphResults, + dep_results: &CheckGraphResults, hier_results_connected: &HashSet<(NodeId, NodeId)>, ) -> Result<(), ScheduleBuildError> { for &(a, b) in &dep_results.connected { @@ -1917,7 +1911,7 @@ impl ScheduleGraph { } fn traverse_sets_containing_node(&self, id: NodeId, f: &mut impl FnMut(NodeId) -> 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) { self.traverse_sets_containing_node(set_id, f); }