From efb8b9472856ea51b174f0c1cd8be174052f3ba8 Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 19 Jul 2022 15:56:44 -0700 Subject: [PATCH 1/9] Use list mode if --list is passed in. This makes a plain `--list` work. Note that this shouldn't result in any compatibility issues since --list is mutually exclusive with --test. --- src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f35d4cdc..7da332c2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -899,10 +899,10 @@ https://bheisler.github.io/criterion.rs/book/faq.html (false, _) => true, // cargo test --benches should run tests }; - self.mode = if test_mode { - Mode::Test - } else if matches.is_present("list") { + self.mode = if matches.is_present("list") { Mode::List + } else if test_mode { + Mode::Test } else if matches.is_present("profile-time") { let num_seconds = matches.value_of_t_or_exit("profile-time"); From 7873adbc06ac22daa04278980ec894d3dcde8f08 Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 19 Jul 2022 16:09:38 -0700 Subject: [PATCH 2/9] Print out gnuplot warnings to stderr. Warnings shouldn't go to stdout. --- src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7da332c2..b8e8f2d0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -108,8 +108,8 @@ static DEFAULT_PLOTTING_BACKEND: Lazy = Lazy::new(|| match &*GN Ok(_) => PlottingBackend::Gnuplot, Err(e) => { match e { - VersionError::Exec(_) => println!("Gnuplot not found, using plotters backend"), - e => println!( + VersionError::Exec(_) => eprintln!("Gnuplot not found, using plotters backend"), + e => eprintln!( "Gnuplot not found or not usable, using plotters backend\n{}", e ), From 0299e239d94e32931365e0b2cb281deffbf4b05e Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 19 Jul 2022 17:12:51 -0700 Subject: [PATCH 3/9] Change `: bench` at the end of benchmark list output to `: benchmark`. Match libtest benchmarks. --- src/benchmark_group.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/benchmark_group.rs b/src/benchmark_group.rs index 5ca0ab3b..bc85f98a 100644 --- a/src/benchmark_group.rs +++ b/src/benchmark_group.rs @@ -330,7 +330,7 @@ impl<'a, M: Measurement> BenchmarkGroup<'a, M> { } Mode::List => { if do_run { - println!("{}: bench", id); + println!("{}: benchmark", id); } } Mode::Test => { From bd3de4162b412374837bca082052dac759d15975 Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 19 Jul 2022 16:08:47 -0700 Subject: [PATCH 4/9] Support --list --format terse. This is required for cargo-nextest. --- src/benchmark_group.rs | 4 ++-- src/lib.rs | 42 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/benchmark_group.rs b/src/benchmark_group.rs index bc85f98a..687fb2f2 100644 --- a/src/benchmark_group.rs +++ b/src/benchmark_group.rs @@ -328,7 +328,7 @@ impl<'a, M: Measurement> BenchmarkGroup<'a, M> { ); } } - Mode::List => { + Mode::List(_) => { if do_run { println!("{}: benchmark", id); } @@ -392,7 +392,7 @@ impl<'a, M: Measurement> Drop for BenchmarkGroup<'a, M> { self.criterion.measurement.formatter(), ); } - if self.any_matched { + if self.any_matched && !self.criterion.mode.is_terse() { self.criterion.report.group_separator(); } } diff --git a/src/lib.rs b/src/lib.rs index b8e8f2d0..46fe5e24 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -301,7 +301,7 @@ pub(crate) enum Mode { /// Run benchmarks normally. Benchmark, /// List all benchmarks but do not run them. - List, + List(ListFormat), /// Run benchmarks once to verify that they work, but otherwise do not measure them. Test, /// Iterate benchmarks for a given length of time but do not analyze or report on them. @@ -311,6 +311,26 @@ impl Mode { pub fn is_benchmark(&self) -> bool { matches!(self, Mode::Benchmark) } + + pub fn is_terse(&self) -> bool { + matches!(self, Mode::List(ListFormat::Terse)) + } +} + +#[derive(Debug, Clone)] +/// Enum representing the list format. +pub(crate) enum ListFormat { + /// The regular, default format. + Pretty, + /// The terse format, where nothing other than the name of the test and ": benchmark" at the end + /// is printed out. + Terse, +} + +impl Default for ListFormat { + fn default() -> Self { + Self::Pretty + } } /// The benchmark manager @@ -769,6 +789,13 @@ impl Criterion { .long("list") .help("List all benchmarks") .conflicts_with_all(&["test", "profile-time"])) + .arg(Arg::new("format") + .long("format") + .possible_values(&["pretty", "terse"]) + .default_value("pretty") + // Note that libtest's --format also works during test execution, but criterion + // doesn't support that at the moment. + .help("Output formatting")) .arg(Arg::new("profile-time") .long("profile-time") .takes_value(true) @@ -900,7 +927,18 @@ https://bheisler.github.io/criterion.rs/book/faq.html }; self.mode = if matches.is_present("list") { - Mode::List + let list_format = match matches + .value_of("format") + .expect("a default value was provided for this") + { + "pretty" => ListFormat::Pretty, + "terse" => ListFormat::Terse, + other => unreachable!( + "unrecognized value for --format that isn't part of possible-values: {}", + other + ), + }; + Mode::List(list_format) } else if test_mode { Mode::Test } else if matches.is_present("profile-time") { From 32afea3ee5749abc6b28af779ee1a15659e3a674 Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 19 Jul 2022 17:49:45 -0700 Subject: [PATCH 5/9] Support --ignored. Since criterion doesn't have a notion of ignored tests, this skips over all tests. --- src/lib.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 46fe5e24..3bf57b58 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -333,6 +333,17 @@ impl Default for ListFormat { } } +/// Benchmark filtering support. +#[derive(Clone, Debug)] +pub enum BenchmarkFilter { + /// Run all benchmarks. + AcceptAll, + /// Run benchmarks matching this regex. + Regex(Regex), + /// Do not run any benchmarks. + RejectAll, +} + /// The benchmark manager /// /// `Criterion` lets you configure and execute benchmarks @@ -349,7 +360,7 @@ impl Default for ListFormat { /// benchmark. pub struct Criterion { config: BenchmarkConfig, - filter: Option, + filter: BenchmarkFilter, report: Reports, output_directory: PathBuf, baseline_directory: String, @@ -417,7 +428,7 @@ impl Default for Criterion { sampling_mode: SamplingMode::Auto, quick_mode: false, }, - filter: None, + filter: BenchmarkFilter::AcceptAll, report: reports, baseline_directory: "base".to_owned(), baseline: Baseline::Save, @@ -678,6 +689,8 @@ impl Criterion { #[must_use] /// Filters the benchmarks. Only benchmarks with names that contain the /// given string will be executed. + /// + /// This overwrites [`Self::with_benchmark_filter`]. pub fn with_filter>(mut self, filter: S) -> Criterion { let filter_text = filter.into(); let filter = Regex::new(&filter_text).unwrap_or_else(|err| { @@ -686,7 +699,16 @@ impl Criterion { filter_text, err ) }); - self.filter = Some(filter); + self.filter = BenchmarkFilter::Regex(filter); + + self + } + + /// Only run benchmarks specified by the given filter. + /// + /// This overwrites [`Self::with_filter`]. + pub fn with_benchmark_filter(mut self, filter: BenchmarkFilter) -> Criterion { + self.filter = filter; self } @@ -796,6 +818,9 @@ impl Criterion { // Note that libtest's --format also works during test execution, but criterion // doesn't support that at the moment. .help("Output formatting")) + .arg(Arg::new("ignored") + .long("ignored") + .help("List or run ignored benchmarks (currently means skip all benchmarks)")) .arg(Arg::new("profile-time") .long("profile-time") .takes_value(true) @@ -959,9 +984,21 @@ https://bheisler.github.io/criterion.rs/book/faq.html self.connection = None; } - if let Some(filter) = matches.value_of("FILTER") { - self = self.with_filter(filter); - } + let filter = if matches.is_present("ignored") { + // --ignored overwrites any name-based filters passed in. + BenchmarkFilter::RejectAll + } else if let Some(filter) = matches.value_of("FILTER") { + let regex = Regex::new(filter).unwrap_or_else(|err| { + panic!( + "Unable to parse '{}' as a regular expression: {}", + filter, err + ) + }); + BenchmarkFilter::Regex(regex) + } else { + BenchmarkFilter::AcceptAll + }; + self = self.with_benchmark_filter(filter); match matches.value_of("plotting-backend") { // Use plotting_backend() here to re-use the panic behavior if Gnuplot is not available. @@ -1097,8 +1134,9 @@ https://bheisler.github.io/criterion.rs/book/faq.html fn filter_matches(&self, id: &str) -> bool { match &self.filter { - Some(regex) => regex.is_match(id), - None => true, + BenchmarkFilter::AcceptAll => true, + BenchmarkFilter::Regex(regex) => regex.is_match(id), + BenchmarkFilter::RejectAll => false, } } From ea3f2891c165f479f3eac3ee27b8e8056896a729 Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 19 Jul 2022 17:55:08 -0700 Subject: [PATCH 6/9] Support running benchmarks with --exact. Required by cargo-nextest. --- src/lib.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3bf57b58..ad7a0b91 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -340,6 +340,8 @@ pub enum BenchmarkFilter { AcceptAll, /// Run benchmarks matching this regex. Regex(Regex), + /// Run the benchmark matching this string exactly. + Exact(String), /// Do not run any benchmarks. RejectAll, } @@ -821,6 +823,9 @@ impl Criterion { .arg(Arg::new("ignored") .long("ignored") .help("List or run ignored benchmarks (currently means skip all benchmarks)")) + .arg(Arg::new("exact") + .long("exact") + .help("Run benchmarks that exactly match the provided filter")) .arg(Arg::new("profile-time") .long("profile-time") .takes_value(true) @@ -988,13 +993,17 @@ https://bheisler.github.io/criterion.rs/book/faq.html // --ignored overwrites any name-based filters passed in. BenchmarkFilter::RejectAll } else if let Some(filter) = matches.value_of("FILTER") { - let regex = Regex::new(filter).unwrap_or_else(|err| { - panic!( - "Unable to parse '{}' as a regular expression: {}", - filter, err - ) - }); - BenchmarkFilter::Regex(regex) + if matches.is_present("exact") { + BenchmarkFilter::Exact(filter.to_owned()) + } else { + let regex = Regex::new(filter).unwrap_or_else(|err| { + panic!( + "Unable to parse '{}' as a regular expression: {}", + filter, err + ) + }); + BenchmarkFilter::Regex(regex) + } } else { BenchmarkFilter::AcceptAll }; @@ -1136,6 +1145,7 @@ https://bheisler.github.io/criterion.rs/book/faq.html match &self.filter { BenchmarkFilter::AcceptAll => true, BenchmarkFilter::Regex(regex) => regex.is_match(id), + BenchmarkFilter::Exact(exact) => id == exact, BenchmarkFilter::RejectAll => false, } } From 785cf8ae26daea67cd904cec0c3a36def41aec26 Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 19 Jul 2022 18:08:36 -0700 Subject: [PATCH 7/9] Set CARGO_TERM_COLOR=always in GitHub Actions CI. GitHub CI supports ANSI color codes, so this makes the output look a lot prettier. --- .github/workflows/ci.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d7f39e49..b798ed63 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -8,6 +8,8 @@ on: - version-0.4 name: tests +env: + CARGO_TERM_COLOR: always jobs: ci: From 523d42c0a767a9e4a77782ea300a31a43a323f03 Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 19 Jul 2022 18:19:39 -0700 Subject: [PATCH 8/9] Add a nextest compatibility check to GitHub Actions CI. --- .github/workflows/ci.yaml | 24 ++++++++++++++++++++++++ ci/nextest-compat.sh | 11 +++++++++++ 2 files changed, 35 insertions(+) create mode 100755 ci/nextest-compat.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b798ed63..5ee644bc 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -86,3 +86,27 @@ jobs: export PATH=$HOME/.wasmtime/bin/:$PATH cargo install cargo-wasi cargo wasi bench --no-default-features -- --test + + nextest-compat: + name: Check compatibility with nextest + runs-on: ubuntu-latest + strategy: + matrix: + rust: + - stable + + steps: + - uses: actions/checkout@v2 + + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: ${{ matrix.rust }} + override: true + components: rustfmt, clippy + + - uses: Swatinem/rust-cache@v1 + + - uses: taiki-e/install-action@nextest + + - run: ci/nextest-compat.sh diff --git a/ci/nextest-compat.sh b/ci/nextest-compat.sh new file mode 100755 index 00000000..fed9f2f9 --- /dev/null +++ b/ci/nextest-compat.sh @@ -0,0 +1,11 @@ +#!/bin/bash +set -ex -o pipefail + +CARGO=${CARGO:-cargo} + +cd "$(git rev-parse --show-toplevel)" + +echo "Checking benches/bench_main..." + +$CARGO nextest list --benches +$CARGO nextest run --benches From 461818f4a726a6da0f460cb5af31732e89582747 Mon Sep 17 00:00:00 2001 From: David Himmelstrup Date: Tue, 6 Dec 2022 10:01:09 +0100 Subject: [PATCH 9/9] Apply clippy suggestion. --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index bfbd3e93..4bd772ca 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -817,7 +817,7 @@ impl Criterion { .conflicts_with_all(&["test", "profile-time"])) .arg(Arg::new("format") .long("format") - .possible_values(&["pretty", "terse"]) + .possible_values(["pretty", "terse"]) .default_value("pretty") // Note that libtest's --format also works during test execution, but criterion // doesn't support that at the moment.