Move implementations of `Query` methods from `QueryState` to `Query`. #17822

chescock · 2025-02-12T15:27:06Z

Objective

Simplify the API surface by removing duplicated functionality between Query and QueryState.

Reduce the amount of unsafe code required in QueryState.

This is a follow-up to #15858.

Solution

Move implementations of Query methods from QueryState to Query. Instead of the original methods being on QueryState, with Query methods calling them by passing the individual parameters, the original methods are now on Query, with QueryState methods calling them by constructing a Query.

This also adds two _inner methods that were missed in #15858: iter_many_unique_inner and single_inner.

One goal here is to be able to deprecate and eventually remove many of the methods on QueryState, reducing the overall API surface. (I expected to do that in this PR, but this change was large enough on its own!) Now that the QueryState methods each consist of a simple expression like self.query(world).get_inner(entity), a future PR can deprecate some or all of them with simple migration instructions.

The other goal is to reduce the amount of unsafe code. The current implementation of a read-only method like QueryState::get directly calls the unsafe fn get_unchecked_manual and needs to repeat the proof that &World has enough access. With this change, QueryState::get is entirely safe code, with the proof that &World has enough access done by the query() method and shared across all read-only operations.

Future Work

The next step will be to mark the QueryState methods as #[deprecated] and migrate callers to the methods on Query.

Add missing `iter_many_unique_inner` and `single_inner` methods.

crates/bevy_ecs/src/query/state.rs

cBournhonesque

Very satisfying PR, it's nice to see it all come together

… `QueryState` to the file with `Query`.

hymm · 2025-02-13T00:54:33Z

We should run the query benches to make sure nothing weird is happening.

Carter0

I like it!

chescock · 2025-02-13T03:37:34Z

We should run the query benches to make sure nothing weird is happening.

That makes sense! I'll try to figure out how to do that, but I probably won't have time until Friday.

# Conflicts: # crates/bevy_ecs/src/system/query.rs

…_unchecked_manual` for parallel iteration.

…s()` and make it a safety requirement.

chescock · 2025-02-15T20:40:19Z

I ran the benches, and found some slowdowns caused by extra calls to validate_world from query_unchecked_manual_with_ticks. In particular, parallel iteration called iter_unchecked_manual, which did not need to validate_world before this change.

I changed the safety requirements of the query_unchecked_manual methods to require a valid world instead of validating it, and now the benchmarks look a little better. They're pretty noisy on my machine, though, so I can't quite tell whether there are any other issues.

chescock · 2025-02-16T17:48:20Z

Here are the results of running cargo bench -p benches --bench ecs -- iter on my machine for commit 14007f9, with commit 0f1c757 as baseline. I think the changes are just noise, especially since the biggest ones were in "events_iter", which don't involve Query at all.

