From 17ad85565351d89ab41db11c25bb0cade5727eb3 Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Sun, 29 Dec 2024 14:33:42 -0500 Subject: [PATCH] Migrate to `core::hint::black_box()` (#16980) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective Many of our benchmarks use [`criterion::black_box()`](https://docs.rs/criterion/latest/criterion/fn.black_box.html), which is used to prevent the compiler from optimizing away computation that we're trying to time. This can be slow, though, because `criterion::black_box()` forces a point read each time it is called through [`ptr::road_volatile()`](https://doc.rust-lang.org/stable/std/ptr/fn.read_volatile.html). In Rust 1.66, the standard library introduced [`core::hint::black_box()`](https://doc.rust-lang.org/nightly/std/hint/fn.black_box.html) (and `std::hint::black_box()`). This is an intended replacement for `criterion`'s version that uses compiler intrinsics instead of volatile pointer reads, and thus has no runtime overhead. This increases benchmark accuracy, which is always nice 👍 Note that benchmarks may _appear_ to improve in performance after this change, but that's just because we are eliminating the pointer read overhead. ## Solution - Deny `criterion::black_box` in `clippy.toml`. - Fix all imports. ## Testing - `cargo clippy -p benches --benches` --- benches/benches/bevy_ecs/change_detection.rs | 4 +++- benches/benches/bevy_ecs/empty_archetypes.rs | 4 +++- benches/benches/bevy_ecs/entity_cloning.rs | 4 +++- benches/benches/bevy_ecs/observers/propagation.rs | 4 +++- benches/benches/bevy_ecs/observers/simple.rs | 4 +++- benches/benches/bevy_ecs/world/commands.rs | 4 +++- benches/benches/bevy_ecs/world/world_get.rs | 4 +++- benches/benches/bevy_math/bezier.rs | 4 +++- benches/benches/bevy_picking/ray_mesh_intersection.rs | 4 +++- benches/benches/bevy_reflect/list.rs | 6 +++--- benches/benches/bevy_reflect/map.rs | 6 +++--- benches/benches/bevy_reflect/path.rs | 4 ++-- benches/benches/bevy_reflect/struct.rs | 6 +++--- benches/benches/bevy_render/render_layers.rs | 4 +++- benches/benches/bevy_render/torus.rs | 4 +++- benches/benches/bevy_tasks/iter.rs | 4 +++- clippy.toml | 1 + 17 files changed, 48 insertions(+), 23 deletions(-) diff --git a/benches/benches/bevy_ecs/change_detection.rs b/benches/benches/bevy_ecs/change_detection.rs index ca57da93fa..8d49c2b8c7 100644 --- a/benches/benches/bevy_ecs/change_detection.rs +++ b/benches/benches/bevy_ecs/change_detection.rs @@ -1,3 +1,5 @@ +use core::hint::black_box; + use bevy_ecs::{ component::{Component, Mutable}, entity::Entity, @@ -5,7 +7,7 @@ use bevy_ecs::{ query::QueryFilter, world::World, }; -use criterion::{black_box, criterion_group, Criterion}; +use criterion::{criterion_group, Criterion}; use rand::{prelude::SliceRandom, SeedableRng}; use rand_chacha::ChaCha8Rng; diff --git a/benches/benches/bevy_ecs/empty_archetypes.rs b/benches/benches/bevy_ecs/empty_archetypes.rs index daec970b74..91c2b5427a 100644 --- a/benches/benches/bevy_ecs/empty_archetypes.rs +++ b/benches/benches/bevy_ecs/empty_archetypes.rs @@ -1,5 +1,7 @@ +use core::hint::black_box; + use bevy_ecs::{component::Component, prelude::*, schedule::ExecutorKind, world::World}; -use criterion::{black_box, criterion_group, BenchmarkId, Criterion}; +use criterion::{criterion_group, BenchmarkId, Criterion}; criterion_group!(benches, empty_archetypes); diff --git a/benches/benches/bevy_ecs/entity_cloning.rs b/benches/benches/bevy_ecs/entity_cloning.rs index 51af20b7b1..218a551584 100644 --- a/benches/benches/bevy_ecs/entity_cloning.rs +++ b/benches/benches/bevy_ecs/entity_cloning.rs @@ -1,10 +1,12 @@ +use core::hint::black_box; + use bevy_ecs::bundle::Bundle; use bevy_ecs::reflect::AppTypeRegistry; use bevy_ecs::{component::Component, reflect::ReflectComponent, world::World}; use bevy_hierarchy::{BuildChildren, CloneEntityHierarchyExt}; use bevy_math::Mat4; use bevy_reflect::{GetTypeRegistration, Reflect}; -use criterion::{black_box, criterion_group, criterion_main, Bencher, Criterion}; +use criterion::{criterion_group, criterion_main, Bencher, Criterion}; criterion_group!(benches, reflect_benches, clone_benches); criterion_main!(benches); diff --git a/benches/benches/bevy_ecs/observers/propagation.rs b/benches/benches/bevy_ecs/observers/propagation.rs index 5de85bc326..1989c05fc6 100644 --- a/benches/benches/bevy_ecs/observers/propagation.rs +++ b/benches/benches/bevy_ecs/observers/propagation.rs @@ -1,9 +1,11 @@ +use core::hint::black_box; + use bevy_ecs::{ component::Component, entity::Entity, event::Event, observer::Trigger, world::World, }; use bevy_hierarchy::{BuildChildren, Parent}; -use criterion::{black_box, Criterion}; +use criterion::Criterion; use rand::SeedableRng; use rand::{seq::IteratorRandom, Rng}; use rand_chacha::ChaCha8Rng; diff --git a/benches/benches/bevy_ecs/observers/simple.rs b/benches/benches/bevy_ecs/observers/simple.rs index 81dd8e021e..bf2dd236d6 100644 --- a/benches/benches/bevy_ecs/observers/simple.rs +++ b/benches/benches/bevy_ecs/observers/simple.rs @@ -1,6 +1,8 @@ +use core::hint::black_box; + use bevy_ecs::{entity::Entity, event::Event, observer::Trigger, world::World}; -use criterion::{black_box, Criterion}; +use criterion::Criterion; use rand::{prelude::SliceRandom, SeedableRng}; use rand_chacha::ChaCha8Rng; fn deterministic_rand() -> ChaCha8Rng { diff --git a/benches/benches/bevy_ecs/world/commands.rs b/benches/benches/bevy_ecs/world/commands.rs index a1d7cdb09e..9f5a93bf14 100644 --- a/benches/benches/bevy_ecs/world/commands.rs +++ b/benches/benches/bevy_ecs/world/commands.rs @@ -1,9 +1,11 @@ +use core::hint::black_box; + use bevy_ecs::{ component::Component, system::Commands, world::{Command, CommandQueue, World}, }; -use criterion::{black_box, Criterion}; +use criterion::Criterion; #[derive(Component)] struct A; diff --git a/benches/benches/bevy_ecs/world/world_get.rs b/benches/benches/bevy_ecs/world/world_get.rs index 190402fbad..fcb9b0116b 100644 --- a/benches/benches/bevy_ecs/world/world_get.rs +++ b/benches/benches/bevy_ecs/world/world_get.rs @@ -1,3 +1,5 @@ +use core::hint::black_box; + use bevy_ecs::{ bundle::Bundle, component::Component, @@ -5,7 +7,7 @@ use bevy_ecs::{ system::{Query, SystemState}, world::World, }; -use criterion::{black_box, Criterion}; +use criterion::Criterion; use rand::{prelude::SliceRandom, SeedableRng}; use rand_chacha::ChaCha8Rng; diff --git a/benches/benches/bevy_math/bezier.rs b/benches/benches/bevy_math/bezier.rs index 404ab08a63..1858a05466 100644 --- a/benches/benches/bevy_math/bezier.rs +++ b/benches/benches/bevy_math/bezier.rs @@ -1,4 +1,6 @@ -use criterion::{black_box, criterion_group, Criterion}; +use core::hint::black_box; + +use criterion::{criterion_group, Criterion}; use bevy_math::prelude::*; diff --git a/benches/benches/bevy_picking/ray_mesh_intersection.rs b/benches/benches/bevy_picking/ray_mesh_intersection.rs index 1d019d43ee..e8b35078f7 100644 --- a/benches/benches/bevy_picking/ray_mesh_intersection.rs +++ b/benches/benches/bevy_picking/ray_mesh_intersection.rs @@ -1,6 +1,8 @@ +use core::hint::black_box; + use bevy_math::{Dir3, Mat4, Ray3d, Vec3}; use bevy_picking::mesh_picking::ray_cast; -use criterion::{black_box, criterion_group, Criterion}; +use criterion::{criterion_group, Criterion}; fn ptoxznorm(p: u32, size: u32) -> (f32, f32) { let ij = (p / (size), p % (size)); diff --git a/benches/benches/bevy_reflect/list.rs b/benches/benches/bevy_reflect/list.rs index dca727594f..872c2dd0cb 100644 --- a/benches/benches/bevy_reflect/list.rs +++ b/benches/benches/bevy_reflect/list.rs @@ -1,10 +1,10 @@ -use core::{iter, time::Duration}; +use core::{hint::black_box, iter, time::Duration}; use benches::bench; use bevy_reflect::{DynamicList, List}; use criterion::{ - black_box, criterion_group, measurement::Measurement, AxisScale, BatchSize, BenchmarkGroup, - BenchmarkId, Criterion, PlotConfiguration, Throughput, + criterion_group, measurement::Measurement, AxisScale, BatchSize, BenchmarkGroup, BenchmarkId, + Criterion, PlotConfiguration, Throughput, }; criterion_group!( diff --git a/benches/benches/bevy_reflect/map.rs b/benches/benches/bevy_reflect/map.rs index f8f47feccb..d1d9d83603 100644 --- a/benches/benches/bevy_reflect/map.rs +++ b/benches/benches/bevy_reflect/map.rs @@ -1,11 +1,11 @@ -use core::{fmt::Write, iter, time::Duration}; +use core::{fmt::Write, hint::black_box, iter, time::Duration}; use benches::bench; use bevy_reflect::{DynamicMap, Map}; use bevy_utils::HashMap; use criterion::{ - black_box, criterion_group, measurement::Measurement, AxisScale, BatchSize, BenchmarkGroup, - BenchmarkId, Criterion, PlotConfiguration, Throughput, + criterion_group, measurement::Measurement, AxisScale, BatchSize, BenchmarkGroup, BenchmarkId, + Criterion, PlotConfiguration, Throughput, }; criterion_group!( diff --git a/benches/benches/bevy_reflect/path.rs b/benches/benches/bevy_reflect/path.rs index 9cf5d7250b..c0d8bfe0da 100644 --- a/benches/benches/bevy_reflect/path.rs +++ b/benches/benches/bevy_reflect/path.rs @@ -1,8 +1,8 @@ -use core::{fmt::Write, str, time::Duration}; +use core::{fmt::Write, hint::black_box, str, time::Duration}; use benches::bench; use bevy_reflect::ParsedPath; -use criterion::{black_box, criterion_group, BatchSize, BenchmarkId, Criterion, Throughput}; +use criterion::{criterion_group, BatchSize, BenchmarkId, Criterion, Throughput}; use rand::{distributions::Uniform, Rng, SeedableRng}; use rand_chacha::ChaCha8Rng; diff --git a/benches/benches/bevy_reflect/struct.rs b/benches/benches/bevy_reflect/struct.rs index d71d006536..d8f25554c3 100644 --- a/benches/benches/bevy_reflect/struct.rs +++ b/benches/benches/bevy_reflect/struct.rs @@ -1,10 +1,10 @@ -use core::time::Duration; +use core::{hint::black_box, time::Duration}; use benches::bench; use bevy_reflect::{DynamicStruct, GetField, PartialReflect, Reflect, Struct}; use criterion::{ - black_box, criterion_group, measurement::Measurement, AxisScale, BatchSize, BenchmarkGroup, - BenchmarkId, Criterion, PlotConfiguration, Throughput, + criterion_group, measurement::Measurement, AxisScale, BatchSize, BenchmarkGroup, BenchmarkId, + Criterion, PlotConfiguration, Throughput, }; criterion_group!( diff --git a/benches/benches/bevy_render/render_layers.rs b/benches/benches/bevy_render/render_layers.rs index 42dd5356b5..d460a7bc96 100644 --- a/benches/benches/bevy_render/render_layers.rs +++ b/benches/benches/bevy_render/render_layers.rs @@ -1,4 +1,6 @@ -use criterion::{black_box, criterion_group, Criterion}; +use core::hint::black_box; + +use criterion::{criterion_group, Criterion}; use bevy_render::view::RenderLayers; diff --git a/benches/benches/bevy_render/torus.rs b/benches/benches/bevy_render/torus.rs index a5ef753bc8..dcadd09180 100644 --- a/benches/benches/bevy_render/torus.rs +++ b/benches/benches/bevy_render/torus.rs @@ -1,4 +1,6 @@ -use criterion::{black_box, criterion_group, Criterion}; +use core::hint::black_box; + +use criterion::{criterion_group, Criterion}; use bevy_render::mesh::TorusMeshBuilder; diff --git a/benches/benches/bevy_tasks/iter.rs b/benches/benches/bevy_tasks/iter.rs index 47c2f6de14..7fe00ecb79 100644 --- a/benches/benches/bevy_tasks/iter.rs +++ b/benches/benches/bevy_tasks/iter.rs @@ -1,5 +1,7 @@ +use core::hint::black_box; + use bevy_tasks::{ParallelIterator, TaskPoolBuilder}; -use criterion::{black_box, criterion_group, BenchmarkId, Criterion}; +use criterion::{criterion_group, BenchmarkId, Criterion}; struct ParChunks<'a, T>(core::slice::Chunks<'a, T>); impl<'a, T> ParallelIterator> for ParChunks<'a, T> diff --git a/clippy.toml b/clippy.toml index d1d234817a..26b39b4e84 100644 --- a/clippy.toml +++ b/clippy.toml @@ -41,4 +41,5 @@ disallowed-methods = [ { path = "f32::asinh", reason = "use bevy_math::ops::asinh instead for libm determinism" }, { path = "f32::acosh", reason = "use bevy_math::ops::acosh instead for libm determinism" }, { path = "f32::atanh", reason = "use bevy_math::ops::atanh instead for libm determinism" }, + { path = "criterion::black_box", reason = "use core::hint::black_box instead" }, ]