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

Remove petgraph from bevy_ecs (#15519)

Browse Source 
# Objective

- Contributes to #15460

## Solution

- Removed `petgraph` as a dependency from the `bevy_ecs` crate.
- Replaced `TarjanScc` and `GraphMap` with specialised in-tree
alternatives.

## Testing

- Ran CI locally.
- Added new unit tests to check ordering invariants.
- Confirmed `petgraph` is no longer present in `cargo tree -p bevy_ecs`

## Migration Guide

The `Dag::graph` method no longer returns a `petgraph` `DiGraph` and
instead returns the new `DiGraph` type within `bevy_ecs`. Edge and node
iteration methods are provided so conversion to the `petgraph` type
should be trivial if required.

## Notes

- `indexmap` was already in the dependency graph for `bevy_ecs`, so its
inclusion here makes no difference to compilation time for Bevy.
- The implementation for `Graph` is heavily inspired from the `petgraph`
original, with specialisations added to simplify and improve the type.
- `petgraph` does have public plans for `no_std` support, however there
is no timeframe on if or when that functionality will be available.
Moving to an in-house solution in the interim allows Bevy to continue
developing its `no_std` offerings and further explore alternate graphing
options.

---------

Co-authored-by: Lixou <82600264+DasLixou@users.noreply.github.com>
Co-authored-by: vero <11307157+atlv24@users.noreply.github.com>
This commit is contained in:
Zachary Harrold 2024-12-04 07:01:55 +11:00 committed by GitHub
parent 410f3c478a
commit c9fa975977
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 843 additions and 118 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crates/bevy_ecs/Cargo.toml
View File

@ -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]

2
crates/bevy_ecs/src/schedule/config.rs
View File

@ -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,
},

480
crates/bevy_ecs/src/schedule/graph/graph_map.rs Normal file
View File

@ -0,0 +1,480 @@
//! `Graph<DIRECTED>` 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<S = BuildHasherDefault<AHasher>> = Graph<false, S>;
/// 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<S = BuildHasherDefault<AHasher>> = Graph<true, S>;
/// `Graph<DIRECTED>` 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<const DIRECTED: bool, S = BuildHasherDefault<AHasher>>
where
S: BuildHasher,
{
nodes: IndexMap<NodeId, Vec<CompactNodeIdAndDirection>, S>,
edges: HashSet<CompactNodeIdPair, S>,
}
impl<const DIRECTED: bool, S: BuildHasher> fmt::Debug for Graph<DIRECTED, S> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
self.nodes.fmt(f)
}
}
impl<const DIRECTED: bool, S> Graph<DIRECTED, S>
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<Item = NodeId> + ExactSizeIterator<Item = NodeId> + '_ {
self.nodes.keys().copied()
}
/// Return an iterator of all nodes with an edge starting from `a`.
pub fn neighbors(&self, a: NodeId) -> impl DoubleEndedIterator<Item = NodeId> + '_ {
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<Item = NodeId> + '_ {
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<Item = (NodeId, NodeId)> + '_ {
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<Item = (NodeId, NodeId)> + '_ {
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<Item = (NodeId, NodeId)> + '_ {
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<const DIRECTED: bool, S> Default for Graph<DIRECTED, S>
where
S: BuildHasher + Default,
{
fn default() -> Self {
Self::with_capacity(0, 0)
}
}
impl<S: BuildHasher> Graph<true, S> {
/// Iterate over all *Strongly Connected Components* in this graph.
pub(crate) fn iter_sccs(&self) -> impl Iterator<Item = SmallVec<[NodeId; 4]>> + '_ {
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::<true>::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<_>>(),
vec![System(1), System(2), System(3), System(4)]
);
graph.remove_node(System(1));
assert_eq!(
graph.nodes().collect::<Vec<_>>(),
vec![System(4), System(2), System(3)]
);
graph.remove_node(System(4));
assert_eq!(
graph.nodes().collect::<Vec<_>>(),
vec![System(3), System(2)]
);
graph.remove_node(System(2));
assert_eq!(graph.nodes().collect::<Vec<_>>(), vec![System(3)]);
graph.remove_node(System(3));
assert_eq!(graph.nodes().collect::<Vec<_>>(), 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::<true>::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::<Vec<_>>();
assert_eq!(
sccs,
vec![
vec![System(3), System(2), System(1)],
vec![System(5), System(4)],
vec![System(6)]
]
);
}
}

111
crates/bevy_ecs/src/schedule/graph_utils.rs → crates/bevy_ecs/src/schedule/graph/mod.rs
View File

