diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs new file mode 100644 index 0000000000000..bca2450eb69ea --- /dev/null +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -0,0 +1,140 @@ +use bevy_utils::tracing::info; +use fixedbitset::FixedBitSet; + +use crate::component::ComponentId; +use crate::schedule::{SystemContainer, SystemStage}; +use crate::world::World; + +impl SystemStage { + /// Logs execution order ambiguities between systems. System orders must be fresh. + pub fn report_ambiguities(&self, world: &World) { + debug_assert!(!self.systems_modified); + use std::fmt::Write; + fn write_display_names_of_pairs( + string: &mut String, + systems: &[impl SystemContainer], + mut ambiguities: Vec<(usize, usize, Vec)>, + world: &World, + ) { + for (index_a, index_b, conflicts) in ambiguities.drain(..) { + writeln!( + string, + " -- {:?} and {:?}", + systems[index_a].name(), + systems[index_b].name() + ) + .unwrap(); + if !conflicts.is_empty() { + let names = conflicts + .iter() + .map(|id| world.components().get_info(*id).unwrap().name()) + .collect::>(); + writeln!(string, " conflicts: {:?}", names).unwrap(); + } + } + } + let parallel = find_ambiguities(&self.parallel); + let at_start = find_ambiguities(&self.exclusive_at_start); + let before_commands = find_ambiguities(&self.exclusive_before_commands); + let at_end = find_ambiguities(&self.exclusive_at_end); + if !(parallel.is_empty() + && at_start.is_empty() + && before_commands.is_empty() + && at_end.is_empty()) + { + let mut string = "Execution order ambiguities detected, you might want to \ + add an explicit dependency relation between some of these systems:\n" + .to_owned(); + if !parallel.is_empty() { + writeln!(string, " * Parallel systems:").unwrap(); + write_display_names_of_pairs(&mut string, &self.parallel, parallel, world); + } + if !at_start.is_empty() { + writeln!(string, " * Exclusive systems at start of stage:").unwrap(); + write_display_names_of_pairs( + &mut string, + &self.exclusive_at_start, + at_start, + world, + ); + } + if !before_commands.is_empty() { + writeln!(string, " * Exclusive systems before commands of stage:").unwrap(); + write_display_names_of_pairs( + &mut string, + &self.exclusive_before_commands, + before_commands, + world, + ); + } + if !at_end.is_empty() { + writeln!(string, " * Exclusive systems at end of stage:").unwrap(); + write_display_names_of_pairs(&mut string, &self.exclusive_at_end, at_end, world); + } + info!("{}", string); + } + } +} + +/// Returns vector containing all pairs of indices of systems with ambiguous execution order, +/// along with specific components that have triggered the warning. +/// Systems must be topologically sorted beforehand. +fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec)> { + let mut all_dependencies = Vec::::with_capacity(systems.len()); + let mut all_dependants = Vec::::with_capacity(systems.len()); + for (index, container) in systems.iter().enumerate() { + let mut dependencies = FixedBitSet::with_capacity(systems.len()); + for &dependency in container.dependencies() { + dependencies.union_with(&all_dependencies[dependency]); + dependencies.insert(dependency); + all_dependants[dependency].insert(index); + } + + all_dependants.push(FixedBitSet::with_capacity(systems.len())); + all_dependencies.push(dependencies); + } + for index in (0..systems.len()).rev() { + let mut dependants = FixedBitSet::with_capacity(systems.len()); + for dependant in all_dependants[index].ones() { + dependants.union_with(&all_dependants[dependant]); + dependants.insert(dependant); + } + all_dependants[index] = dependants; + } + let mut all_relations = all_dependencies + .drain(..) + .zip(all_dependants.drain(..)) + .enumerate() + .map(|(index, (dependencies, dependants))| { + let mut relations = FixedBitSet::with_capacity(systems.len()); + relations.union_with(&dependencies); + relations.union_with(&dependants); + relations.insert(index); + relations + }) + .collect::>(); + let mut ambiguities = Vec::new(); + let full_bitset: FixedBitSet = (0..systems.len()).collect(); + let mut processed = FixedBitSet::with_capacity(systems.len()); + for (index_a, relations) in all_relations.drain(..).enumerate() { + // TODO: prove that `.take(index_a)` would be correct here, and uncomment it if so. + for index_b in full_bitset.difference(&relations) + // .take(index_a) + { + if !processed.contains(index_b) { + let a_access = systems[index_a].component_access(); + let b_access = systems[index_b].component_access(); + if let (Some(a), Some(b)) = (a_access, b_access) { + let conflicts = a.get_conflicts(b); + if !conflicts.is_empty() { + ambiguities.push((index_a, index_b, conflicts)); + } + } else { + ambiguities.push((index_a, index_b, Vec::new())); + } + } + } + processed.insert(index_a); + } + ambiguities +} diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index a402523c2dd7d..a6d41240e96f4 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -3,6 +3,7 @@ //! When using Bevy ECS, systems are usually not run directly, but are inserted into a //! [`Stage`], which then lives within a [`Schedule`]. +mod ambiguity_detection; mod executor; mod executor_parallel; pub mod graph_utils; diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 307f652b57fe2..14e9c622f142b 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -14,13 +14,8 @@ use crate::{ world::{World, WorldId}, }; use bevy_ecs_macros::Resource; -use bevy_utils::{ - tracing::{info, warn}, - HashMap, HashSet, -}; +use bevy_utils::{tracing::warn, HashMap, HashSet}; use downcast_rs::{impl_downcast, Downcast}; -use fixedbitset::FixedBitSet; -use std::fmt::Debug; use super::IntoSystemDescriptor; @@ -67,16 +62,16 @@ pub struct SystemStage { /// Topologically sorted run criteria of systems. run_criteria: Vec, /// Topologically sorted exclusive systems that want to be run at the start of the stage. - exclusive_at_start: Vec, + pub(super) exclusive_at_start: Vec, /// Topologically sorted exclusive systems that want to be run after parallel systems but /// before the application of their command buffers. - exclusive_before_commands: Vec, + pub(super) exclusive_before_commands: Vec, /// Topologically sorted exclusive systems that want to be run at the end of the stage. - exclusive_at_end: Vec, + pub(super) exclusive_at_end: Vec, /// Topologically sorted parallel systems. - parallel: Vec, + pub(super) parallel: Vec, /// Determines if the stage was modified and needs to rebuild its graphs and orders. - systems_modified: bool, + pub(super) systems_modified: bool, /// Determines if the stage's executor was changed. executor_modified: bool, /// Newly inserted run criteria that will be initialized at the next opportunity. @@ -453,7 +448,7 @@ impl SystemStage { && self.uninitialized_before_commands.is_empty() && self.uninitialized_at_end.is_empty() ); - fn unwrap_dependency_cycle_error( + fn unwrap_dependency_cycle_error( result: Result>, nodes: &[Node], nodes_description: &'static str, @@ -505,75 +500,6 @@ impl SystemStage { ); } - /// Logs execution order ambiguities between systems. System orders must be fresh. - fn report_ambiguities(&self, world: &World) { - debug_assert!(!self.systems_modified); - use std::fmt::Write; - fn write_display_names_of_pairs( - string: &mut String, - systems: &[impl SystemContainer], - mut ambiguities: Vec<(usize, usize, Vec)>, - world: &World, - ) { - for (index_a, index_b, conflicts) in ambiguities.drain(..) { - writeln!( - string, - " -- {:?} and {:?}", - systems[index_a].name(), - systems[index_b].name() - ) - .unwrap(); - if !conflicts.is_empty() { - let names = conflicts - .iter() - .map(|id| world.components().get_info(*id).unwrap().name()) - .collect::>(); - writeln!(string, " conflicts: {:?}", names).unwrap(); - } - } - } - let parallel = find_ambiguities(&self.parallel); - let at_start = find_ambiguities(&self.exclusive_at_start); - let before_commands = find_ambiguities(&self.exclusive_before_commands); - let at_end = find_ambiguities(&self.exclusive_at_end); - if !(parallel.is_empty() - && at_start.is_empty() - && before_commands.is_empty() - && at_end.is_empty()) - { - let mut string = "Execution order ambiguities detected, you might want to \ - add an explicit dependency relation between some of these systems:\n" - .to_owned(); - if !parallel.is_empty() { - writeln!(string, " * Parallel systems:").unwrap(); - write_display_names_of_pairs(&mut string, &self.parallel, parallel, world); - } - if !at_start.is_empty() { - writeln!(string, " * Exclusive systems at start of stage:").unwrap(); - write_display_names_of_pairs( - &mut string, - &self.exclusive_at_start, - at_start, - world, - ); - } - if !before_commands.is_empty() { - writeln!(string, " * Exclusive systems before commands of stage:").unwrap(); - write_display_names_of_pairs( - &mut string, - &self.exclusive_before_commands, - before_commands, - world, - ); - } - if !at_end.is_empty() { - writeln!(string, " * Exclusive systems at end of stage:").unwrap(); - write_display_names_of_pairs(&mut string, &self.exclusive_at_end, at_end, world); - } - info!("{}", string); - } - } - fn check_uses_resource(&self, resource_id: ComponentId, world: &World) { debug_assert!(!self.systems_modified); for system in &self.parallel { @@ -709,69 +635,6 @@ fn process_systems( Ok(()) } -/// Returns vector containing all pairs of indices of systems with ambiguous execution order, -/// along with specific components that have triggered the warning. -/// Systems must be topologically sorted beforehand. -fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec)> { - let mut all_dependencies = Vec::::with_capacity(systems.len()); - let mut all_dependants = Vec::::with_capacity(systems.len()); - for (index, container) in systems.iter().enumerate() { - let mut dependencies = FixedBitSet::with_capacity(systems.len()); - for &dependency in container.dependencies() { - dependencies.union_with(&all_dependencies[dependency]); - dependencies.insert(dependency); - all_dependants[dependency].insert(index); - } - - all_dependants.push(FixedBitSet::with_capacity(systems.len())); - all_dependencies.push(dependencies); - } - for index in (0..systems.len()).rev() { - let mut dependants = FixedBitSet::with_capacity(systems.len()); - for dependant in all_dependants[index].ones() { - dependants.union_with(&all_dependants[dependant]); - dependants.insert(dependant); - } - all_dependants[index] = dependants; - } - let mut all_relations = all_dependencies - .drain(..) - .zip(all_dependants.drain(..)) - .enumerate() - .map(|(index, (dependencies, dependants))| { - let mut relations = FixedBitSet::with_capacity(systems.len()); - relations.union_with(&dependencies); - relations.union_with(&dependants); - relations.insert(index); - relations - }) - .collect::>(); - let mut ambiguities = Vec::new(); - let full_bitset: FixedBitSet = (0..systems.len()).collect(); - let mut processed = FixedBitSet::with_capacity(systems.len()); - for (index_a, relations) in all_relations.drain(..).enumerate() { - // TODO: prove that `.take(index_a)` would be correct here, and uncomment it if so. - for index_b in full_bitset.difference(&relations) - // .take(index_a) - { - if !processed.contains(index_b) { - let a_access = systems[index_a].component_access(); - let b_access = systems[index_b].component_access(); - if let (Some(a), Some(b)) = (a_access, b_access) { - let conflicts = a.get_conflicts(b); - if !conflicts.is_empty() { - ambiguities.push((index_a, index_b, conflicts)); - } - } else { - ambiguities.push((index_a, index_b, Vec::new())); - } - } - } - processed.insert(index_a); - } - ambiguities -} - impl Stage for SystemStage { fn run(&mut self, world: &mut World) { if let Some(world_id) = self.world_id {