From 91a5e3585decd70f1f9c7e2c20261c1304799cdd Mon Sep 17 00:00:00 2001 From: Afonso Lage Date: Thu, 2 Sep 2021 18:16:18 -0300 Subject: [PATCH 001/113] Number the reported ambiguities, to make them easier to discuss in a team --- crates/bevy_ecs/src/schedule/stage.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index c2a040d648517..a48d5c5b0c125 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -501,11 +501,14 @@ impl SystemStage { systems: &[impl SystemContainer], mut ambiguities: Vec<(usize, usize, Vec)>, world: &World, + output_prefix: &str, ) { - for (index_a, index_b, conflicts) in ambiguities.drain(..) { + for (idx, (index_a, index_b, conflicts)) in ambiguities.drain(..).enumerate() { writeln!( string, - " -- {:?} and {:?}", + "{}.{} - {:?} and {:?}", + output_prefix, + idx, systems[index_a].name(), systems[index_b].name() ) @@ -532,30 +535,32 @@ impl SystemStage { 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); + writeln!(string, "1. Parallel systems:").unwrap(); + write_display_names_of_pairs(&mut string, &self.parallel, parallel, world, "1"); } if !at_start.is_empty() { - writeln!(string, " * Exclusive systems at start of stage:").unwrap(); + writeln!(string, "2. Exclusive systems at start of stage:").unwrap(); write_display_names_of_pairs( &mut string, &self.exclusive_at_start, at_start, world, + "1", ); } if !before_commands.is_empty() { - writeln!(string, " * Exclusive systems before commands of stage:").unwrap(); + writeln!(string, "3. Exclusive systems before commands of stage:").unwrap(); write_display_names_of_pairs( &mut string, &self.exclusive_before_commands, before_commands, world, + "1", ); } 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); + writeln!(string, "4. Exclusive systems at end of stage:").unwrap(); + write_display_names_of_pairs(&mut string, &self.exclusive_at_end, at_end, world,"1"); } info!("{}", string); } From 9892cc1447babdd3569802645550688b5f4d28df Mon Sep 17 00:00:00 2001 From: Afonso Lage Date: Thu, 2 Sep 2021 18:28:31 -0300 Subject: [PATCH 002/113] Fixed formatting --- crates/bevy_ecs/src/schedule/stage.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index a48d5c5b0c125..1fa452f4f3233 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -508,7 +508,7 @@ impl SystemStage { string, "{}.{} - {:?} and {:?}", output_prefix, - idx, + idx, systems[index_a].name(), systems[index_b].name() ) @@ -560,7 +560,13 @@ impl SystemStage { } if !at_end.is_empty() { writeln!(string, "4. Exclusive systems at end of stage:").unwrap(); - write_display_names_of_pairs(&mut string, &self.exclusive_at_end, at_end, world,"1"); + write_display_names_of_pairs( + &mut string, + &self.exclusive_at_end, + at_end, + world, + "1", + ); } info!("{}", string); } From 3383bf07edb5dc013e08cb6cd3da8ef577077658 Mon Sep 17 00:00:00 2001 From: Afonso Lage Date: Thu, 2 Sep 2021 19:35:11 -0300 Subject: [PATCH 003/113] Explain which components/resources the systems are conflicting on. --- crates/bevy_ecs/src/schedule/stage.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 1fa452f4f3233..7de1667af70a5 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -506,7 +506,7 @@ impl SystemStage { for (idx, (index_a, index_b, conflicts)) in ambiguities.drain(..).enumerate() { writeln!( string, - "{}.{} - {:?} and {:?}", + "{}.{} - {:?} and {:?} conflicts on the following components/resources:", output_prefix, idx, systems[index_a].name(), @@ -518,7 +518,7 @@ impl SystemStage { .iter() .map(|id| world.components().get_info(*id).unwrap().name()) .collect::>(); - writeln!(string, " conflicts: {:?}", names).unwrap(); + writeln!(string, "{:?}", names).unwrap(); } } } From 50b86c66a541853680e0999e1424af0a237000d1 Mon Sep 17 00:00:00 2001 From: Afonso Lage Date: Thu, 2 Sep 2021 20:04:19 -0300 Subject: [PATCH 004/113] Fixed wrong output_prefix --- crates/bevy_ecs/src/schedule/stage.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 7de1667af70a5..0b897778280b0 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -545,7 +545,7 @@ impl SystemStage { &self.exclusive_at_start, at_start, world, - "1", + "2", ); } if !before_commands.is_empty() { @@ -555,7 +555,7 @@ impl SystemStage { &self.exclusive_before_commands, before_commands, world, - "1", + "3", ); } if !at_end.is_empty() { @@ -565,7 +565,7 @@ impl SystemStage { &self.exclusive_at_end, at_end, world, - "1", + "4", ); } info!("{}", string); From 21aad3eb7802dbfb0a546445e9be36581dcd528c Mon Sep 17 00:00:00 2001 From: Afonso Lage Date: Thu, 2 Sep 2021 21:25:21 -0300 Subject: [PATCH 005/113] Report ambiguities where all pairwise combinations of systems conflict on the same data as a group. --- crates/bevy_ecs/src/schedule/stage.rs | 41 ++++++++++++++++++--------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 0b897778280b0..2e3f1be2604b0 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -503,23 +503,38 @@ impl SystemStage { world: &World, output_prefix: &str, ) { - for (idx, (index_a, index_b, conflicts)) in ambiguities.drain(..).enumerate() { + use std::collections::hash_map; + + let mut ambiguities_map: hash_map::HashMap, Vec> = + hash_map::HashMap::new(); + + for (index_a, index_b, components) in ambiguities.drain(..) { + let systems = ambiguities_map.entry(components).or_default(); + if !systems.contains(&index_a) { + systems.push(index_a); + } + if !systems.contains(&index_b) { + systems.push(index_b); + } + } + + for (idx, (conflicts, systems_indices)) in ambiguities_map.drain().enumerate() { + let system_names = systems_indices + .iter() + .map(|index| systems[*index].name()) + .collect::>(); + + let system_components = conflicts + .iter() + .map(|id| world.components().get_info(*id).unwrap().name()) + .collect::>(); + writeln!( string, - "{}.{} - {:?} and {:?} conflicts on the following components/resources:", - output_prefix, - idx, - systems[index_a].name(), - systems[index_b].name() + "{}.{} - Systems:\n {:?}\n - Conflicts on the following components/resources:\n {:?}", + output_prefix, idx, system_names, system_components ) .unwrap(); - if !conflicts.is_empty() { - let names = conflicts - .iter() - .map(|id| world.components().get_info(*id).unwrap().name()) - .collect::>(); - writeln!(string, "{:?}", names).unwrap(); - } } } let parallel = find_ambiguities(&self.parallel); From f0c71341efc6497fcc57dffb7204594685ca2108 Mon Sep 17 00:00:00 2001 From: Afonso Lage Date: Fri, 3 Sep 2021 11:15:54 -0300 Subject: [PATCH 006/113] Create a simple ambiguous method that causes a system to be ignored completely in the ambiguity checker. --- crates/bevy_ecs/src/schedule/stage.rs | 38 ++++++++++++++++++- .../bevy_ecs/src/schedule/system_container.rs | 15 ++++++++ .../src/schedule/system_descriptor.rs | 37 ++++++++++++++++++ 3 files changed, 89 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 2e3f1be2604b0..d481d167503d8 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -16,7 +16,7 @@ use downcast_rs::{impl_downcast, Downcast}; use fixedbitset::FixedBitSet; use std::fmt::Debug; -use super::IntoSystemDescriptor; +use super::{AmbiguityDetection, IntoSystemDescriptor}; /// A type that can run as a step of a [`Schedule`](super::Schedule). pub trait Stage: Downcast + Send + Sync { @@ -766,6 +766,16 @@ fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec< for index_b in full_bitset.difference(&relations) // .take(index_a) { + match systems[index_a].ambiguity_detection() { + AmbiguityDetection::Ignore => continue, + AmbiguityDetection::Check => (), + } + + match systems[index_b].ambiguity_detection() { + AmbiguityDetection::Ignore => continue, + AmbiguityDetection::Check => (), + } + if !processed.contains(index_b) && all_ambiguity_sets[index_a].is_disjoint(&all_ambiguity_sets[index_b]) { @@ -1983,6 +1993,32 @@ mod tests { stage.rebuild_orders_and_dependencies(); let ambiguities = find_ambiguities_first_str_labels(&stage.exclusive_at_start); assert_eq!(ambiguities.len(), 0); + + let mut stage = SystemStage::parallel() + .with_system(component.label("0")) + .with_system(resource.label("1").after("0").ambiguous()) + .with_system(empty.label("2")) + .with_system(component.label("3").after("2").before("4")) + .with_system(resource.label("4")); + stage.initialize_systems(&mut world); + stage.rebuild_orders_and_dependencies(); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); + assert!( + ambiguities.contains(&(Box::new("0"), Box::new("3"))) + || ambiguities.contains(&(Box::new("3"), Box::new("0"))) + ); + assert_eq!(ambiguities.len(), 1); + + let mut stage = SystemStage::parallel() + .with_system(component.label("0").ambiguous()) + .with_system(resource.label("1").after("0").ambiguous()) + .with_system(empty.label("2")) + .with_system(component.label("3").after("2").before("4")) + .with_system(resource.label("4")); + stage.initialize_systems(&mut world); + stage.rebuild_orders_and_dependencies(); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); + assert_eq!(ambiguities.len(), 0); } #[test] diff --git a/crates/bevy_ecs/src/schedule/system_container.rs b/crates/bevy_ecs/src/schedule/system_container.rs index 108e391d606f8..89e67f8e184e5 100644 --- a/crates/bevy_ecs/src/schedule/system_container.rs +++ b/crates/bevy_ecs/src/schedule/system_container.rs @@ -9,6 +9,8 @@ use crate::{ }; use std::borrow::Cow; +use super::AmbiguityDetection; + /// System metadata like its name, labels, order requirements and component access. pub trait SystemContainer: GraphNode