From 11b89ad32de4c224795be808363625d69cad7b0d Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Mon, 27 Dec 2021 18:43:00 +0000 Subject: [PATCH] Add a `cargo_criterion_support` feature flag This enables `cargo-criterion` support to be compiled out if it is not required. A compile-time error is added to require that at least one of [`cargo_bench_support`, `cargo_criterion_support`] must be enabled. --- Cargo.toml | 9 ++++++-- src/analysis/mod.rs | 4 +++- src/benchmark_group.rs | 3 +++ src/lib.rs | 47 +++++++++++++++++++++++++++++++++++------- src/routine.rs | 5 ++++- 5 files changed, 57 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 42d8f436..6f4a5a0e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,6 @@ itertools = "0.10" serde = "1.0" serde_json = "1.0" serde_derive = "1.0" -serde_cbor = "0.11" atty = "0.2" clap = { version = "2.33", default-features = false } walkdir = "2.3" @@ -34,6 +33,9 @@ num-traits = { version = "0.2", default-features = false, features = ["std"] oorandom = "11.1" regex = { version = "1.3", default-features = false, features = ["std"] } +# cargo-criterion support +serde_cbor = { version = "0.11", optional = true } + # Optional dependencies rayon = { version = "1.3", optional = true } csv = { version = "1.1", optional = true } @@ -67,7 +69,7 @@ stable = [ "async_tokio", "async_std", ] -default = ["rayon", "plotters", "cargo_bench_support"] +default = ["rayon", "plotters", "cargo_bench_support", "cargo_criterion_support"] # Enable use of the nightly-only test::black_box function to discourage compiler optimizations. real_blackbox = [] @@ -90,6 +92,9 @@ html_reports = [] # required in order to have Criterion.rs be usable outside of cargo-criterion. cargo_bench_support = [] +# This feature enables Criterion.rs to be usable with cargo-criterion. +cargo_criterion_support = ["serde_cbor"] + # This feature _currently_ does nothing, but in 0.4.0 it will be # required in order to have Criterion.rs generate CSV files. This feature is deprecated in favor of # cargo-criterion's --message-format=json option. diff --git a/src/analysis/mod.rs b/src/analysis/mod.rs index d091f634..a1d32dca 100644 --- a/src/analysis/mod.rs +++ b/src/analysis/mod.rs @@ -7,6 +7,7 @@ use crate::stats::univariate::Sample; use crate::stats::{Distribution, Tails}; use crate::benchmark::BenchmarkConfig; +#[cfg(feature = "cargo_criterion_support")] use crate::connection::OutgoingMessage; use crate::estimate::{ build_estimates, ConfidenceInterval, Distributions, Estimate, Estimates, PointEstimates, @@ -92,6 +93,7 @@ pub(crate) fn common( iters = sample.1; times = sample.2; + #[cfg(feature = "cargo_criterion_support")] if let Some(conn) = &criterion.connection { conn.send(&OutgoingMessage::MeasurementComplete { id: id.into(), @@ -247,7 +249,7 @@ pub(crate) fn common( }); } - if criterion.connection.is_none() { + if !criterion.has_connection() { if let Baseline::Save = criterion.baseline { copy_new_dir_to_base( id.as_directory_name(), diff --git a/src/benchmark_group.rs b/src/benchmark_group.rs index 2d9fbddb..106850f7 100644 --- a/src/benchmark_group.rs +++ b/src/benchmark_group.rs @@ -1,5 +1,6 @@ use crate::analysis; use crate::benchmark::PartialBenchmarkConfig; +#[cfg(feature = "cargo_criterion_support")] use crate::connection::OutgoingMessage; use crate::measurement::Measurement; use crate::report::BenchmarkId as InternalBenchmarkId; @@ -306,6 +307,7 @@ impl<'a, M: Measurement> BenchmarkGroup<'a, M> { match self.criterion.mode { Mode::Benchmark => { + #[cfg(feature = "cargo_criterion_support")] if let Some(conn) = &self.criterion.connection { if do_run { conn.send(&OutgoingMessage::BeginningBenchmark { id: (&id).into() }) @@ -369,6 +371,7 @@ impl<'a, M: Measurement> Drop for BenchmarkGroup<'a, M> { fn drop(&mut self) { // I don't really like having a bunch of non-trivial code in drop, but this is the only way // to really write linear types like this in Rust... + #[cfg(feature = "cargo_criterion_support")] if let Some(conn) = &mut self.criterion.connection { conn.send(&OutgoingMessage::FinishedBenchmarkGroup { group: &self.group_name, diff --git a/src/lib.rs b/src/lib.rs index 0f3d7a67..1c0a970c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,6 +27,11 @@ ) )] +#[cfg(not(any(feature = "cargo_bench_support", feature = "cargo_criterion_support")))] +compile_error!( + "At least one of [`cargo_bench_support`, `cargo_criterion_support`] must be enabled" +); + #[cfg(test)] extern crate approx; @@ -56,6 +61,7 @@ mod benchmark; mod benchmark_group; pub mod async_executor; mod bencher; +#[cfg(feature = "cargo_criterion_support")] mod connection; #[cfg(feature = "csv_output")] mod csv_report; @@ -77,17 +83,19 @@ use std::cell::RefCell; use std::collections::HashSet; use std::default::Default; use std::env; +#[cfg(feature = "cargo_criterion_support")] use std::net::TcpStream; use std::path::{Path, PathBuf}; use std::process::Command; +#[cfg(feature = "cargo_criterion_support")] use std::sync::{Mutex, MutexGuard}; use std::time::Duration; use criterion_plot::{Version, VersionError}; use crate::benchmark::BenchmarkConfig; -use crate::connection::Connection; -use crate::connection::OutgoingMessage; +#[cfg(feature = "cargo_criterion_support")] +use crate::connection::{Connection, OutgoingMessage}; use crate::html::Html; use crate::measurement::{Measurement, WallTime}; #[cfg(feature = "plotters")] @@ -123,6 +131,9 @@ lazy_static! { PlottingBackend::None } }; +} +#[cfg(feature = "cargo_criterion_support")] +lazy_static! { static ref CARGO_CRITERION_CONNECTION: Option> = { match std::env::var("CARGO_CRITERION_PORT") { Ok(port_str) => { @@ -133,6 +144,8 @@ lazy_static! { Err(_) => None, } }; +} +lazy_static! { static ref DEFAULT_OUTPUT_DIRECTORY: PathBuf = { // Set criterion home to (in descending order of preference): // - $CRITERION_HOME (cargo-criterion sets this, but other users could as well) @@ -344,6 +357,7 @@ pub struct Criterion { all_titles: HashSet, measurement: M, profiler: Box>, + #[cfg(feature = "cargo_criterion_support")] connection: Option>, mode: Mode, } @@ -411,13 +425,14 @@ impl Default for Criterion { all_titles: HashSet::new(), measurement: WallTime, profiler: Box::new(RefCell::new(ExternalProfiler)), + #[cfg(feature = "cargo_criterion_support")] connection: CARGO_CRITERION_CONNECTION .as_ref() .map(|mtx| mtx.lock().unwrap()), mode: Mode::Benchmark, }; - if criterion.connection.is_some() { + if criterion.has_connection() { // disable all reports when connected to cargo-criterion; it will do the reporting. criterion.report.cli_enabled = false; criterion.report.bencher_enabled = false; @@ -429,6 +444,21 @@ impl Default for Criterion { } impl Criterion { + /// Private helper that returns true if we are running under `cargo-criterion`. + /// + /// Use this function if you have logic that is conditional on `cargo-criterion` being + /// used (or not), but that should always be compiled in. For code that should only be + /// conditionally compiled, use `#[cfg(feature = "cargo_criterion_support")]`. + fn has_connection(&self) -> bool { + #[cfg(not(feature = "cargo_criterion_support"))] + { + false + } + + #[cfg(feature = "cargo_criterion_support")] + self.connection.is_some() + } + /// Changes the measurement for the benchmarks run with this runner. See the /// Measurement trait for more details pub fn with_measurement(self, m: M2) -> Criterion { @@ -445,6 +475,7 @@ impl Criterion { all_titles: self.all_titles, measurement: m, profiler: self.profiler, + #[cfg(feature = "cargo_criterion_support")] connection: self.connection, mode: self.mode, } @@ -611,7 +642,7 @@ impl Criterion { /// Enables plotting pub fn with_plots(mut self) -> Criterion { // If running under cargo-criterion then don't re-enable the reports; let it do the reporting. - if self.connection.is_none() && self.report.html.is_none() { + if !self.has_connection() && self.report.html.is_none() { let default_backend = DEFAULT_PLOTTING_BACKEND.create_plotter(); if let Some(backend) = default_backend { self.report.html = Some(Html::new(backend)); @@ -827,7 +858,7 @@ https://bheisler.github.io/criterion.rs/book/faq.html ") .get_matches(); - if self.connection.is_some() { + if self.has_connection() { if let Some(color) = matches.value_of("color") { if color != "auto" { eprintln!("Warning: --color will be ignored when running with cargo-criterion. Use `cargo criterion --color {} -- ` instead.", color); @@ -889,6 +920,7 @@ https://bheisler.github.io/criterion.rs/book/faq.html }; // This is kind of a hack, but disable the connection to the runner if we're not benchmarking. + #[cfg(feature = "cargo_criterion_support")] if !self.mode.is_benchmark() { self.connection = None; } @@ -921,7 +953,7 @@ https://bheisler.github.io/criterion.rs/book/faq.html self.baseline_directory = dir.to_owned(); } - if self.connection.is_some() { + if self.has_connection() { // disable all reports when connected to cargo-criterion; it will do the reporting. self.report.cli_enabled = false; self.report.bencher_enabled = false; @@ -1057,7 +1089,7 @@ https://bheisler.github.io/criterion.rs/book/faq.html /// Returns true iff we should save the benchmark results in /// json files on the local disk. fn should_save_baseline(&self) -> bool { - self.connection.is_none() + !self.has_connection() && self.load_baseline.is_none() && !matches!(self.baseline, Baseline::Discard) } @@ -1089,6 +1121,7 @@ https://bheisler.github.io/criterion.rs/book/faq.html let group_name = group_name.into(); assert!(!group_name.is_empty(), "Group name must not be empty."); + #[cfg(feature = "cargo_criterion_support")] if let Some(conn) = &self.connection { conn.send(&OutgoingMessage::BeginningBenchmarkGroup { group: &group_name }) .unwrap(); diff --git a/src/routine.rs b/src/routine.rs index b2ad5159..2f1ed06b 100644 --- a/src/routine.rs +++ b/src/routine.rs @@ -1,4 +1,5 @@ use crate::benchmark::BenchmarkConfig; +#[cfg(feature = "cargo_criterion_support")] use crate::connection::OutgoingMessage; use crate::measurement::Measurement; use crate::report::{BenchmarkId, Report, ReportContext}; @@ -37,7 +38,7 @@ pub(crate) trait Routine { .profile(id, report_context, time.as_nanos() as f64); let mut profile_path = report_context.output_directory.clone(); - if (*crate::CARGO_CRITERION_CONNECTION).is_some() { + if criterion.has_connection() { // If connected to cargo-criterion, generate a cargo-criterion-style path. // This is kind of a hack. profile_path.push("profile"); @@ -95,6 +96,7 @@ pub(crate) trait Routine { .report .warmup(id, report_context, wu.as_nanos() as f64); + #[cfg(feature = "cargo_criterion_support")] if let Some(conn) = &criterion.connection { conn.send(&OutgoingMessage::Warmup { id: id.into(), @@ -140,6 +142,7 @@ pub(crate) trait Routine { .report .measurement_start(id, report_context, n, expected_ns, total_iters); + #[cfg(feature = "cargo_criterion_support")] if let Some(conn) = &criterion.connection { conn.send(&OutgoingMessage::MeasurementStart { id: id.into(),