Skip to content

Commit ec8c8fb

Browse files
james7132james7132
committed
Remove unnecesary branches/panics from Query accesses (#6461)
# Objective Supercedes #6452. Upon inspection of the [generated assembly](https://gist.github.com/james7132/c2740c6941b80d7912f1e8888e223cbb#file-original-s) of a [simple Bevy binary](https://gist.github.com/james7132/c2740c6941b80d7912f1e8888e223cbb#file-source-rs) compiled with `cargo rustc --release -- --emit asm`, it's apparent that there are multiple unnecessary branches in the generated assembly: ```assembly .LBB5_5: cmpq %r10, %r11 je .LBB5_15 movq (%r11), %rcx movq 328(%r15), %rdx cmpq %rdx, %rcx jae .LBB5_14 movq 312(%r15), %rdi leaq (%rcx,%rcx,2), %rcx shlq $5, %rcx movq 336(%r12), %rdx movq 64(%rdi,%rcx), %rax cmpq %rdx, %rax jbe .LBB5_4 leaq (%rdi,%rcx), %rsi movq 48(%rsi), %rbp shlq $4, %rdx cmpq $0, (%rbp,%rdx) je .LBB5_4 movq 344(%r12), %rbx cmpq %rbx, %rax jbe .LBB5_4 shlq $4, %rbx cmpq $0, (%rbp,%rbx) je .LBB5_4 addq $8, %r11 movq 88(%rdi,%rcx), %rcx testq %rcx, %rcx je .LBB5_5 movq (%rsi), %rax movq 8(%rbp,%rdx), %rdx leaq (%rdx,%rdx,4), %rdi shlq $4, %rdi movq 32(%rax,%rdi), %rdx movq 56(%rax,%rdi), %r8 movq 8(%rbp,%rbx), %rbp leaq (%rbp,%rbp,4), %rbp shlq $4, %rbp movq 32(%rax,%rbp), %r9 xorl %ebp, %ebp jmp .LBB5_13 .p2align 4, 0x90 ``` Almost every one of the instructions starting with `j` is a potential branch, which can significantly slow down accesses. Of these, two labels are both common and never used: ```asm .LBB5_14: leaq __unnamed_2(%rip), %r8 callq _ZN4core9panicking18panic_bounds_check17h70367088e72af65aE ud2 .LBB5_4: callq _ZN8bevy_ecs5query25debug_checked_unreachable17h0855ff520ceaea77E ud2 .seh_endproc ``` These correpsond to subprocedure calls to panicking due to out of bounds from indexing `Tables` and `debug_checked_unreadable`. Both of which should be inlined and optimized out, but are not. ## Solution Make `debug_checked_unreachable` a macro to forcibly inline either `unreachable!()` in debug builds, and `std::hint::unreachable_unchecked()` in release mode. Replace the `Tables` and `Archetype` index access with `get(id).unwrap_or_else(|| debug_checked_unreachable!())` to assume that the table or archetype provided exists. This has no external breaking change of any kind. The equivalent section of code with these changes removes most of the conditional jump instructions: ```asm .LBB5_5: movss (%rbx,%rbp,4), %xmm0 movl %r14d, 4(%r8,%rbp,8) addss (%rdi,%rbp,4), %xmm0 movss %xmm0, (%rdi,%rbp,4) incq %rbp .LBB5_1: cmpq %rdx, %rbp jne .LBB5_5 .p2align 4, 0x90 .LBB5_2: cmpq %rcx, %rax je .LBB5_6 movq (%rax), %rdx addq $8, %rax movq 312(%rsi), %rbp leaq (%rdx,%rdx,2), %rbx shlq $5, %rbx movq 88(%rbp,%rbx), %rdx testq %rdx, %rdx je .LBB5_2 leaq (%rbx,%rbp), %r8 movq 336(%r15), %rdi movq 344(%r15), %r9 movq 48(%rbp,%rbx), %r10 shlq $4, %rdi movq (%r8), %rbx movq 8(%r10,%rdi), %rdi leaq (%rdi,%rdi,4), %rbp shlq $4, %rbp movq 32(%rbx,%rbp), %rdi movq 56(%rbx,%rbp), %r8 shlq $4, %r9 movq 8(%r10,%r9), %rbp leaq (%rbp,%rbp,4), %rbp shlq $4, %rbp movq 32(%rbx,%rbp), %rbx xorl %ebp, %ebp jmp .LBB5_5 .LBB5_6: addq $40, %rsp popq %rbx popq %rbp popq %rdi popq %rsi popq %r14 popq %r15 retq .seh_endproc ``` ## Performance Microbenchmarks results: <details> ``` group main no-panic-query ----- ---- -------------- busy_systems/01x_entities_03_systems 1.20 42.4±2.66µs ? ?/sec 1.00 35.3±1.68µs ? ?/sec busy_systems/01x_entities_06_systems 1.32 83.8±3.50µs ? ?/sec 1.00 63.6±1.72µs ? ?/sec busy_systems/01x_entities_09_systems 1.15 113.3±8.90µs ? ?/sec 1.00 98.2±6.15µs ? ?/sec busy_systems/01x_entities_12_systems 1.27 160.8±32.44µs ? ?/sec 1.00 126.6±4.70µs ? ?/sec busy_systems/01x_entities_15_systems 1.12 179.6±3.71µs ? ?/sec 1.00 160.3±11.03µs ? ?/sec busy_systems/02x_entities_03_systems 1.18 76.8±3.14µs ? ?/sec 1.00 65.2±3.17µs ? ?/sec busy_systems/02x_entities_06_systems 1.16 144.6±6.10µs ? ?/sec 1.00 124.5±5.14µs ? ?/sec busy_systems/02x_entities_09_systems 1.19 215.3±9.18µs ? ?/sec 1.00 181.5±5.67µs ? ?/sec busy_systems/02x_entities_12_systems 1.20 266.7±8.33µs ? ?/sec 1.00 222.0±9.53µs ? ?/sec busy_systems/02x_entities_15_systems 1.23 338.8±10.53µs ? ?/sec 1.00 276.3±6.94µs ? ?/sec busy_systems/03x_entities_03_systems 1.43 113.5±5.06µs ? ?/sec 1.00 79.6±1.49µs ? ?/sec busy_systems/03x_entities_06_systems 1.38 217.3±12.67µs ? ?/sec 1.00 157.5±3.07µs ? ?/sec busy_systems/03x_entities_09_systems 1.23 308.8±24.75µs ? ?/sec 1.00 251.6±8.93µs ? ?/sec busy_systems/03x_entities_12_systems 1.05 347.7±12.43µs ? ?/sec 1.00 330.6±11.43µs ? ?/sec busy_systems/03x_entities_15_systems 1.13 455.5±13.88µs ? ?/sec 1.00 401.7±17.29µs ? ?/sec busy_systems/04x_entities_03_systems 1.24 144.7±5.89µs ? ?/sec 1.00 116.9±6.29µs ? ?/sec busy_systems/04x_entities_06_systems 1.24 282.8±21.40µs ? ?/sec 1.00 228.6±21.31µs ? ?/sec busy_systems/04x_entities_09_systems 1.35 431.8±14.10µs ? ?/sec 1.00 319.6±9.83µs ? ?/sec busy_systems/04x_entities_12_systems 1.16 493.8±22.87µs ? ?/sec 1.00 424.9±15.24µs ? ?/sec busy_systems/04x_entities_15_systems 1.10 587.5±23.25µs ? ?/sec 1.00 531.7±16.32µs ? ?/sec busy_systems/05x_entities_03_systems 1.14 148.2±9.61µs ? ?/sec 1.00 129.5±4.32µs ? ?/sec busy_systems/05x_entities_06_systems 1.31 359.7±17.46µs ? ?/sec 1.00 273.6±10.55µs ? ?/sec busy_systems/05x_entities_09_systems 1.22 473.5±23.11µs ? ?/sec 1.00 389.3±13.62µs ? ?/sec busy_systems/05x_entities_12_systems 1.05 562.9±20.76µs ? ?/sec 1.00 536.5±24.35µs ? ?/sec busy_systems/05x_entities_15_systems 1.23 818.5±28.70µs ? ?/sec 1.00 666.6±45.87µs ? ?/sec contrived/01x_entities_03_systems 1.27 27.5±0.49µs ? ?/sec 1.00 21.6±1.71µs ? ?/sec contrived/01x_entities_06_systems 1.22 49.9±1.18µs ? ?/sec 1.00 40.7±2.62µs ? ?/sec contrived/01x_entities_09_systems 1.30 72.3±2.39µs ? ?/sec 1.00 55.4±2.60µs ? ?/sec contrived/01x_entities_12_systems 1.28 94.3±9.44µs ? ?/sec 1.00 73.7±3.62µs ? ?/sec contrived/01x_entities_15_systems 1.25 118.0±2.43µs ? ?/sec 1.00 94.1±3.99µs ? ?/sec contrived/02x_entities_03_systems 1.23 41.6±1.71µs ? ?/sec 1.00 33.7±2.30µs ? ?/sec contrived/02x_entities_06_systems 1.19 78.6±2.63µs ? ?/sec 1.00 65.9±2.35µs ? ?/sec contrived/02x_entities_09_systems 1.28 113.6±3.60µs ? ?/sec 1.00 88.6±3.60µs ? ?/sec contrived/02x_entities_12_systems 1.20 146.4±5.75µs ? ?/sec 1.00 121.7±3.35µs ? ?/sec contrived/02x_entities_15_systems 1.23 178.5±4.86µs ? ?/sec 1.00 145.7±4.00µs ? ?/sec contrived/03x_entities_03_systems 1.42 58.3±2.77µs ? ?/sec 1.00 41.1±1.54µs ? ?/sec contrived/03x_entities_06_systems 1.32 108.5±7.30µs ? ?/sec 1.00 82.4±4.86µs ? ?/sec contrived/03x_entities_09_systems 1.23 153.7±4.61µs ? ?/sec 1.00 125.0±4.76µs ? ?/sec contrived/03x_entities_12_systems 1.18 197.5±5.12µs ? ?/sec 1.00 166.8±8.14µs ? ?/sec contrived/03x_entities_15_systems 1.23 238.8±6.38µs ? ?/sec 1.00 194.6±4.55µs ? ?/sec contrived/04x_entities_03_systems 1.34 66.4±3.42µs ? ?/sec 1.00 49.5±1.98µs ? ?/sec contrived/04x_entities_06_systems 1.27 134.3±4.86µs ? ?/sec 1.00 105.8±3.58µs ? ?/sec contrived/04x_entities_09_systems 1.26 193.2±3.83µs ? ?/sec 1.00 153.0±5.60µs ? ?/sec contrived/04x_entities_12_systems 1.16 237.1±5.78µs ? ?/sec 1.00 204.9±18.77µs ? ?/sec contrived/04x_entities_15_systems 1.17 289.2±4.76µs ? ?/sec 1.00 246.3±8.57µs ? ?/sec contrived/05x_entities_03_systems 1.26 80.4±2.90µs ? ?/sec 1.00 63.7±3.07µs ? ?/sec contrived/05x_entities_06_systems 1.27 161.6±13.47µs ? ?/sec 1.00 127.2±5.59µs ? ?/sec contrived/05x_entities_09_systems 1.22 228.0±7.76µs ? ?/sec 1.00 186.2±7.68µs ? ?/sec contrived/05x_entities_12_systems 1.20 289.5±6.21µs ? ?/sec 1.00 241.8±7.52µs ? ?/sec contrived/05x_entities_15_systems 1.18 357.3±11.24µs ? ?/sec 1.00 302.7±7.21µs ? ?/sec heavy_compute/base 1.01 302.4±3.52µs ? ?/sec 1.00 300.2±3.40µs ? ?/sec iter_fragmented/base 1.00 348.1±7.51ns ? ?/sec 1.01 351.9±8.32ns ? ?/sec iter_fragmented/foreach 1.03 239.8±23.78ns ? ?/sec 1.00 233.8±18.12ns ? ?/sec iter_fragmented/foreach_wide 1.00 3.9±0.13µs ? ?/sec 1.02 4.0±0.22µs ? ?/sec iter_fragmented/wide 1.18 4.6±0.15µs ? ?/sec 1.00 3.9±0.10µs ? ?/sec iter_fragmented_sparse/base 1.02 8.1±0.15ns ? ?/sec 1.00 7.9±0.56ns ? ?/sec iter_fragmented_sparse/foreach 1.00 7.8±0.22ns ? ?/sec 1.01 7.9±0.62ns ? ?/sec iter_fragmented_sparse/foreach_wide 1.00 37.2±1.17ns ? ?/sec 1.10 40.9±0.95ns ? ?/sec iter_fragmented_sparse/wide 1.09 48.4±2.13ns ? ?/sec 1.00 44.5±18.34ns ? ?/sec iter_simple/base 1.02 8.4±0.10µs ? ?/sec 1.00 8.2±0.14µs ? ?/sec iter_simple/foreach 1.01 8.3±0.07µs ? ?/sec 1.00 8.2±0.09µs ? ?/sec iter_simple/foreach_sparse_set 1.00 25.3±0.32µs ? ?/sec 1.02 25.7±0.42µs ? ?/sec iter_simple/foreach_wide 1.03 41.1±0.94µs ? ?/sec 1.00 39.9±0.41µs ? ?/sec iter_simple/foreach_wide_sparse_set 1.05 123.6±2.05µs ? ?/sec 1.00 118.1±2.78µs ? ?/sec iter_simple/sparse_set 1.14 30.5±1.40µs ? ?/sec 1.00 26.9±0.64µs ? ?/sec iter_simple/system 1.01 8.4±0.25µs ? ?/sec 1.00 8.4±0.11µs ? ?/sec iter_simple/wide 1.18 48.2±0.62µs ? ?/sec 1.00 40.7±0.38µs ? ?/sec iter_simple/wide_sparse_set 1.12 140.8±21.56µs ? ?/sec 1.00 126.0±2.30µs ? ?/sec query_get/50000_entities_sparse 1.17 378.6±7.60µs ? ?/sec 1.00 324.1±23.17µs ? ?/sec query_get/50000_entities_table 1.08 330.9±10.90µs ? ?/sec 1.00 306.8±4.98µs ? ?/sec query_get_component/50000_entities_sparse 1.00 976.7±19.55µs ? ?/sec 1.00 979.8±35.87µs ? ?/sec query_get_component/50000_entities_table 1.00 1029.0±15.11µs ? ?/sec 1.05 1080.0±59.18µs ? ?/sec query_get_component_simple/system 1.13 839.7±14.18µs ? ?/sec 1.00 742.8±10.72µs ? ?/sec query_get_component_simple/unchecked 1.01 909.0±15.17µs ? ?/sec 1.00 898.0±13.56µs ? ?/sec query_get_many_10/50000_calls_sparse 1.04 5.5±0.54ms ? ?/sec 1.00 5.3±0.67ms ? ?/sec query_get_many_10/50000_calls_table 1.01 4.9±0.49ms ? ?/sec 1.00 4.8±0.45ms ? ?/sec query_get_many_2/50000_calls_sparse 1.28 848.4±210.89µs ? ?/sec 1.00 664.8±47.69µs ? ?/sec query_get_many_2/50000_calls_table 1.05 779.0±73.85µs ? ?/sec 1.00 739.2±83.02µs ? ?/sec query_get_many_5/50000_calls_sparse 1.05 2.4±0.37ms ? ?/sec 1.00 2.3±0.33ms ? ?/sec query_get_many_5/50000_calls_table 1.00 1939.9±75.22µs ? ?/sec 1.04 2.0±0.19ms ? ?/sec run_criteria/yes_using_query/001_systems 1.00 3.7±0.38µs ? ?/sec 1.30 4.9±0.14µs ? ?/sec run_criteria/yes_using_query/006_systems 1.00 8.9±0.40µs ? ?/sec 1.17 10.3±0.57µs ? ?/sec run_criteria/yes_using_query/011_systems 1.00 13.9±0.49µs ? ?/sec 1.08 15.0±0.89µs ? ?/sec run_criteria/yes_using_query/016_systems 1.00 18.8±0.74µs ? ?/sec 1.00 18.8±1.43µs ? ?/sec run_criteria/yes_using_query/021_systems 1.07 24.1±0.87µs ? ?/sec 1.00 22.6±1.58µs ? ?/sec run_criteria/yes_using_query/026_systems 1.04 27.9±0.62µs ? ?/sec 1.00 26.8±1.71µs ? ?/sec run_criteria/yes_using_query/031_systems 1.09 33.3±1.03µs ? ?/sec 1.00 30.5±2.18µs ? ?/sec run_criteria/yes_using_query/036_systems 1.14 38.7±0.80µs ? ?/sec 1.00 33.9±1.75µs ? ?/sec run_criteria/yes_using_query/041_systems 1.18 43.7±1.07µs ? ?/sec 1.00 37.0±2.39µs ? ?/sec run_criteria/yes_using_query/046_systems 1.14 47.6±1.16µs ? ?/sec 1.00 41.9±2.09µs ? ?/sec run_criteria/yes_using_query/051_systems 1.17 52.9±2.04µs ? ?/sec 1.00 45.3±1.75µs ? ?/sec run_criteria/yes_using_query/056_systems 1.25 59.2±2.38µs ? ?/sec 1.00 47.2±2.01µs ? ?/sec run_criteria/yes_using_query/061_systems 1.28 66.1±15.84µs ? ?/sec 1.00 51.5±2.47µs ? ?/sec run_criteria/yes_using_query/066_systems 1.28 70.2±2.57µs ? ?/sec 1.00 54.7±2.58µs ? ?/sec run_criteria/yes_using_query/071_systems 1.30 75.5±2.27µs ? ?/sec 1.00 58.2±3.31µs ? ?/sec run_criteria/yes_using_query/076_systems 1.26 81.5±2.66µs ? ?/sec 1.00 64.5±3.13µs ? ?/sec run_criteria/yes_using_query/081_systems 1.29 89.7±2.58µs ? ?/sec 1.00 69.3±3.47µs ? ?/sec run_criteria/yes_using_query/086_systems 1.33 95.6±3.39µs ? ?/sec 1.00 71.8±3.48µs ? ?/sec run_criteria/yes_using_query/091_systems 1.25 102.0±3.67µs ? ?/sec 1.00 81.4±4.82µs ? ?/sec run_criteria/yes_using_query/096_systems 1.33 111.7±3.29µs ? ?/sec 1.00 83.8±4.15µs ? ?/sec run_criteria/yes_using_query/101_systems 1.29 113.2±12.04µs ? ?/sec 1.00 87.7±5.15µs ? ?/sec world_query_for_each/50000_entities_sparse 1.00 47.4±0.51µs ? ?/sec 1.00 47.3±0.33µs ? ?/sec world_query_for_each/50000_entities_table 1.00 27.2±0.50µs ? ?/sec 1.00 27.2±0.17µs ? ?/sec world_query_get/50000_entities_sparse_wide 1.09 210.5±1.78µs ? ?/sec 1.00 192.5±2.61µs ? ?/sec world_query_get/50000_entities_table 1.00 127.7±2.09µs ? ?/sec 1.07 136.2±5.95µs ? ?/sec world_query_get/50000_entities_table_wide 1.00 209.8±2.37µs ? ?/sec 1.15 240.6±2.04µs ? ?/sec world_query_iter/50000_entities_sparse 1.00 54.2±0.36µs ? ?/sec 1.01 54.7±0.61µs ? ?/sec world_query_iter/50000_entities_table 1.00 27.2±0.31µs ? ?/sec 1.00 27.3±0.64µs ? ?/sec ``` </details> NOTE: This PR includes a change to enable LTO on our benchmarks to get a "fully optimized" baseline for our benchmarks. Both the main and the current PR's results were with LTO enabled.
1 parent 2c5d072 commit ec8c8fb

File tree

8 files changed

+102
-50
lines changed

8 files changed

+102
-50
lines changed

‎Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1610,6 +1610,9 @@ target_sdk_version = 31
16101610
icon = "@mipmap/ic_launcher"
16111611
label = "Bevy Example"
16121612

1613+
[profile.release]
1614+
lto = true
1615+
16131616
[profile.wasm-release]
16141617
inherits = "release"
16151618
opt-level = "z"

‎benches/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ bevy_reflect = { path = "../crates/bevy_reflect" }
1717
bevy_tasks = { path = "../crates/bevy_tasks" }
1818
bevy_utils = { path = "../crates/bevy_utils" }
1919

20+
[profile.release]
21+
opt-level = 3
22+
lto = true
23+
2024
[[bench]]
2125
name = "ecs"
2226
path = "benches/bevy_ecs/benches.rs"

‎crates/bevy_ecs/src/query/fetch.rs

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::{
33
change_detection::Ticks,
44
component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType},
55
entity::Entity,
6-
query::{debug_checked_unreachable, Access, FilteredAccess},
6+
query::{Access, DebugCheckedUnwrap, FilteredAccess},
77
storage::{ComponentSparseSet, Table},
88
world::{Mut, World},
99
};
@@ -552,7 +552,7 @@ unsafe impl<T: Component> WorldQuery for &T {
552552
.storages()
553553
.sparse_sets
554554
.get(component_id)
555-
.unwrap_or_else(|| debug_checked_unreachable())
555+
.debug_checked_unwrap()
556556
}),
557557
}
558558
}
@@ -585,7 +585,7 @@ unsafe impl<T: Component> WorldQuery for &T {
585585
fetch.table_components = Some(
586586
table
587587
.get_column(component_id)
588-
.unwrap_or_else(|| debug_checked_unreachable())
588+
.debug_checked_unwrap()
589589
.get_data_slice()
590590
.into(),
591591
);
@@ -600,14 +600,14 @@ unsafe impl<T: Component> WorldQuery for &T {
600600
match T::Storage::STORAGE_TYPE {
601601
StorageType::Table => fetch
602602
.table_components
603-
.unwrap_or_else(|| debug_checked_unreachable())
603+
.debug_checked_unwrap()
604604
.get(table_row)
605605
.deref(),
606606
StorageType::SparseSet => fetch
607607
.sparse_set
608-
.unwrap_or_else(|| debug_checked_unreachable())
608+
.debug_checked_unwrap()
609609
.get(entity)
610-
.unwrap_or_else(|| debug_checked_unreachable())
610+
.debug_checked_unwrap()
611611
.deref(),
612612
}
613613
}
@@ -696,7 +696,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
696696
.storages()
697697
.sparse_sets
698698
.get(component_id)
699-
.unwrap_or_else(|| debug_checked_unreachable())
699+
.debug_checked_unwrap()
700700
}),
701701
last_change_tick,
702702
change_tick,
@@ -730,9 +730,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
730730
&component_id: &ComponentId,
731731
table: &'w Table,
732732
) {
733-
let column = table
734-
.get_column(component_id)
735-
.unwrap_or_else(|| debug_checked_unreachable());
733+
let column = table.get_column(component_id).debug_checked_unwrap();
736734
fetch.table_data = Some((
737735
column.get_data_slice().into(),
738736
column.get_ticks_slice().into(),
@@ -747,9 +745,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
747745
) -> Self::Item<'w> {
748746
match T::Storage::STORAGE_TYPE {
749747
StorageType::Table => {
750-
let (table_components, table_ticks) = fetch
751-
.table_data
752-
.unwrap_or_else(|| debug_checked_unreachable());
748+
let (table_components, table_ticks) = fetch.table_data.debug_checked_unwrap();
753749
Mut {
754750
value: table_components.get(table_row).deref_mut(),
755751
ticks: Ticks {
@@ -762,9 +758,9 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
762758
StorageType::SparseSet => {
763759
let (component, component_ticks) = fetch
764760
.sparse_set
765-
.unwrap_or_else(|| debug_checked_unreachable())
761+
.debug_checked_unwrap()
766762
.get_with_ticks(entity)
767-
.unwrap_or_else(|| debug_checked_unreachable());
763+
.debug_checked_unwrap();
768764
Mut {
769765
value: component.assert_unique().deref_mut(),
770766
ticks: Ticks {
@@ -1038,7 +1034,7 @@ unsafe impl<T: Component> WorldQuery for ChangeTrackers<T> {
10381034
.storages()
10391035
.sparse_sets
10401036
.get(component_id)
1041-
.unwrap_or_else(|| debug_checked_unreachable())
1037+
.debug_checked_unwrap()
10421038
}),
10431039
marker: PhantomData,
10441040
last_change_tick,
@@ -1077,7 +1073,7 @@ unsafe impl<T: Component> WorldQuery for ChangeTrackers<T> {
10771073
fetch.table_ticks = Some(
10781074
table
10791075
.get_column(id)
1080-
.unwrap_or_else(|| debug_checked_unreachable())
1076+
.debug_checked_unwrap()
10811077
.get_ticks_slice()
10821078
.into(),
10831079
);
@@ -1092,9 +1088,7 @@ unsafe impl<T: Component> WorldQuery for ChangeTrackers<T> {
10921088
match T::Storage::STORAGE_TYPE {
10931089
StorageType::Table => ChangeTrackers {
10941090
component_ticks: {
1095-
let table_ticks = fetch
1096-
.table_ticks
1097-
.unwrap_or_else(|| debug_checked_unreachable());
1091+
let table_ticks = fetch.table_ticks.debug_checked_unwrap();
10981092
table_ticks.get(table_row).read()
10991093
},
11001094
marker: PhantomData,
@@ -1104,9 +1098,9 @@ unsafe impl<T: Component> WorldQuery for ChangeTrackers<T> {
11041098
StorageType::SparseSet => ChangeTrackers {
11051099
component_ticks: *fetch
11061100
.sparse_set
1107-
.unwrap_or_else(|| debug_checked_unreachable())
1101+
.debug_checked_unwrap()
11081102
.get_ticks(entity)
1109-
.unwrap_or_else(|| debug_checked_unreachable())
1103+
.debug_checked_unwrap()
11101104
.get(),
11111105
marker: PhantomData,
11121106
last_change_tick: fetch.last_change_tick,

‎crates/bevy_ecs/src/query/filter.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use crate::{
22
archetype::{Archetype, ArchetypeComponentId},
33
component::{Component, ComponentId, ComponentStorage, ComponentTicks, StorageType},
44
entity::Entity,
5-
query::{debug_checked_unreachable, Access, FilteredAccess, WorldQuery},
5+
query::{Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery},
66
storage::{ComponentSparseSet, Table},
77
world::World,
88
};
@@ -439,7 +439,7 @@ macro_rules! impl_tick_filter {
439439
world.storages()
440440
.sparse_sets
441441
.get(id)
442-
.unwrap_or_else(|| debug_checked_unreachable())
442+
.debug_checked_unwrap()
443443
}),
444444
marker: PhantomData,
445445
last_change_tick,
@@ -476,7 +476,7 @@ macro_rules! impl_tick_filter {
476476
) {
477477
fetch.table_ticks = Some(
478478
table.get_column(component_id)
479-
.unwrap_or_else(|| debug_checked_unreachable())
479+
.debug_checked_unwrap()
480480
.get_ticks_slice()
481481
.into()
482482
);
@@ -504,7 +504,7 @@ macro_rules! impl_tick_filter {
504504
StorageType::Table => {
505505
$is_detected(&*(
506506
fetch.table_ticks
507-
.unwrap_or_else(|| debug_checked_unreachable())
507+
.debug_checked_unwrap()
508508
.get(table_row))
509509
.deref(),
510510
fetch.last_change_tick,
@@ -514,9 +514,9 @@ macro_rules! impl_tick_filter {
514514
StorageType::SparseSet => {
515515
let ticks = &*fetch
516516
.sparse_set
517-
.unwrap_or_else(|| debug_checked_unreachable())
517+
.debug_checked_unwrap()
518518
.get_ticks(entity)
519-
.unwrap_or_else(|| debug_checked_unreachable())
519+
.debug_checked_unwrap()
520520
.get();
521521
$is_detected(ticks, fetch.last_change_tick, fetch.change_tick)
522522
}

‎crates/bevy_ecs/src/query/iter.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use crate::{
22
archetype::{ArchetypeEntity, ArchetypeId, Archetypes},
33
entity::{Entities, Entity},
44
prelude::World,
5-
query::{ArchetypeFilter, QueryState, WorldQuery},
5+
query::{ArchetypeFilter, DebugCheckedUnwrap, QueryState, WorldQuery},
66
storage::{TableId, Tables},
77
};
88
use std::{borrow::Borrow, iter::FusedIterator, marker::PhantomData, mem::MaybeUninit};
@@ -153,8 +153,11 @@ where
153153
continue;
154154
}
155155

156-
let archetype = &self.archetypes[location.archetype_id];
157-
let table = &self.tables[archetype.table_id()];
156+
let archetype = self
157+
.archetypes
158+
.get(location.archetype_id)
159+
.debug_checked_unwrap();
160+
let table = self.tables.get(archetype.table_id()).debug_checked_unwrap();
158161

159162
// SAFETY: `archetype` is from the world that `fetch/filter` were created for,
160163
// `fetch_state`/`filter_state` are the states that `fetch/filter` were initialized with
@@ -586,7 +589,7 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's,
586589
// we are on the beginning of the query, or finished processing a table, so skip to the next
587590
if self.current_index == self.current_len {
588591
let table_id = self.table_id_iter.next()?;
589-
let table = &tables[*table_id];
592+
let table = tables.get(*table_id).debug_checked_unwrap();
590593
// SAFETY: `table` is from the world that `fetch/filter` were created for,
591594
// `fetch_state`/`filter_state` are the states that `fetch/filter` were initialized with
592595
Q::set_table(&mut self.fetch, &query_state.fetch_state, table);
@@ -616,10 +619,10 @@ impl<'w, 's, Q: WorldQuery, F: ReadOnlyWorldQuery> QueryIterationCursor<'w, 's,
616619
loop {
617620
if self.current_index == self.current_len {
618621
let archetype_id = self.archetype_id_iter.next()?;
619-
let archetype = &archetypes[*archetype_id];
622+
let archetype = archetypes.get(*archetype_id).debug_checked_unwrap();
620623
// SAFETY: `archetype` and `tables` are from the world that `fetch/filter` were created for,
621624
// `fetch_state`/`filter_state` are the states that `fetch/filter` were initialized with
622-
let table = &tables[archetype.table_id()];
625+
let table = tables.get(archetype.table_id()).debug_checked_unwrap();
623626
Q::set_archetype(&mut self.fetch, &query_state.fetch_state, archetype, table);
624627
F::set_archetype(
625628
&mut self.filter,

‎crates/bevy_ecs/src/query/mod.rs

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,49 @@ pub use filter::*;
1010
pub use iter::*;
1111
pub use state::*;
1212

13-
#[allow(unreachable_code)]
14-
pub(crate) unsafe fn debug_checked_unreachable() -> ! {
15-
#[cfg(debug_assertions)]
16-
unreachable!();
17-
std::hint::unreachable_unchecked();
13+
/// A debug checked version of [`Option::unwrap_unchecked`]. Will panic in
14+
/// debug modes if unwrapping a `None` or `Err` value in debug mode, but is
15+
/// equivalent to `Option::unwrap_uncheched` or `Result::unwrap_unchecked`
16+
/// in release mode.
17+
pub(crate) trait DebugCheckedUnwrap {
18+
type Item;
19+
/// # Panics
20+
/// Panics if the value is `None` or `Err`, only in debug mode.
21+
///
22+
/// # Safety
23+
/// This must never be called on a `None` or `Err` value. This can
24+
/// only be called on `Some` or `Ok` values.
25+
unsafe fn debug_checked_unwrap(self) -> Self::Item;
26+
}
27+
28+
// Thes two impls are explicitly split to ensure that the unreachable! macro
29+
// does not cause inlining to fail when compiling in release mode.
30+
#[cfg(debug_assertions)]
31+
impl<T> DebugCheckedUnwrap for Option<T> {
32+
type Item = T;
33+
34+
#[inline(always)]
35+
unsafe fn debug_checked_unwrap(self) -> Self::Item {
36+
if let Some(inner) = self {
37+
inner
38+
} else {
39+
unreachable!()
40+
}
41+
}
42+
}
43+
44+
#[cfg(not(debug_assertions))]
45+
impl<T> DebugCheckedUnwrap for Option<T> {
46+
type Item = T;
47+
48+
#[inline(always)]
49+
unsafe fn debug_checked_unwrap(self) -> Self::Item {
50+
if let Some(inner) = self {
51+
inner
52+
} else {
53+
std::hint::unreachable_unchecked()
54+
}
55+
}
1856
}
1957

2058
#[cfg(test)]

‎crates/bevy_ecs/src/query/state.rs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ use crate::{
33
component::ComponentId,
44
entity::Entity,
55
prelude::FromWorld,
6-
query::{Access, FilteredAccess, QueryCombinationIter, QueryIter, WorldQuery},
6+
query::{
7+
Access, DebugCheckedUnwrap, FilteredAccess, QueryCombinationIter, QueryIter, WorldQuery,
8+
},
79
storage::TableId,
810
world::{World, WorldId},
911
};
@@ -409,11 +411,18 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
409411
{
410412
return Err(QueryEntityError::QueryDoesNotMatch(entity));
411413
}
412-
let archetype = &world.archetypes[location.archetype_id];
414+
let archetype = world
415+
.archetypes
416+
.get(location.archetype_id)
417+
.debug_checked_unwrap();
413418
let mut fetch = Q::init_fetch(world, &self.fetch_state, last_change_tick, change_tick);
414419
let mut filter = F::init_fetch(world, &self.filter_state, last_change_tick, change_tick);
415420

416-
let table = &world.storages().tables[archetype.table_id()];
421+
let table = world
422+
.storages()
423+
.tables
424+
.get(archetype.table_id())
425+
.debug_checked_unwrap();
417426
Q::set_archetype(&mut fetch, &self.fetch_state, archetype, table);
418427
F::set_archetype(&mut filter, &self.filter_state, archetype, table);
419428

@@ -930,7 +939,7 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
930939
let tables = &world.storages().tables;
931940
if Q::IS_DENSE && F::IS_DENSE {
932941
for table_id in &self.matched_table_ids {
933-
let table = &tables[*table_id];
942+
let table = tables.get(*table_id).debug_checked_unwrap();
934943
Q::set_table(&mut fetch, &self.fetch_state, table);
935944
F::set_table(&mut filter, &self.filter_state, table);
936945

@@ -946,8 +955,8 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
946955
} else {
947956
let archetypes = &world.archetypes;
948957
for archetype_id in &self.matched_archetype_ids {
949-
let archetype = &archetypes[*archetype_id];
950-
let table = &tables[archetype.table_id()];
958+
let archetype = archetypes.get(*archetype_id).debug_checked_unwrap();
959+
let table = tables.get(archetype.table_id()).debug_checked_unwrap();
951960
Q::set_archetype(&mut fetch, &self.fetch_state, archetype, table);
952961
F::set_archetype(&mut filter, &self.filter_state, archetype, table);
953962

@@ -1025,7 +1034,7 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
10251034
change_tick,
10261035
);
10271036
let tables = &world.storages().tables;
1028-
let table = &tables[*table_id];
1037+
let table = tables.get(*table_id).debug_checked_unwrap();
10291038
let entities = table.entities();
10301039
Q::set_table(&mut fetch, &self.fetch_state, table);
10311040
F::set_table(&mut filter, &self.filter_state, table);
@@ -1076,8 +1085,9 @@ impl<Q: WorldQuery, F: ReadOnlyWorldQuery> QueryState<Q, F> {
10761085
change_tick,
10771086
);
10781087
let tables = &world.storages().tables;
1079-
let archetype = &world.archetypes[*archetype_id];
1080-
let table = &tables[archetype.table_id()];
1088+
let archetype =
1089+
world.archetypes.get(*archetype_id).debug_checked_unwrap();
1090+
let table = tables.get(archetype.table_id()).debug_checked_unwrap();
10811091
Q::set_archetype(&mut fetch, &self.fetch_state, archetype, table);
10821092
F::set_archetype(&mut filter, &self.filter_state, archetype, table);
10831093

‎crates/bevy_ecs/src/storage/table.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::{
22
component::{ComponentId, ComponentInfo, ComponentTicks, Components},
33
entity::Entity,
4-
query::debug_checked_unreachable,
4+
query::DebugCheckedUnwrap,
55
storage::{blob_vec::BlobVec, SparseSet},
66
};
77
use bevy_ptr::{OwningPtr, Ptr, PtrMut};
@@ -386,7 +386,7 @@ impl Table {
386386
for (component_id, column) in self.columns.iter_mut() {
387387
new_table
388388
.get_column_mut(*component_id)
389-
.unwrap_or_else(|| debug_checked_unreachable())
389+
.debug_checked_unwrap()
390390
.initialize_from_unchecked(column, row, new_row);
391391
}
392392
TableMoveResult {

0 commit comments

Comments
 (0)