empty_archetypes/iter/10
                        time:   [2.6944 ┬╡s 2.7537 ┬╡s 2.8218 ┬╡s]
                        change: [-16.115% -12.272% -8.8267%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  3 (3.00%) high mild
  1 (1.00%) high severe
empty_archetypes/iter/100
                        time:   [2.8837 ┬╡s 2.9505 ┬╡s 3.0259 ┬╡s]
                        change: [-2.8784% +3.8987% +11.558%] (p = 0.31 > 0.05)
                        No change in performance detected.
Found 10 outliers among 100 measurements (10.00%)
  4 (4.00%) high mild
  6 (6.00%) high severe
empty_archetypes/iter/500
                        time:   [2.9537 ┬╡s 3.0101 ┬╡s 3.0901 ┬╡s]
                        change: [-24.025% -13.469% -2.6613%] (p = 0.03 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  7 (7.00%) high mild
  3 (3.00%) high severe
empty_archetypes/iter/1000
                        time:   [3.4352 ┬╡s 3.5428 ┬╡s 3.6698 ┬╡s]
                        change: [-17.310% -13.424% -9.2730%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 15 outliers among 100 measurements (15.00%)
  9 (9.00%) high mild
  6 (6.00%) high severe
empty_archetypes/iter/2000
                        time:   [15.636 ┬╡s 15.840 ┬╡s 16.040 ┬╡s]
                        change: [-17.194% -15.659% -14.324%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
  3 (3.00%) low severe
  5 (5.00%) low mild
  1 (1.00%) high mild
  2 (2.00%) high severe
empty_archetypes/iter/5000
                        time:   [18.760 ┬╡s 19.004 ┬╡s 19.318 ┬╡s]
                        change: [-13.959% -12.026% -9.9948%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe
empty_archetypes/iter/10000
                        time:   [25.292 ┬╡s 25.506 ┬╡s 25.739 ┬╡s]
                        change: [-13.832% -12.473% -11.157%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) low mild
  2 (2.00%) high mild
  1 (1.00%) high severe

events_iter/size_4_events_100
                        time:   [103.97 ns 106.04 ns 108.90 ns]
                        change: [-7.6737% -5.1041% -2.4028%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  4 (4.00%) high mild
  5 (5.00%) high severe
events_iter/size_4_events_1000
                        time:   [1.0088 ┬╡s 1.0301 ┬╡s 1.0530 ┬╡s]
                        change: [-0.9734% +1.4987% +4.0128%] (p = 0.24 > 0.05)
                        No change in performance detected.
Found 5 outliers among 100 measurements (5.00%)
  5 (5.00%) high mild
events_iter/size_4_events_10000
                        time:   [11.388 ┬╡s 11.528 ┬╡s 11.673 ┬╡s]
                        change: [+17.348% +20.553% +23.923%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
events_iter/size_4_events_50000
                        time:   [55.771 ┬╡s 56.443 ┬╡s 57.127 ┬╡s]
                        change: [+12.461% +15.141% +17.827%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild
  1 (1.00%) high severe
events_iter/size_16_events_100
                        time:   [111.38 ns 112.23 ns 113.13 ns]
                        change: [-0.2764% +4.2106% +9.8724%] (p = 0.11 > 0.05)
                        No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
  2 (2.00%) low mild
  3 (3.00%) high mild
  2 (2.00%) high severe
events_iter/size_16_events_1000
                        time:   [1.1076 ┬╡s 1.1316 ┬╡s 1.1566 ┬╡s]
                        change: [+10.825% +13.775% +16.571%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild
events_iter/size_16_events_10000
                        time:   [11.309 ┬╡s 11.520 ┬╡s 11.710 ┬╡s]
                        change: [+13.326% +16.171% +19.064%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) low mild
events_iter/size_16_events_50000
                        time:   [56.300 ┬╡s 56.962 ┬╡s 57.601 ┬╡s]
                        change: [+11.748% +14.588% +17.358%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 8 outliers among 100 measurements (8.00%)
  4 (4.00%) low mild
  2 (2.00%) high mild
  2 (2.00%) high severe
events_iter/size_512_events_100
                        time:   [106.19 ns 107.28 ns 108.32 ns]
                        change: [-7.0186% -4.8028% -2.6515%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe
events_iter/size_512_events_1000
                        time:   [994.50 ns 1.0076 ┬╡s 1.0217 ┬╡s]
                        change: [-2.4829% -0.2611% +2.0389%] (p = 0.82 > 0.05)
                        No change in performance detected.
Found 5 outliers among 100 measurements (5.00%)
  1 (1.00%) high mild
  4 (4.00%) high severe
events_iter/size_512_events_10000
                        time:   [10.331 ┬╡s 10.546 ┬╡s 10.848 ┬╡s]
                        change: [+3.6853% +7.3787% +11.547%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 8 outliers among 100 measurements (8.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild
  5 (5.00%) high severe
events_iter/size_512_events_50000
                        time:   [46.527 ┬╡s 46.981 ┬╡s 47.625 ┬╡s]
                        change: [-4.9797% -2.3378% +0.0769%] (p = 0.08 > 0.05)
                        No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

iter_fragmented/base    time:   [694.52 ns 704.73 ns 719.24 ns]
                        change: [-5.2310% -3.1407% -1.1244%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe
iter_fragmented/wide    time:   [9.8758 ┬╡s 9.9879 ┬╡s 10.097 ┬╡s]
                        change: [-8.0516% -5.5802% -3.3688%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe
iter_fragmented/foreach time:   [254.96 ns 259.03 ns 263.43 ns]
                        change: [-9.4897% -7.1459% -4.6658%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  4 (4.00%) high mild
  6 (6.00%) high severe
iter_fragmented/foreach_wide
                        time:   [7.1064 ┬╡s 7.1438 ┬╡s 7.1862 ┬╡s]
                        change: [-10.806% -6.2049% +0.7355%] (p = 0.03 < 0.05)
                        Change within noise threshold.
Found 8 outliers among 100 measurements (8.00%)
  2 (2.00%) high mild
  6 (6.00%) high severe

iter_fragmented_sparse/base
                        time:   [12.646 ns 12.876 ns 13.213 ns]
                        change: [-12.079% -9.9790% -7.8835%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe
iter_fragmented_sparse/wide
                        time:   [98.926 ns 99.890 ns 100.83 ns]
                        change: [-8.0508% -6.0187% -4.0919%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe
iter_fragmented_sparse/foreach
                        time:   [14.694 ns 14.851 ns 15.001 ns]
                        change: [-10.230% -7.9759% -5.7839%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe
iter_fragmented_sparse/foreach_wide
                        time:   [72.154 ns 72.987 ns 73.823 ns]
                        change: [-10.014% -8.1119% -6.2246%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

iter_simple/base        time:   [14.568 ┬╡s 14.671 ┬╡s 14.780 ┬╡s]
                        change: [-8.6101% -6.6358% -4.6108%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  2 (2.00%) high mild
  4 (4.00%) high severe
iter_simple/wide        time:   [75.024 ┬╡s 76.149 ┬╡s 77.702 ┬╡s]
                        change: [-0.5804% +1.7097% +3.8907%] (p = 0.14 > 0.05)
                        No change in performance detected.
Found 10 outliers among 100 measurements (10.00%)
  3 (3.00%) low mild
  5 (5.00%) high mild
  2 (2.00%) high severe
iter_simple/system      time:   [14.883 ┬╡s 15.004 ┬╡s 15.136 ┬╡s]
                        change: [-8.9091% -6.8658% -4.8763%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  5 (5.00%) high mild
  1 (1.00%) high severe
iter_simple/sparse_set  time:   [33.194 ┬╡s 33.386 ┬╡s 33.621 ┬╡s]
                        change: [-5.6541% -3.5465% -1.4908%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  3 (3.00%) high mild
  5 (5.00%) high severe
iter_simple/wide_sparse_set
                        time:   [154.98 ┬╡s 155.33 ┬╡s 155.70 ┬╡s]
                        change: [-9.3508% -7.4441% -5.5506%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  3 (3.00%) high mild
  6 (6.00%) high severe
iter_simple/foreach     time:   [14.208 ┬╡s 14.495 ┬╡s 15.107 ┬╡s]
                        change: [-9.9295% -3.9401% +5.9480%] (p = 0.51 > 0.05)
                        No change in performance detected.
Found 12 outliers among 100 measurements (12.00%)
  5 (5.00%) high mild
  7 (7.00%) high severe
iter_simple/foreach_wide
                        time:   [76.377 ┬╡s 77.296 ┬╡s 78.725 ┬╡s]
                        change: [-6.0147% -3.7571% -1.5488%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high severe
iter_simple/foreach_sparse_set
                        time:   [29.020 ┬╡s 29.381 ┬╡s 29.880 ┬╡s]
                        change: [-5.8107% -3.6934% -1.6238%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe
iter_simple/foreach_wide_sparse_set
                        time:   [172.86 ┬╡s 175.90 ┬╡s 180.27 ┬╡s]
                        change: [+2.4747% +4.4123% +6.4415%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe
iter_simple/foreach_hybrid
                        time:   [18.747 ┬╡s 18.980 ┬╡s 19.219 ┬╡s]
                        change: [-5.6230% -3.2137% -0.7279%] (p = 0.01 < 0.05)
                        Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

par_iter_simple/with_0_fragment
                        time:   [61.199 ┬╡s 61.998 ┬╡s 62.945 ┬╡s]
                        change: [-13.128% -10.087% -7.0325%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
  3 (3.00%) high mild
  8 (8.00%) high severe
par_iter_simple/with_10_fragment
                        time:   [61.146 ┬╡s 61.695 ┬╡s 62.442 ┬╡s]
                        change: [-13.508% -9.3113% -4.4109%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 16 outliers among 100 measurements (16.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  12 (12.00%) high severe
par_iter_simple/with_100_fragment
                        time:   [65.350 ┬╡s 66.216 ┬╡s 67.203 ┬╡s]
                        change: [-13.946% -10.639% -6.9263%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
  5 (5.00%) high mild
  6 (6.00%) high severe
par_iter_simple/with_1000_fragment
                        time:   [84.223 ┬╡s 85.536 ┬╡s 87.631 ┬╡s]
                        change: [-22.306% -19.001% -15.570%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
  9 (9.00%) high mild
  4 (4.00%) high severe
par_iter_simple/hybrid  time:   [114.74 ┬╡s 116.46 ┬╡s 118.28 ┬╡s]
                        change: [-30.363% -25.841% -20.903%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  2 (2.00%) high mild
  2 (2.00%) high severe

iter_fragmented(4096)_empty/foreach_table
                        time:   [6.0533 ┬╡s 6.1104 ┬╡s 6.1708 ┬╡s]
                        change: [-10.591% -8.9078% -7.1521%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  5 (5.00%) high mild
iter_fragmented(4096)_empty/foreach_sparse
                        time:   [17.254 ┬╡s 17.430 ┬╡s 17.612 ┬╡s]
                        change: [-7.1579% -5.4131% -3.6311%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe

world_query_iter/50000_entities_table
                        time:   [32.081 ┬╡s 32.689 ┬╡s 33.641 ┬╡s]
                        change: [-5.4646% -2.7668% -0.1285%] (p = 0.05 < 0.05)
                        Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe
world_query_iter/50000_entities_sparse
                        time:   [70.429 ┬╡s 71.046 ┬╡s 71.597 ┬╡s]
                        change: [-9.9288% -7.3588% -4.9031%] (p = 0.00 < 0.05)
                        Performance has improved.

chescock added 2 commits February 11, 2025 21:50

Move implementations of Query methods from QueryState to Query.

33ed41c

Add missing `iter_many_unique_inner` and `single_inner` methods.

Replace use of removed is_empty_unsafe_world_cell.

6da68a5

chescock added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 12, 2025

cBournhonesque reviewed Feb 12, 2025

View reviewed changes

crates/bevy_ecs/src/query/state.rs Show resolved Hide resolved

cBournhonesque approved these changes Feb 12, 2025

View reviewed changes

Move get_many_unchecked_manual_uniqueness() test from the file with…

d55f688

… `QueryState` to the file with `Query`.

Carter0 approved these changes Feb 13, 2025

View reviewed changes

chescock added 3 commits February 13, 2025 19:23

Merge remote-tracking branch 'remotes/origin/main' into move-query-impls

f2c3d89

# Conflicts: # crates/bevy_ecs/src/system/query.rs

Update new uses of iter_many_unchecked_manual and `iter_many_unique…

5e38516

…_unchecked_manual` for parallel iteration.

Missed an _inner suffix.

51046ce

chescock added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 14, 2025

Remove validate_world() call from `query_unchecked_manual_with_tick…

14007f9

…s()` and make it a safety requirement.

chescock added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Feb 16, 2025

alice-i-cecile added this pull request to the merge queue Feb 16, 2025

Merged via the queue into bevyengine:main with commit 794bf6a Feb 16, 2025
30 checks passed

chescock mentioned this pull request May 10, 2025

Combine Query and QueryLens using a type parameter for state #18162

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Move implementations of `Query` methods from `QueryState` to `Query`. #17822

Move implementations of `Query` methods from `QueryState` to `Query`. #17822

Uh oh!

chescock commented Feb 12, 2025

Uh oh!

Uh oh!

cBournhonesque left a comment

Uh oh!

hymm commented Feb 13, 2025

Uh oh!

Carter0 left a comment

Uh oh!

chescock commented Feb 13, 2025

Uh oh!

chescock commented Feb 15, 2025

Uh oh!

chescock commented Feb 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Move implementations of Query methods from QueryState to Query. #17822

Move implementations of Query methods from QueryState to Query. #17822

Uh oh!

Conversation

chescock commented Feb 12, 2025

Objective

Solution

Future Work

Uh oh!

Uh oh!

cBournhonesque left a comment

Choose a reason for hiding this comment

Uh oh!

hymm commented Feb 13, 2025

Uh oh!

Carter0 left a comment

Choose a reason for hiding this comment

Uh oh!

chescock commented Feb 13, 2025

Uh oh!

chescock commented Feb 15, 2025

Uh oh!

chescock commented Feb 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Move implementations of `Query` methods from `QueryState` to `Query`. #17822

Move implementations of `Query` methods from `QueryState` to `Query`. #17822