@ -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),
}
mod graph_map;
mod node;
mod tarjan_scc;
impl NodeId {
/// Returns the internal integer value.
pub(crate) 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(_))
}
}
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<V> {
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<V, ()>,
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<V, ()>,
pub(crate) transitive_closure: DiGraph,
}
impl<V: NodeTrait + Debug> Default for CheckGraphResults<V> {
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<V: NodeTrait + Debug> Default for CheckGraphResults<V> {
/// ["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<V>(
graph: &DiGraphMap<V, ()>,
topological_order: &[V],
) -> CheckGraphResults<V>
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::<V, ()>::new();
let mut topsorted = DiGraph::<BuildHasherDefault<AHasher>>::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::<V, ()>::new();
let mut transitive_closure = DiGraphMap::<V, ()>::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 <a, b> 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::<Vec<_>>();
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<N>(graph: &DiGraphMap<N, ()>, scc: &[N]) -> Vec<Vec<N>>
where
N: NodeTrait + Debug,
{
pub fn simple_cycles_in_component(graph: &DiGraph, scc: &[NodeId]) -> Vec<Vec<NodeId>> {
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::<BuildHasherDefault<AHasher>>::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<N, HashSet<N>> =
let mut unblock_together: HashMap<NodeId, HashSet<NodeId>> =
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<N> = HashSet::with_capacity(subgraph.node_count());
let mut maybe_in_more_cycles: HashSet<NodeId> =
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

59
crates/bevy_ecs/src/schedule/graph/node.rs Normal file
View File

@ -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<core::cmp::Ordering> {
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,
}
}
}

223
crates/bevy_ecs/src/schedule/graph/tarjan_scc.rs Normal file
View File

@ -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<S: BuildHasher>(
graph: &DiGraph<S>,
) -> impl Iterator<Item = SmallVec<[NodeId; 4]>> + '_ {
// 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::<Vec<_>>();
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<N: Iterator<Item = NodeId>> {
root_index: Option<NonZeroUsize>,
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<Item = NodeId>,
Neighbors: Iterator<Item = NodeId>,
{
/// Source of truth [`DiGraph`]
graph: &'graph DiGraph<Hasher>,
/// 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<NodeData<Neighbors>>,
/// A stack of [`NodeId`]s where a SCC will be found starting at the top of the stack.
stack: Vec<NodeId>,
/// 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<usize>,
/// An adjustment to the `index` which will be applied once the current SCC is found.
index_adjustment: Option<usize>,
}
impl<'graph, S: BuildHasher, A: Iterator<Item = NodeId>, N: Iterator<Item = NodeId>>
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<usize> {
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<Item = NodeId>, N: Iterator<Item = NodeId>> 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<Self::Item> {
let next = SmallVec::from_slice(self.next_scc()?);
Some(next)
}
fn size_hint(&self) -> (usize, Option<usize>) {
// There can be no more than the number of nodes in a graph worth of SCCs
(0, Some(self.nodes.len()))
}
}

6
crates/bevy_ecs/src/schedule/mod.rs
View File

@ -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 {

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

@ -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<NodeId, ()>,
graph: DiGraph<BuildHasherDefault<AHasher>>,
/// A cached topological ordering of the graph.
topsort: Vec<NodeId>,
}
@ -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<NodeId, ()> {
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<NodeId, ()>,
ambiguous_with: UnGraph<BuildHasherDefault<AHasher>>,
ambiguous_with_all: HashSet<NodeId>,
conflicting_systems: Vec<(NodeId, NodeId, Vec<ComponentId>)>,
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<NodeId, (), Directed>,
) -> Result<GraphMap<NodeId, (), Directed>, ScheduleBuildError> {
dependency_flattened: &mut DiGraph,
) -> Result<DiGraph, ScheduleBuildError> {
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<NodeId, (), Directed>,
hierarchy_graph: &DiGraph,
) -> (HashMap<NodeId, Vec<NodeId>>, HashMap<NodeId, FixedBitSet>) {
let mut set_systems: HashMap<NodeId, Vec<NodeId>> =
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<NodeId, Vec<NodeId>>,
) -> GraphMap<NodeId, (), Directed> {
fn get_dependency_flattened(&mut self, set_systems: &HashMap<NodeId, Vec<NodeId>>) -> 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<NodeId, Vec<NodeId>>,
) -> GraphMap<NodeId, (), Undirected> {
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<NodeId, Vec<NodeId>>) -> 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<NodeId, (), Undirected>,
ambiguous_with_flattened: &UnGraph,
ignored_ambiguities: &BTreeSet<ComponentId>,
) -> Vec<(NodeId, NodeId, Vec<ComponentId>)> {
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<NodeId, ()>,
graph: &DiGraph,
report: ReportCycles,
) -> Result<Vec<NodeId>, 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<NodeId>,
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);
}