Skip to content

Commit

Permalink
Stepping disabled performance fix (bevyengine#11959)
Browse files Browse the repository at this point in the history
# Objective

* Fixes bevyengine#11932 (performance impact when stepping is disabled)

## Solution

The `Option<FixedBitSet>` argument added to `ScheduleExecutor::run()` in
bevyengine#8453 caused a measurable performance impact even when stepping is
disabled. This can be seen by the benchmark of running `Schedule:run()`
on an empty schedule in a tight loop
(bevyengine#11932 (comment)).

I was able to get the same performance results as on 0.12.1 by changing
the argument
`ScheduleExecutor::run()` from `Option<FixedBitSet>` to
`Option<&FixedBitSet>`. The down-side of this change is that
`Schedule::run()` now takes about 6% longer (3.7319 ms vs 3.9855ns) when
stepping is enabled

---

## Changelog
* Change `ScheduleExecutor::run()` `_skipped_systems` from
`Option<FixedBitSet>` to `Option<&FixedBitSet>`
* Added a few benchmarks to measure `Schedule::run()` performance with
various executors
  • Loading branch information
dmlary authored and msvbg committed Feb 26, 2024
1 parent 65b15e5 commit d099f04
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 23 deletions.
1 change: 1 addition & 0 deletions benches/benches/bevy_ecs/scheduling/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ criterion_group!(
contrived,
schedule,
build_schedule,
empty_schedule_run,
);
25 changes: 25 additions & 0 deletions benches/benches/bevy_ecs/scheduling/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,28 @@ pub fn build_schedule(criterion: &mut Criterion) {

group.finish();
}

pub fn empty_schedule_run(criterion: &mut Criterion) {
let mut app = bevy_app::App::default();

let mut group = criterion.benchmark_group("run_empty_schedule");

let mut schedule = Schedule::default();
schedule.set_executor_kind(bevy_ecs::schedule::ExecutorKind::SingleThreaded);
group.bench_function("SingleThreaded", |bencher| {
bencher.iter(|| schedule.run(&mut app.world));
});

let mut schedule = Schedule::default();
schedule.set_executor_kind(bevy_ecs::schedule::ExecutorKind::MultiThreaded);
group.bench_function("MultiThreaded", |bencher| {
bencher.iter(|| schedule.run(&mut app.world));
});

let mut schedule = Schedule::default();
schedule.set_executor_kind(bevy_ecs::schedule::ExecutorKind::Simple);
group.bench_function("Simple", |bencher| {
bencher.iter(|| schedule.run(&mut app.world));
});
group.finish();
}
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/schedule/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ pub(super) trait SystemExecutor: Send + Sync {
fn run(
&mut self,
schedule: &mut SystemSchedule,
skip_systems: Option<FixedBitSet>,
world: &mut World,
skip_systems: Option<&FixedBitSet>,
);
fn set_apply_final_deferred(&mut self, value: bool);
}
Expand Down
16 changes: 4 additions & 12 deletions crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ impl SystemExecutor for MultiThreadedExecutor {
fn run(
&mut self,
schedule: &mut SystemSchedule,
_skip_systems: Option<FixedBitSet>,
world: &mut World,
_skip_systems: Option<&FixedBitSet>,
) {
// reset counts
self.num_systems = schedule.systems.len();
Expand All @@ -189,26 +189,18 @@ impl SystemExecutor for MultiThreadedExecutor {
// If stepping is enabled, make sure we skip those systems that should
// not be run.
#[cfg(feature = "bevy_debug_stepping")]
if let Some(mut skipped_systems) = _skip_systems {
if let Some(skipped_systems) = _skip_systems {
debug_assert_eq!(skipped_systems.len(), self.completed_systems.len());
// mark skipped systems as completed
self.completed_systems |= &skipped_systems;
self.completed_systems |= skipped_systems;
self.num_completed_systems = self.completed_systems.count_ones(..);

// signal the dependencies for each of the skipped systems, as
// though they had run
for system_index in skipped_systems.ones() {
self.signal_dependents(system_index);
self.ready_systems.set(system_index, false);
}

// Finally, we need to clear all skipped systems from the ready
// list.
//
// We invert the skipped system mask to get the list of systems
// that should be run. Then we bitwise AND it with the ready list,
// resulting in a list of ready systems that aren't skipped.
skipped_systems.toggle_range(..);
self.ready_systems &= skipped_systems;
}

let thread_executor = world
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/schedule/executor/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ impl SystemExecutor for SimpleExecutor {
fn run(
&mut self,
schedule: &mut SystemSchedule,
_skip_systems: Option<FixedBitSet>,
world: &mut World,
_skip_systems: Option<&FixedBitSet>,
) {
// If stepping is enabled, make sure we skip those systems that should
// not be run.
#[cfg(feature = "bevy_debug_stepping")]
if let Some(skipped_systems) = _skip_systems {
// mark skipped systems as completed
self.completed_systems |= &skipped_systems;
self.completed_systems |= skipped_systems;
}

for system_index in 0..schedule.systems.len() {
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/schedule/executor/single_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ impl SystemExecutor for SingleThreadedExecutor {
fn run(
&mut self,
schedule: &mut SystemSchedule,
_skip_systems: Option<FixedBitSet>,
world: &mut World,
_skip_systems: Option<&FixedBitSet>,
) {
// If stepping is enabled, make sure we skip those systems that should
// not be run.
#[cfg(feature = "bevy_debug_stepping")]
if let Some(skipped_systems) = _skip_systems {
// mark skipped systems as completed
self.completed_systems |= &skipped_systems;
self.completed_systems |= skipped_systems;
}

for system_index in 0..schedule.systems.len() {
Expand Down
15 changes: 9 additions & 6 deletions crates/bevy_ecs/src/schedule/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,15 +333,18 @@ impl Schedule {
.unwrap_or_else(|e| panic!("Error when initializing schedule {:?}: {e}", self.label));

#[cfg(not(feature = "bevy_debug_stepping"))]
let skip_systems = None;
self.executor.run(&mut self.executable, world, None);

#[cfg(feature = "bevy_debug_stepping")]
let skip_systems = match world.get_resource_mut::<Stepping>() {
None => None,
Some(mut stepping) => stepping.skipped_systems(self),
};
{
let skip_systems = match world.get_resource_mut::<Stepping>() {
None => None,
Some(mut stepping) => stepping.skipped_systems(self),
};

self.executor.run(&mut self.executable, skip_systems, world);
self.executor
.run(&mut self.executable, world, skip_systems.as_ref());
}
}

/// Initializes any newly-added systems and conditions, rebuilds the executable schedule,
Expand Down

0 comments on commit d099f04

Please sign in to comment.