From d68bb96d159b62832365951290ebbf7760690312 Mon Sep 17 00:00:00 2001 From: Bruce Mitchener Date: Fri, 26 Jul 2024 00:56:03 +0700 Subject: [PATCH] Always use `std::hint::black_box()`. This has been stable since Rust 1.66 and our MSRV is now 1.70. The feature is kept around for now as removing it would break people's configurations if they were enabling it. It can be removed at the next semver break. Supercedes #701. Fixes #633. Fixes #700. --- CHANGELOG.md | 4 ++++ Cargo.toml | 2 +- README.md | 3 ++- bencher_compat/src/lib.rs | 4 ++-- benches/benchmarks/custom_measurement.rs | 3 ++- book/src/faq.md | 6 ++--- book/src/getting_started.md | 6 +++-- book/src/user_guide/comparing_functions.md | 3 ++- book/src/user_guide/custom_test_framework.md | 3 ++- book/src/user_guide/known_limitations.md | 8 ------- book/src/user_guide/migrating_from_libtest.md | 6 +++-- macro/benches/test_macro_bench.rs | 5 ++-- src/bencher.rs | 8 +++---- src/lib.rs | 24 ++----------------- src/routine.rs | 3 ++- 15 files changed, 35 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec47a13cf..6cc2dd406 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] - MSRV bumped to 1.70 +### Changed +- The `real_blackbox` feature no longer has any impact. Criterion always uses `std::hint::black_box()` now. + Users of `criterion::black_box()` should switch to `std::hint::black_box()`. + ### Fixed - gnuplot version is now correctly detected when using certain Windows binaries/configurations that used to fail diff --git a/Cargo.toml b/Cargo.toml index cea0bc3d8..649147282 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -72,7 +72,7 @@ stable = [ ] default = ["rayon", "plotters", "cargo_bench_support"] -# Enable use of the nightly-only test::black_box function to discourage compiler optimizations. +# This is a legacy feature that no longer does anything, but removing it would be a semver break. real_blackbox = [] # Enable async/await support diff --git a/README.md b/README.md index 5e2dabc71..0c440790d 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,8 @@ harness = false Next, define a benchmark by creating a file at `$PROJECT/benches/my_benchmark.rs` with the following contents: ```rust -use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use std::hint::black_box; +use criterion::{criterion_group, criterion_main, Criterion}; fn fibonacci(n: u64) -> u64 { match n { diff --git a/bencher_compat/src/lib.rs b/bencher_compat/src/lib.rs index 1ddf67563..c50ed7ca4 100644 --- a/bencher_compat/src/lib.rs +++ b/bencher_compat/src/lib.rs @@ -1,7 +1,7 @@ extern crate criterion; +pub use std::hint::black_box; pub use criterion::Criterion; -pub use criterion::black_box; use criterion::measurement::WallTime; /// Stand-in for `bencher::Bencher` which uses Criterion.rs to perform the benchmark instead. @@ -60,4 +60,4 @@ macro_rules! benchmark_main { ($($group_name:path,)+) => { benchmark_main!($($group_name),+); }; -} \ No newline at end of file +} diff --git a/benches/benchmarks/custom_measurement.rs b/benches/benchmarks/custom_measurement.rs index e671f3712..4ab5aa9f6 100644 --- a/benches/benchmarks/custom_measurement.rs +++ b/benches/benchmarks/custom_measurement.rs @@ -1,8 +1,9 @@ use criterion::{ - black_box, criterion_group, + criterion_group, measurement::{Measurement, ValueFormatter}, Criterion, Throughput, }; +use std::hint::black_box; use std::time::{Duration, Instant}; struct HalfSecFormatter; diff --git a/book/src/faq.md b/book/src/faq.md index dd7dc0bf0..109f702fb 100644 --- a/book/src/faq.md +++ b/book/src/faq.md @@ -77,13 +77,11 @@ To see this, consider the following benchmark: ```rust fn compare_small(c: &mut Criterion) { - use criterion::black_box; - let mut group = c.benchmark_group("small"); group.bench_with_input("unlooped", 10, |b, i| b.iter(|| i + 10)); group.bench_with_input("looped", 10, |b, i| b.iter(|| { for _ in 0..10000 { - black_box(i + 10); + std::hint::black_box(i + 10); } })); group.finish(); @@ -111,7 +109,7 @@ Process](./analysis.md) page for more details on how Criterion.rs performs its m the [Timing Loops](./user_guide/timing_loops.md) page for details on choosing a timing loop to minimize measurement overhead. -### When Should I Use `criterion::black_box`? +### When Should I Use `std::hint::black_box`? `black_box` is a function which prevents certain compiler optimizations. Benchmarks are often slightly artificial in nature and the compiler can take advantage of that to generate faster code diff --git a/book/src/getting_started.md b/book/src/getting_started.md index ecee62d73..42ffb3efe 100644 --- a/book/src/getting_started.md +++ b/book/src/getting_started.md @@ -39,7 +39,8 @@ file at `$PROJECT/benches/my_benchmark.rs` with the following contents (see the below for an explanation of this code): ```rust -use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use criterion::{criterion_group, criterion_main, Criterion}; +use std::hint::black_box; use mycrate::fibonacci; fn criterion_benchmark(c: &mut Criterion) { @@ -78,7 +79,8 @@ median [25.733 us 25.988 us] med. abs. dev. [234.09 ns 544.07 ns] Let's go back and walk through that benchmark code in more detail. ```rust -use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use criterion::{criterion_group, criterion_main, Criterion}; +use std::hint::black_box; use mycrate::fibonacci; ``` diff --git a/book/src/user_guide/comparing_functions.md b/book/src/user_guide/comparing_functions.md index 0bc05e830..232137ac2 100644 --- a/book/src/user_guide/comparing_functions.md +++ b/book/src/user_guide/comparing_functions.md @@ -5,7 +5,8 @@ graphs to show the differences in performance between them. First, lets create a benchmark. We can even combine this with benchmarking over a range of inputs. ```rust -use criterion::{black_box, criterion_group, criterion_main, Criterion, BenchmarkId}; +use criterion::{criterion_group, criterion_main, Criterion, BenchmarkId}; +use std::hint::black_box; fn fibonacci_slow(n: u64) -> u64 { match n { diff --git a/book/src/user_guide/custom_test_framework.md b/book/src/user_guide/custom_test_framework.md index d3b5527a8..72618b420 100644 --- a/book/src/user_guide/custom_test_framework.md +++ b/book/src/user_guide/custom_test_framework.md @@ -28,8 +28,9 @@ Let's take a look at an example benchmark (note that this example assumes you're #![feature(custom_test_frameworks)] #![test_runner(criterion::runner)] -use criterion::{Criterion, black_box}; +use criterion::Criterion; use criterion_macro::criterion; +use std::hint::black_box; fn fibonacci(n: u64) -> u64 { match n { diff --git a/book/src/user_guide/known_limitations.md b/book/src/user_guide/known_limitations.md index dd1117419..47448ed56 100644 --- a/book/src/user_guide/known_limitations.md +++ b/book/src/user_guide/known_limitations.md @@ -14,11 +14,3 @@ This results in several limitations: * It is not possible to benchmark functions in crates that do not provide an `rlib`. Criterion.rs cannot currently solve these issues. An [experimental RFC](https://github.com/rust-lang/rust/issues/50297) is being implemented to enable custom test and benchmarking frameworks. - -Second, Criterion.rs provides a stable-compatible replacement for the `black_box` function provided by the standard test crate. This replacement is not as reliable as the official one, and it may allow dead-code-elimination to affect the benchmarks in some circumstances. If you're using a Nightly build of Rust, you can add the `real_blackbox` feature to your dependency on Criterion.rs to use the standard `black_box` function instead. - -Example: - -```toml -criterion = { version = '...', features=['real_blackbox'] } -``` diff --git a/book/src/user_guide/migrating_from_libtest.md b/book/src/user_guide/migrating_from_libtest.md index 88b9ddaa4..ea2058d60 100644 --- a/book/src/user_guide/migrating_from_libtest.md +++ b/book/src/user_guide/migrating_from_libtest.md @@ -48,7 +48,8 @@ criterion = "0.5.1" The next step is to update the imports: ```rust -use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use criterion::{criterion_group, criterion_main, Criterion}; +use std::hint::black_box; ``` Then, we can change the `bench_fib` function. Remove the `#[bench]` and change @@ -72,7 +73,8 @@ criterion_main!(benches); And that's it! The complete migrated benchmark code is below: ```rust -use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use criterion::{criterion_group, criterion_main, Criterion}; +use std::hint::black_box; fn fibonacci(n: u64) -> u64 { match n { diff --git a/macro/benches/test_macro_bench.rs b/macro/benches/test_macro_bench.rs index 7369fd27b..af4696d60 100644 --- a/macro/benches/test_macro_bench.rs +++ b/macro/benches/test_macro_bench.rs @@ -1,7 +1,8 @@ #![feature(custom_test_frameworks)] #![test_runner(criterion::runner)] -use criterion::{Criterion, black_box}; +use std::hint::black_box; +use criterion::Criterion; use criterion_macro::criterion; fn fibonacci(n: u64) -> u64 { @@ -24,4 +25,4 @@ fn bench_simple(c: &mut Criterion) { #[criterion(custom_criterion())] fn bench_custom(c: &mut Criterion) { c.bench_function("Fibonacci-Custom", |b| b.iter(|| fibonacci(black_box(20)))); -} \ No newline at end of file +} diff --git a/src/bencher.rs b/src/bencher.rs index a508fd3e4..a50d168dc 100644 --- a/src/bencher.rs +++ b/src/bencher.rs @@ -1,7 +1,7 @@ +use std::hint::black_box; use std::time::Duration; use std::time::Instant; -use crate::black_box; use crate::measurement::{Measurement, WallTime}; use crate::BatchSize; @@ -103,7 +103,6 @@ impl<'a, M: Measurement> Bencher<'a, M> { /// ```rust /// #[macro_use] extern crate criterion; /// use criterion::*; - /// use criterion::black_box; /// use std::time::Instant; /// /// fn foo() { @@ -115,7 +114,7 @@ impl<'a, M: Measurement> Bencher<'a, M> { /// b.iter_custom(|iters| { /// let start = Instant::now(); /// for _i in 0..iters { - /// black_box(foo()); + /// std::hint::black_box(foo()); /// } /// start.elapsed() /// }) @@ -461,7 +460,6 @@ impl<'a, 'b, A: AsyncExecutor, M: Measurement> AsyncBencher<'a, 'b, A, M> { /// ```rust /// #[macro_use] extern crate criterion; /// use criterion::*; - /// use criterion::black_box; /// use criterion::async_executor::FuturesExecutor; /// use std::time::Instant; /// @@ -475,7 +473,7 @@ impl<'a, 'b, A: AsyncExecutor, M: Measurement> AsyncBencher<'a, 'b, A, M> { /// async move { /// let start = Instant::now(); /// for _i in 0..iters { - /// black_box(foo().await); + /// std::hint::black_box(foo().await); /// } /// start.elapsed() /// } diff --git a/src/lib.rs b/src/lib.rs index 59c973d16..c0dbdde86 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,7 +17,6 @@ #![warn(missing_docs)] #![warn(bare_trait_objects)] -#![cfg_attr(feature = "real_blackbox", feature(test))] #![allow( clippy::just_underscores_and_digits, // Used in the stats code clippy::transmute_ptr_to_ptr, // Used in the stats code @@ -36,9 +35,6 @@ extern crate quickcheck; use regex::Regex; use serde::{Deserialize, Serialize}; -#[cfg(feature = "real_blackbox")] -extern crate test; - // Needs to be declared before other modules // in order to be usable there. #[macro_use] @@ -144,25 +140,9 @@ fn debug_enabled() -> bool { /// A function that is opaque to the optimizer, used to prevent the compiler from /// optimizing away computations in a benchmark. -/// -/// This variant is backed by the (unstable) test::black_box function. -#[cfg(feature = "real_blackbox")] +#[deprecated(note = "use `std::hint::black_box()` instead")] pub fn black_box(dummy: T) -> T { - test::black_box(dummy) -} - -/// A function that is opaque to the optimizer, used to prevent the compiler from -/// optimizing away computations in a benchmark. -/// -/// This variant is stable-compatible, but it may cause some performance overhead -/// or fail to prevent code from being eliminated. -#[cfg(not(feature = "real_blackbox"))] -pub fn black_box(dummy: T) -> T { - unsafe { - let ret = std::ptr::read_volatile(&dummy); - std::mem::forget(dummy); - ret - } + std::hint::black_box(dummy) } /// Argument to [`Bencher::iter_batched`] and [`Bencher::iter_batched_ref`] which controls the diff --git a/src/routine.rs b/src/routine.rs index 88e4318bb..949fb3383 100644 --- a/src/routine.rs +++ b/src/routine.rs @@ -2,7 +2,8 @@ use crate::benchmark::BenchmarkConfig; use crate::connection::OutgoingMessage; use crate::measurement::Measurement; use crate::report::{BenchmarkId, Report, ReportContext}; -use crate::{black_box, ActualSamplingMode, Bencher, Criterion}; +use crate::{ActualSamplingMode, Bencher, Criterion}; +use std::hint::black_box; use std::marker::PhantomData; use std::time::Duration;