Skip to content

Allow fallback error handlers to queue commands - #25288

Open
0xEgao wants to merge 1 commit into
bevyengine:mainfrom
0xEgao:fallback-error
Open

Allow fallback error handlers to queue commands#25288
0xEgao wants to merge 1 commit into
bevyengine:mainfrom
0xEgao:fallback-error

Conversation

@0xEgao

@0xEgao 0xEgao commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Objective

Fixes #24821.

Solution

  • Pass Commands to ErrorHandler and return it from the handler.
  • Apply commands queued by error handlers across executors, observers, combinators, and command error paths.
  • Preserve existing scheduling behavior when no error occurs.
  • Add a set_error_handler doc example demonstrating how to send AppExit.

@alice-i-cecile

Copy link
Copy Markdown
Member

I really want a feature like this, but this might have surprising performance implications. Can you please benchmark this?

@0xEgao

0xEgao commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

I really want a feature like this, but this might have surprising performance implications. Can you please benchmark this?

Yeah sure :)

@0xEgao

0xEgao commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

I really want a feature like this, but this might have surprising performance implications. Can you please benchmark this?

Hey sorry for taking time, I benchmarked this against origin/main using Bevy's existing Criterion benchmarks.

Environment:

  • Apple M4 MacBook Air (10 cores, 16 GB RAM)
  • macOS / Darwin 24.6.0, arm64
  • rustc 1.97.0 (2d8144b78 2026-07-07)
  • Baseline: origin/main at c7a16115c
  • Branch: 9ea9e652e
  • Both revisions used the same Cargo.lock and isolated Cargo target directories.

Results:

Benchmark Mean change (95% CI) p-value Criterion result
run_empty_schedule/SingleThreaded -1.90% [-3.09%, -0.76%] 0.00 Within noise threshold
run_empty_schedule/MultiThreaded +0.06% [-0.65%, +0.80%] 0.88 No change detected
empty_systems/0_systems +1.49% [+0.97%, +1.99%] 0.00 Within noise threshold
empty_systems/2_systems -2.77% [-11.01%, +2.25%] 0.69 No change detected
empty_systems/4_systems +1.05% [+0.21%, +1.88%] 0.02 Within noise threshold
empty_systems/10_systems -0.37% [-2.56%, +2.21%] 0.78 No change detected
empty_systems/100_systems -0.16% [-0.90%, +0.63%] 0.69 No change detected
empty_systems/1000_systems +0.45% [-2.62%, +3.23%] 0.78 No change detected
param/combinator_system/8_piped_systems +0.86% [+0.33%, +1.44%] 0.00 Within noise threshold

The first sequential sweep reported regressions for the 10- and 1,000-system cases. I immediately repeated those cases with main and the branch run back-to-back. The regressions did not reproduce: Criterion reported no change (p = 0.78) for both. The table uses those controlled repeat results.

No benchmark was classified as a performance regression by Criterion.

Commands used:

cargo bench --locked -p benches --bench ecs -- run_empty_schedule --save-baseline main
cargo bench --locked -p benches --bench ecs -- run_empty_schedule --baseline main

cargo bench --locked -p benches --bench ecs -- empty_systems --save-baseline main
cargo bench --locked -p benches --bench ecs -- empty_systems --baseline main

cargo bench --locked -p benches --bench ecs -- param/combinator_system/8_piped_systems --save-baseline main
cargo bench --locked -p benches --bench ecs -- param/combinator_system/8_piped_systems --baseline main

@JaySpruce JaySpruce added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 6, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in ECS Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

Allow FallbackErrorHandler to send AppExit

3 participants