Skip to content

Commit f163649

Browse files
authored
Use component_access_set to determine the set of conflicting accesses between two systems. (#19495)
# Objective - Fixes #4381 ## Solution - Replace `component_access` with `component_access_set` when determining conflicting systems during schedule building. - All `component_access()` impls just forward to `&component_access_set().combined_access`, so we are essentially trading `Access::is_compatible` for `FilteredAccessSet::is_compatible`. - `FilteredAccessSet::get_conflicts` internally calls `combined_access.is_compatible` as the first step, so we can remove that redundant check. ## Testing - Un-ignored a previously failing test now that it passes! - Ran the `build_schedule` benchmark and got basically no change in the results. Perhaps are benchmarks are just not targetted towards this situation. ``` $ critcmp main fix-ambiguity -f 'build_schedule' group fix-ambiguity main ----- ------------- ---- build_schedule/1000_schedule 1.00 2.9±0.02s ? ?/sec 1.01 2.9±0.05s ? ?/sec build_schedule/1000_schedule_no_constraints 1.02 48.3±1.48ms ? ?/sec 1.00 47.4±1.78ms ? ?/sec build_schedule/100_schedule 1.00 9.9±0.17ms ? ?/sec 1.06 10.5±0.32ms ? ?/sec build_schedule/100_schedule_no_constraints 1.00 804.7±21.85µs ? ?/sec 1.03 828.7±19.36µs ? ?/sec build_schedule/500_schedule 1.00 451.7±7.25ms ? ?/sec 1.04 468.9±11.70ms ? ?/sec build_schedule/500_schedule_no_constraints 1.02 12.7±0.46ms ? ?/sec 1.00 12.5±0.44ms ? ?/sec ```
1 parent 064e5e4 commit f163649

File tree

2 files changed

+16
-19
lines changed

2 files changed

+16
-19
lines changed

crates/bevy_ecs/src/schedule/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,6 @@ mod tests {
874874
}
875875

876876
#[test]
877-
#[ignore = "Known failing but fix is non-trivial: https://github.com/bevyengine/bevy/issues/4381"]
878877
fn filtered_components() {
879878
let mut world = World::new();
880879
world.spawn(A);

crates/bevy_ecs/src/schedule/schedule.rs

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1418,26 +1418,24 @@ impl ScheduleGraph {
14181418
if system_a.is_exclusive() || system_b.is_exclusive() {
14191419
conflicting_systems.push((a, b, Vec::new()));
14201420
} else {
1421-
let access_a = system_a.component_access();
1422-
let access_b = system_b.component_access();
1423-
if !access_a.is_compatible(access_b) {
1424-
match access_a.get_conflicts(access_b) {
1425-
AccessConflicts::Individual(conflicts) => {
1426-
let conflicts: Vec<_> = conflicts
1427-
.ones()
1428-
.map(ComponentId::get_sparse_set_index)
1429-
.filter(|id| !ignored_ambiguities.contains(id))
1430-
.collect();
1431-
if !conflicts.is_empty() {
1432-
conflicting_systems.push((a, b, conflicts));
1433-
}
1434-
}
1435-
AccessConflicts::All => {
1436-
// there is no specific component conflicting, but the systems are overall incompatible
1437-
// for example 2 systems with `Query<EntityMut>`
1438-
conflicting_systems.push((a, b, Vec::new()));
1421+
let access_a = system_a.component_access_set();
1422+
let access_b = system_b.component_access_set();
1423+
match access_a.get_conflicts(access_b) {
1424+
AccessConflicts::Individual(conflicts) => {
1425+
let conflicts: Vec<_> = conflicts
1426+
.ones()
1427+
.map(ComponentId::get_sparse_set_index)
1428+
.filter(|id| !ignored_ambiguities.contains(id))
1429+
.collect();
1430+
if !conflicts.is_empty() {
1431+
conflicting_systems.push((a, b, conflicts));
14391432
}
14401433
}
1434+
AccessConflicts::All => {
1435+
// there is no specific component conflicting, but the systems are overall incompatible
1436+
// for example 2 systems with `Query<EntityMut>`
1437+
conflicting_systems.push((a, b, Vec::new()));
1438+
}
14411439
}
14421440
}
14431441
}

0 commit comments

Comments
 (0)