From c3020eee9b50f1cb8270099599889082966ccd4d Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Fri, 28 Oct 2022 17:23:59 -0400 Subject: [PATCH 1/6] add a benchmark for `get_many` --- benches/benches/bevy_ecs/world/mod.rs | 1 + benches/benches/bevy_ecs/world/world_get.rs | 63 +++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/benches/benches/bevy_ecs/world/mod.rs b/benches/benches/bevy_ecs/world/mod.rs index 8505087e48894..507d9c5583ebe 100644 --- a/benches/benches/bevy_ecs/world/mod.rs +++ b/benches/benches/bevy_ecs/world/mod.rs @@ -27,4 +27,5 @@ criterion_group!( query_get_component_simple, query_get_component, query_get, + query_get_many2, ); diff --git a/benches/benches/bevy_ecs/world/world_get.rs b/benches/benches/bevy_ecs/world/world_get.rs index 1feb7ca07a3eb..bfba124537bdc 100644 --- a/benches/benches/bevy_ecs/world/world_get.rs +++ b/benches/benches/bevy_ecs/world/world_get.rs @@ -402,3 +402,66 @@ pub fn query_get(criterion: &mut Criterion) { group.finish(); } + +pub fn query_get_many2(criterion: &mut Criterion) { + let mut group = criterion.benchmark_group("query_get_many2"); + group.warm_up_time(std::time::Duration::from_millis(500)); + group.measurement_time(std::time::Duration::from_secs(4)); + + for entity_count in RANGE.map(|i| i * 10_000) { + group.bench_function(format!("{}_calls_table", entity_count), |bencher| { + let mut world = World::default(); + let mut entities1: Vec<_> = world + .spawn_batch((0..entity_count).map(|_| (Table::default(),))) + .collect(); + let mut entities2: Vec<_> = world + .spawn_batch((0..entity_count).map(|_| (Table::default(),))) + .collect(); + entities1.shuffle(&mut deterministic_rand()); + entities2.shuffle(&mut deterministic_rand()); + let mut query = SystemState::>::new(&mut world); + let query = query.get(&world); + + bencher.iter(|| { + let mut count = 0; + for [comp_a, comp_b] in std::iter::zip(&entities1, &entities2) + .flat_map(|(&a, &b)| query.get_many([a, b])) + { + black_box(comp_a); + black_box(comp_b); + count += 1; + black_box(count); + } + assert_eq!(black_box(count), entity_count); + }); + }); + group.bench_function(format!("{}_calls_sparse", entity_count), |bencher| { + let mut world = World::default(); + let mut entities1: Vec<_> = world + .spawn_batch((0..entity_count).map(|_| (Sparse::default(),))) + .collect(); + let mut entities2: Vec<_> = world + .spawn_batch((0..entity_count).map(|_| (Sparse::default(),))) + .collect(); + entities1.shuffle(&mut deterministic_rand()); + entities2.shuffle(&mut deterministic_rand()); + let mut query = SystemState::>::new(&mut world); + let query = query.get(&world); + + bencher.iter(|| { + let mut count = 0; + for [comp_a, comp_b] in std::iter::zip(&entities1, &entities2) + .flat_map(|(&a, &b)| query.get_many([a, b])) + { + black_box(comp_a); + black_box(comp_b); + count += 1; + black_box(count); + } + assert_eq!(black_box(count), entity_count); + }); + }); + } + + group.finish(); +} From 8fef159e37f3a0118b3cb9d5c42c9ee1e837688f Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Fri, 28 Oct 2022 17:53:28 -0400 Subject: [PATCH 2/6] make benchmarks generic --- benches/benches/bevy_ecs/world/mod.rs | 4 +- benches/benches/bevy_ecs/world/world_get.rs | 44 +++++++++------------ 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/benches/benches/bevy_ecs/world/mod.rs b/benches/benches/bevy_ecs/world/mod.rs index 507d9c5583ebe..696c5c1fbe410 100644 --- a/benches/benches/bevy_ecs/world/mod.rs +++ b/benches/benches/bevy_ecs/world/mod.rs @@ -27,5 +27,7 @@ criterion_group!( query_get_component_simple, query_get_component, query_get, - query_get_many2, + query_get_many::<2>, + query_get_many::<3>, + query_get_many::<5>, ); diff --git a/benches/benches/bevy_ecs/world/world_get.rs b/benches/benches/bevy_ecs/world/world_get.rs index bfba124537bdc..599f818e6c5d1 100644 --- a/benches/benches/bevy_ecs/world/world_get.rs +++ b/benches/benches/bevy_ecs/world/world_get.rs @@ -403,32 +403,29 @@ pub fn query_get(criterion: &mut Criterion) { group.finish(); } -pub fn query_get_many2(criterion: &mut Criterion) { - let mut group = criterion.benchmark_group("query_get_many2"); +pub fn query_get_many(criterion: &mut Criterion) { + let mut group = criterion.benchmark_group(&format!("query_get_many_{N}")); group.warm_up_time(std::time::Duration::from_millis(500)); group.measurement_time(std::time::Duration::from_secs(4)); for entity_count in RANGE.map(|i| i * 10_000) { group.bench_function(format!("{}_calls_table", entity_count), |bencher| { let mut world = World::default(); - let mut entities1: Vec<_> = world - .spawn_batch((0..entity_count).map(|_| (Table::default(),))) - .collect(); - let mut entities2: Vec<_> = world - .spawn_batch((0..entity_count).map(|_| (Table::default(),))) + let mut entity_groups: Vec<_> = (0..entity_count) + .map(|_| [(); N].map(|_| world.spawn(Table::default()).id())) .collect(); - entities1.shuffle(&mut deterministic_rand()); - entities2.shuffle(&mut deterministic_rand()); + entity_groups.shuffle(&mut deterministic_rand()); + let mut query = SystemState::>::new(&mut world); let query = query.get(&world); bencher.iter(|| { let mut count = 0; - for [comp_a, comp_b] in std::iter::zip(&entities1, &entities2) - .flat_map(|(&a, &b)| query.get_many([a, b])) + for comp in entity_groups + .iter() + .filter_map(|&ids| query.get_many(ids).ok()) { - black_box(comp_a); - black_box(comp_b); + black_box(comp); count += 1; black_box(count); } @@ -437,24 +434,21 @@ pub fn query_get_many2(criterion: &mut Criterion) { }); group.bench_function(format!("{}_calls_sparse", entity_count), |bencher| { let mut world = World::default(); - let mut entities1: Vec<_> = world - .spawn_batch((0..entity_count).map(|_| (Sparse::default(),))) + let mut entity_groups: Vec<_> = (0..entity_count) + .map(|_| [(); N].map(|_| world.spawn(Sparse::default()).id())) .collect(); - let mut entities2: Vec<_> = world - .spawn_batch((0..entity_count).map(|_| (Sparse::default(),))) - .collect(); - entities1.shuffle(&mut deterministic_rand()); - entities2.shuffle(&mut deterministic_rand()); + entity_groups.shuffle(&mut deterministic_rand()); + let mut query = SystemState::>::new(&mut world); let query = query.get(&world); bencher.iter(|| { let mut count = 0; - for [comp_a, comp_b] in std::iter::zip(&entities1, &entities2) - .flat_map(|(&a, &b)| query.get_many([a, b])) + for comp in entity_groups + .iter() + .filter_map(|&ids| query.get_many(ids).ok()) { - black_box(comp_a); - black_box(comp_b); + black_box(comp); count += 1; black_box(count); } @@ -462,6 +456,4 @@ pub fn query_get_many2(criterion: &mut Criterion) { }); }); } - - group.finish(); } From af1c7b7be27922b0205977308bdd2f7fa5970d5a Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Fri, 28 Oct 2022 17:59:38 -0400 Subject: [PATCH 3/6] tweak benchmarks --- benches/benches/bevy_ecs/world/mod.rs | 2 +- benches/benches/bevy_ecs/world/world_get.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/benches/benches/bevy_ecs/world/mod.rs b/benches/benches/bevy_ecs/world/mod.rs index 696c5c1fbe410..f594d082d7fb0 100644 --- a/benches/benches/bevy_ecs/world/mod.rs +++ b/benches/benches/bevy_ecs/world/mod.rs @@ -28,6 +28,6 @@ criterion_group!( query_get_component, query_get, query_get_many::<2>, - query_get_many::<3>, query_get_many::<5>, + query_get_many::<10>, ); diff --git a/benches/benches/bevy_ecs/world/world_get.rs b/benches/benches/bevy_ecs/world/world_get.rs index 599f818e6c5d1..c651c4b09bd8a 100644 --- a/benches/benches/bevy_ecs/world/world_get.rs +++ b/benches/benches/bevy_ecs/world/world_get.rs @@ -406,7 +406,7 @@ pub fn query_get(criterion: &mut Criterion) { pub fn query_get_many(criterion: &mut Criterion) { let mut group = criterion.benchmark_group(&format!("query_get_many_{N}")); group.warm_up_time(std::time::Duration::from_millis(500)); - group.measurement_time(std::time::Duration::from_secs(4)); + group.measurement_time(std::time::Duration::from_secs(2 * N as u64)); for entity_count in RANGE.map(|i| i * 10_000) { group.bench_function(format!("{}_calls_table", entity_count), |bencher| { From fdf566099f4f2771de71bba4be55aa2db59157f4 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Fri, 28 Oct 2022 18:03:47 -0400 Subject: [PATCH 4/6] optimize `Query::get_many{_mut}` --- crates/bevy_ecs/src/query/state.rs | 48 ++++++++++++++---------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 7bbbe3dad266c..738607fe1a467 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -11,7 +11,7 @@ use bevy_tasks::ComputeTaskPool; #[cfg(feature = "trace")] use bevy_utils::tracing::Instrument; use fixedbitset::FixedBitSet; -use std::{borrow::Borrow, fmt}; +use std::{borrow::Borrow, fmt, mem::MaybeUninit}; use super::{NopWorldQuery, QueryItem, QueryManyIter, ROQueryItem, ReadOnlyWorldQuery}; @@ -438,24 +438,23 @@ impl QueryState { last_change_tick: u32, change_tick: u32, ) -> Result<[ROQueryItem<'w, Q>; N], QueryEntityError> { - // SAFETY: fetch is read-only - // and world must be validated - let array_of_results = entities.map(|entity| { - self.as_readonly() - .get_unchecked_manual(world, entity, last_change_tick, change_tick) - }); + let mut values = [(); N].map(|_| MaybeUninit::uninit()); - // TODO: Replace with TryMap once https://github.com/rust-lang/rust/issues/79711 is stabilized - // If any of the get calls failed, bubble up the error - for result in &array_of_results { - match result { - Ok(_) => (), - Err(error) => return Err(*error), - } + for (i, entity) in entities.into_iter().enumerate() { + // SAFETY: fetch is read-only + // and world must be validated + let item = self.as_readonly().get_unchecked_manual( + world, + entity, + last_change_tick, + change_tick, + )?; + values[i] = MaybeUninit::new(item); } - // Since we have verified that all entities are present, we can safely unwrap - Ok(array_of_results.map(|result| result.unwrap())) + // SAFETY: Each value has been fully initialized. + let values = values.map(|x| x.assume_init()); + Ok(values) } /// Gets the query results for the given [`World`] and array of [`Entity`], where the last change and @@ -484,19 +483,16 @@ impl QueryState { } } - let array_of_results = entities - .map(|entity| self.get_unchecked_manual(world, entity, last_change_tick, change_tick)); + let mut values = [(); N].map(|_| MaybeUninit::uninit()); - // If any of the get calls failed, bubble up the error - for result in &array_of_results { - match result { - Ok(_) => (), - Err(error) => return Err(*error), - } + for (i, entity) in entities.into_iter().enumerate() { + let item = self.get_unchecked_manual(world, entity, last_change_tick, change_tick)?; + values[i] = MaybeUninit::new(item); } - // Since we have verified that all entities are present, we can safely unwrap - Ok(array_of_results.map(|result| result.unwrap())) + // SAFETY: Each value has been fully initialized. + let values = values.map(|x| x.assume_init()); + Ok(values) } /// Returns an [`Iterator`] over the query results for the given [`World`]. From 264af4fa1950d30b3ee6dd3b8ecb1138fb4a55ed Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Fri, 28 Oct 2022 18:56:43 -0400 Subject: [PATCH 5/6] safely remove bounds checks --- crates/bevy_ecs/src/query/state.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 738607fe1a467..0d953ea1aaa87 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -440,7 +440,7 @@ impl QueryState { ) -> Result<[ROQueryItem<'w, Q>; N], QueryEntityError> { let mut values = [(); N].map(|_| MaybeUninit::uninit()); - for (i, entity) in entities.into_iter().enumerate() { + for (value, entity) in std::iter::zip(&mut values, entities) { // SAFETY: fetch is read-only // and world must be validated let item = self.as_readonly().get_unchecked_manual( @@ -449,7 +449,7 @@ impl QueryState { last_change_tick, change_tick, )?; - values[i] = MaybeUninit::new(item); + *value = MaybeUninit::new(item); } // SAFETY: Each value has been fully initialized. @@ -485,9 +485,9 @@ impl QueryState { let mut values = [(); N].map(|_| MaybeUninit::uninit()); - for (i, entity) in entities.into_iter().enumerate() { + for (value, entity) in std::iter::zip(&mut values, entities) { let item = self.get_unchecked_manual(world, entity, last_change_tick, change_tick)?; - values[i] = MaybeUninit::new(item); + *value = MaybeUninit::new(item); } // SAFETY: Each value has been fully initialized. From 684a1e79d1ed467ff78f4d38037fa684038c9c25 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 29 Oct 2022 08:19:50 -0400 Subject: [PATCH 6/6] don't use a local when assuming initialization --- crates/bevy_ecs/src/query/state.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 0d953ea1aaa87..a1056162a1d71 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -453,8 +453,7 @@ impl QueryState { } // SAFETY: Each value has been fully initialized. - let values = values.map(|x| x.assume_init()); - Ok(values) + Ok(values.map(|x| x.assume_init())) } /// Gets the query results for the given [`World`] and array of [`Entity`], where the last change and @@ -491,8 +490,7 @@ impl QueryState { } // SAFETY: Each value has been fully initialized. - let values = values.map(|x| x.assume_init()); - Ok(values) + Ok(values.map(|x| x.assume_init())) } /// Returns an [`Iterator`] over the query results for the given [`World`].