Skip to content

Commit 0235bdb

Browse files
authored
Replace BTreeMap with IndexMap for stored ScheduleBuildPasses (#21930)
# Objective - Part of #20115 We currently store schedule build passes as a `BTreeMap<TypeId, _>`; however `TypeId` doesn't have a useful ordering for us, and is subject to change across compiles. ## Solution Switch to a `IndexMap<TypeId, _>`, so we can iterate in insertion order instead. ## Testing Added a test to ensure consistent iteration order.
1 parent cb719be commit 0235bdb

File tree

1 file changed

+64
-6
lines changed

1 file changed

+64
-6
lines changed

crates/bevy_ecs/src/schedule/schedule.rs

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,23 @@
44
)]
55
use alloc::{
66
boxed::Box,
7-
collections::{BTreeMap, BTreeSet},
7+
collections::BTreeSet,
88
format,
99
string::{String, ToString},
1010
vec,
1111
vec::Vec,
1212
};
13-
use bevy_platform::collections::{HashMap, HashSet};
13+
use bevy_platform::{
14+
collections::{HashMap, HashSet},
15+
hash::FixedHasher,
16+
};
1417
use bevy_utils::{default, TypeIdMap};
1518
use core::{
1619
any::{Any, TypeId},
1720
fmt::{Debug, Write},
1821
};
1922
use fixedbitset::FixedBitSet;
23+
use indexmap::IndexMap;
2024
use log::{info, warn};
2125
use pass::ScheduleBuildPassObj;
2226
use thiserror::Error;
@@ -465,7 +469,7 @@ impl Schedule {
465469

466470
/// Remove a custom build pass.
467471
pub fn remove_build_pass<T: ScheduleBuildPass>(&mut self) {
468-
self.graph.passes.remove(&TypeId::of::<T>());
472+
self.graph.passes.shift_remove(&TypeId::of::<T>());
469473
}
470474

471475
/// Changes miscellaneous build settings.
@@ -697,7 +701,7 @@ pub struct ScheduleGraph {
697701
anonymous_sets: usize,
698702
changed: bool,
699703
settings: ScheduleBuildSettings,
700-
passes: BTreeMap<TypeId, Box<dyn ScheduleBuildPassObj>>,
704+
passes: IndexMap<TypeId, Box<dyn ScheduleBuildPassObj>, FixedHasher>,
701705
}
702706

703707
impl ScheduleGraph {
@@ -1586,14 +1590,17 @@ pub struct ScheduleNotInitialized;
15861590

15871591
#[cfg(test)]
15881592
mod tests {
1593+
use alloc::{vec, vec::Vec};
1594+
use core::any::TypeId;
1595+
15891596
use bevy_ecs_macros::ScheduleLabel;
15901597

15911598
use crate::{
15921599
error::{ignore, panic, DefaultErrorHandler, Result},
15931600
prelude::{ApplyDeferred, IntoSystemSet, Res, Resource},
15941601
schedule::{
1595-
tests::ResMut, IntoScheduleConfigs, Schedule, ScheduleBuildSettings,
1596-
ScheduleCleanupPolicy, SystemSet,
1602+
passes::AutoInsertApplyDeferredPass, tests::ResMut, IntoScheduleConfigs, Schedule,
1603+
ScheduleBuildPass, ScheduleBuildSettings, ScheduleCleanupPolicy, SystemSet,
15971604
},
15981605
system::Commands,
15991606
world::World,
@@ -2571,4 +2578,55 @@ mod tests {
25712578
let conflicts = schedule.graph().conflicting_systems();
25722579
assert!(conflicts.is_empty());
25732580
}
2581+
2582+
#[test]
2583+
fn build_pass_iteration_order() {
2584+
#[derive(Debug)]
2585+
struct Pass<const N: usize>;
2586+
2587+
impl<const N: usize> ScheduleBuildPass for Pass<N> {
2588+
type EdgeOptions = ();
2589+
fn add_dependency(
2590+
&mut self,
2591+
_from: crate::schedule::NodeId,
2592+
_to: crate::schedule::NodeId,
2593+
_options: Option<&Self::EdgeOptions>,
2594+
) {
2595+
}
2596+
fn build(
2597+
&mut self,
2598+
_world: &mut World,
2599+
_graph: &mut super::ScheduleGraph,
2600+
_dependency_flattened: &mut crate::schedule::graph::Dag<crate::schedule::SystemKey>,
2601+
) -> core::result::Result<(), crate::schedule::ScheduleBuildError> {
2602+
Ok(())
2603+
}
2604+
fn collapse_set(
2605+
&mut self,
2606+
_set: crate::schedule::SystemSetKey,
2607+
_systems: &bevy_platform::collections::HashSet<crate::schedule::SystemKey>,
2608+
_dependency_flattening: &crate::schedule::graph::DiGraph<crate::schedule::NodeId>,
2609+
) -> impl Iterator<Item = (crate::schedule::NodeId, crate::schedule::NodeId)>
2610+
{
2611+
core::iter::empty()
2612+
}
2613+
}
2614+
2615+
let mut schedule = Schedule::default();
2616+
schedule.add_build_pass(Pass::<0>);
2617+
schedule.add_build_pass(Pass::<1>);
2618+
schedule.add_build_pass(Pass::<2>);
2619+
2620+
let pass_order: Vec<TypeId> = schedule.graph().passes.keys().cloned().collect();
2621+
2622+
assert_eq!(
2623+
pass_order,
2624+
vec![
2625+
TypeId::of::<AutoInsertApplyDeferredPass>(),
2626+
TypeId::of::<Pass<0>>(),
2627+
TypeId::of::<Pass<1>>(),
2628+
TypeId::of::<Pass<2>>()
2629+
]
2630+
);
2631+
}
25742632
}

0 commit comments

Comments
 (0)