add schedule docs (#13174)

# Objective

I'm reading through the schedule code, which is somewhat lacking
documentation.
I've been adding some docstrings to help me understand the code; I feel
like some of them could be useful to also help others read this code.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
This commit is contained in:
Charles Bournhonesque 2024-05-02 14:31:32 -04:00 committed by GitHub
parent 6d25545c51
commit fab83471b5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 60 additions and 12 deletions

View File

@ -48,9 +48,15 @@ impl IntoSystemConfigs<()> for BoxedSystem<(), ()> {
} }
} }
/// Stores configuration for a single generic node. /// Stores configuration for a single generic node (a system or a system set)
///
/// The configuration includes the node itself, scheduling metadata
/// (hierarchy: in which sets is the node contained,
/// dependencies: before/after which other nodes should this node run)
/// and the run conditions associated with this node.
pub struct NodeConfig<T> { pub struct NodeConfig<T> {
pub(crate) node: T, pub(crate) node: T,
/// Hierarchy and dependency metadata for this node
pub(crate) graph_info: GraphInfo, pub(crate) graph_info: GraphInfo,
pub(crate) conditions: Vec<BoxedCondition>, pub(crate) conditions: Vec<BoxedCondition>,
} }
@ -83,7 +89,7 @@ impl SystemConfigs {
Self::NodeConfig(SystemConfig { Self::NodeConfig(SystemConfig {
node: system, node: system,
graph_info: GraphInfo { graph_info: GraphInfo {
sets, hierarchy: sets,
..Default::default() ..Default::default()
}, },
conditions: Vec::new(), conditions: Vec::new(),
@ -96,7 +102,7 @@ impl<T> NodeConfigs<T> {
pub fn in_set_inner(&mut self, set: InternedSystemSet) { pub fn in_set_inner(&mut self, set: InternedSystemSet) {
match self { match self {
Self::NodeConfig(config) => { Self::NodeConfig(config) => {
config.graph_info.sets.push(set); config.graph_info.hierarchy.push(set);
} }
Self::Configs { configs, .. } => { Self::Configs { configs, .. } => {
for config in configs { for config in configs {

View File

@ -62,16 +62,22 @@ pub struct SystemSchedule {
/// Indexed by system node id. /// Indexed by system node id.
pub(super) system_conditions: Vec<Vec<BoxedCondition>>, pub(super) system_conditions: Vec<Vec<BoxedCondition>>,
/// Indexed by system node id. /// Indexed by system node id.
/// Number of systems that the system immediately depends on.
pub(super) system_dependencies: Vec<usize>, pub(super) system_dependencies: Vec<usize>,
/// Indexed by system node id. /// Indexed by system node id.
/// List of systems that immediately depend on the system.
pub(super) system_dependents: Vec<Vec<usize>>, pub(super) system_dependents: Vec<Vec<usize>>,
/// Indexed by system node id. /// Indexed by system node id.
/// List of sets containing the system that have conditions
pub(super) sets_with_conditions_of_systems: Vec<FixedBitSet>, pub(super) sets_with_conditions_of_systems: Vec<FixedBitSet>,
/// List of system set node ids. /// List of system set node ids.
pub(super) set_ids: Vec<NodeId>, pub(super) set_ids: Vec<NodeId>,
/// Indexed by system set node id. /// Indexed by system set node id.
pub(super) set_conditions: Vec<Vec<BoxedCondition>>, pub(super) set_conditions: Vec<Vec<BoxedCondition>>,
/// Indexed by system set node id. /// Indexed by system set node id.
/// List of systems that are in sets that have conditions.
///
/// If a set doesn't run because of its conditions, this is used to skip all systems in it.
pub(super) systems_in_sets_with_conditions: Vec<FixedBitSet>, pub(super) systems_in_sets_with_conditions: Vec<FixedBitSet>,
} }

View File

@ -73,9 +73,12 @@ pub(crate) enum Ambiguity {
IgnoreAll, IgnoreAll,
} }
/// Metadata about how the node fits in the schedule graph
#[derive(Clone, Default)] #[derive(Clone, Default)]
pub(crate) struct GraphInfo { pub(crate) struct GraphInfo {
pub(crate) sets: Vec<InternedSystemSet>, /// the sets that the node belongs to (hierarchy)
pub(crate) hierarchy: Vec<InternedSystemSet>,
/// the sets that the node depends on (must run before or after)
pub(crate) dependencies: Vec<Dependency>, pub(crate) dependencies: Vec<Dependency>,
pub(crate) ambiguous_with: Ambiguity, pub(crate) ambiguous_with: Ambiguity,
} }

View File

@ -532,15 +532,27 @@ impl SystemNode {
} }
/// Metadata for a [`Schedule`]. /// Metadata for a [`Schedule`].
///
/// The order isn't optimized; calling `ScheduleGraph::build_schedule` will return a
/// `SystemSchedule` where the order is optimized for execution.
#[derive(Default)] #[derive(Default)]
pub struct ScheduleGraph { pub struct ScheduleGraph {
/// List of systems in the schedule
systems: Vec<SystemNode>, systems: Vec<SystemNode>,
/// List of conditions for each system, in the same order as `systems`
system_conditions: Vec<Vec<BoxedCondition>>, system_conditions: Vec<Vec<BoxedCondition>>,
/// List of system sets in the schedule
system_sets: Vec<SystemSetNode>, system_sets: Vec<SystemSetNode>,
/// List of conditions for each system set, in the same order as `system_sets`
system_set_conditions: Vec<Vec<BoxedCondition>>, system_set_conditions: Vec<Vec<BoxedCondition>>,
/// Map from system set to node id
system_set_ids: HashMap<InternedSystemSet, NodeId>, system_set_ids: HashMap<InternedSystemSet, NodeId>,
/// 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<(NodeId, usize)>,
/// Directed acyclic graph of the hierarchy (which systems/sets are children of which sets)
hierarchy: Dag, hierarchy: Dag,
/// Directed acyclic graph of the dependency (which systems/sets have to run before which other systems/sets)
dependency: Dag, dependency: Dag,
ambiguous_with: UnGraphMap<NodeId, ()>, ambiguous_with: UnGraphMap<NodeId, ()>,
ambiguous_with_all: HashSet<NodeId>, ambiguous_with_all: HashSet<NodeId>,
@ -548,6 +560,7 @@ pub struct ScheduleGraph {
anonymous_sets: usize, anonymous_sets: usize,
changed: bool, changed: bool,
settings: ScheduleBuildSettings, settings: ScheduleBuildSettings,
/// Dependency edges that will **not** automatically insert an instance of `apply_deferred` on the edge.
no_sync_edges: BTreeSet<(NodeId, NodeId)>, no_sync_edges: BTreeSet<(NodeId, NodeId)>,
auto_sync_node_ids: HashMap<u32, NodeId>, auto_sync_node_ids: HashMap<u32, NodeId>,
} }
@ -613,7 +626,7 @@ impl ScheduleGraph {
.unwrap() .unwrap()
} }
/// Returns an iterator over all systems in this schedule. /// Returns an iterator over all systems in this schedule, along with the conditions for each system.
pub fn systems( pub fn systems(
&self, &self,
) -> impl Iterator<Item = (NodeId, &dyn System<In = (), Out = ()>, &[BoxedCondition])> { ) -> impl Iterator<Item = (NodeId, &dyn System<In = (), Out = ()>, &[BoxedCondition])> {
@ -627,7 +640,8 @@ impl ScheduleGraph {
}) })
} }
/// Returns an iterator over all system sets in this schedule. /// 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<Item = (NodeId, &dyn SystemSet, &[BoxedCondition])> { pub fn system_sets(&self) -> impl Iterator<Item = (NodeId, &dyn SystemSet, &[BoxedCondition])> {
self.system_set_ids.iter().map(|(_, &node_id)| { self.system_set_ids.iter().map(|(_, &node_id)| {
let set_node = &self.system_sets[node_id.index()]; let set_node = &self.system_sets[node_id.index()];
@ -787,6 +801,7 @@ impl ScheduleGraph {
} }
} }
/// Add a [`SystemConfig`] to the graph, including its dependencies and conditions.
fn add_system_inner(&mut self, config: SystemConfig) -> Result<NodeId, ScheduleBuildError> { fn add_system_inner(&mut self, config: SystemConfig) -> Result<NodeId, ScheduleBuildError> {
let id = NodeId::System(self.systems.len()); let id = NodeId::System(self.systems.len());
@ -806,6 +821,7 @@ impl ScheduleGraph {
self.process_configs(sets.into_configs(), false); self.process_configs(sets.into_configs(), false);
} }
/// Add a single `SystemSetConfig` to the graph, including its dependencies and conditions.
fn configure_set_inner(&mut self, set: SystemSetConfig) -> Result<NodeId, ScheduleBuildError> { fn configure_set_inner(&mut self, set: SystemSetConfig) -> Result<NodeId, ScheduleBuildError> {
let SystemSetConfig { let SystemSetConfig {
node: set, node: set,
@ -837,7 +853,13 @@ impl ScheduleGraph {
id id
} }
fn check_set(&mut self, id: &NodeId, set: InternedSystemSet) -> Result<(), ScheduleBuildError> { /// 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) { match self.system_set_ids.get(&set) {
Some(set_id) => { Some(set_id) => {
if id == set_id { if id == set_id {
@ -858,18 +880,22 @@ impl ScheduleGraph {
AnonymousSet::new(id) AnonymousSet::new(id)
} }
fn check_sets( /// Check that no set is included in itself.
/// Add all the sets from the [`GraphInfo`]'s hierarchy to the graph.
fn check_hierarchy_sets(
&mut self, &mut self,
id: &NodeId, id: &NodeId,
graph_info: &GraphInfo, graph_info: &GraphInfo,
) -> Result<(), ScheduleBuildError> { ) -> Result<(), ScheduleBuildError> {
for &set in &graph_info.sets { for &set in &graph_info.hierarchy {
self.check_set(id, set)?; self.check_hierarchy_set(id, set)?;
} }
Ok(()) Ok(())
} }
/// Checks that no system set is dependent on itself.
/// Add all the sets from the [`GraphInfo`]'s dependencies to the graph.
fn check_edges( fn check_edges(
&mut self, &mut self,
id: &NodeId, id: &NodeId,
@ -899,17 +925,18 @@ impl ScheduleGraph {
Ok(()) Ok(())
} }
/// Update the internal graphs (hierarchy, dependency, ambiguity) by adding a single [`GraphInfo`]
fn update_graphs( fn update_graphs(
&mut self, &mut self,
id: NodeId, id: NodeId,
graph_info: GraphInfo, graph_info: GraphInfo,
) -> Result<(), ScheduleBuildError> { ) -> Result<(), ScheduleBuildError> {
self.check_sets(&id, &graph_info)?; self.check_hierarchy_sets(&id, &graph_info)?;
self.check_edges(&id, &graph_info)?; self.check_edges(&id, &graph_info)?;
self.changed = true; self.changed = true;
let GraphInfo { let GraphInfo {
sets, hierarchy: sets,
dependencies, dependencies,
ambiguous_with, ambiguous_with,
.. ..
@ -1137,6 +1164,9 @@ impl ScheduleGraph {
.unwrap() .unwrap()
} }
/// Return a map from system set `NodeId` to a list of system `NodeId`s that are included in the set.
/// Also return a map from system set `NodeId` to a `FixedBitSet` of system `NodeId`s that are included in the set,
/// where the bitset order is the same as `self.systems`
fn map_sets_to_systems( fn map_sets_to_systems(
&self, &self,
hierarchy_topsort: &[NodeId], hierarchy_topsort: &[NodeId],
@ -1397,6 +1427,7 @@ impl ScheduleGraph {
} }
} }
/// Updates the `SystemSchedule` from the `ScheduleGraph`.
fn update_schedule( fn update_schedule(
&mut self, &mut self,
schedule: &mut SystemSchedule, schedule: &mut SystemSchedule,

View File

@ -105,6 +105,8 @@ pub trait System: Send + Sync + 'static {
fn check_change_tick(&mut self, change_tick: Tick); fn check_change_tick(&mut self, change_tick: Tick);
/// Returns the system's default [system sets](crate::schedule::SystemSet). /// Returns the system's default [system sets](crate::schedule::SystemSet).
///
/// Each system will create a default system set that contains the system.
fn default_system_sets(&self) -> Vec<InternedSystemSet> { fn default_system_sets(&self) -> Vec<InternedSystemSet> {
Vec::new() Vec::new()
} }