Consider switching to aborting on panic in hot ECS functions
- Dominant language
- Rust
- Stars
- 48.2k
- Forks
- 4.8k
- Avg merge
- 3d 22h
- Merged PRs (30d)
- 161
Description
## What problem does this solve or what need does it fill?
Some potentially very hot and heavily monomorphized ECS functions still have panics in the case something goes wrong. Panicking results in a lot of codegen, which can make binary sizes much larger, and can affect instruction fetch locality and thus performance in very hot ECS functions.
As seen via the [asembly generation tests](https://github.com/search?q=repo%3Ajames7132%2Fbevy_asm_tests%20panic&type=code), there are still many locations where panics may crop up.
## What solution would you like?
Create a utility called [`AbortOnPanic`](https://github.com/tokio-rs/tokio/blob/a2096049ee2b8e30b420ba7ecff6b81e609428b3/tokio/src/runtime/scheduler/multi_thread/worker.rs#L453) which turns all panics in the local scope into aborts, only in release mode.
This technique is used by a number of fairly low level crates like `tokio` or `async-task`, where a panic would otherwise be an artifact of incorrect safety or correctness-related invariants not being satisfied. Any location where it's calling into user provided code (i.e. trait functions turned into function pointers) must be wrapped in `std::panic::catch_unwind` to avoid aborting on user-provided code.
This may also be a correctness and soundness issue, since panicking on unsatisfied invariant can be caught via `catch_unwind`, and leave the ECS in an invalid and unsound state.
Note that this likely won't affect builds using `panic = "abort"` in their build profiles, but will likely improve the default `--release ` configuration that cargo provides.
This should only be used in locations where we know that we're not calling into user provided code, and we should only be panicking
## What alternative(s) have you considered?
Leaving it as is, panicking instead of aborting.
The other alternative is to use `std::hint::unreachable_unchecked`, which is both unsafe and very easy to result in undefined behavior is used incorrectly.
Contributor guide
Assessment
This issue has not been assessed yet.