diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20e423ab..5abde412 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,6 +87,7 @@ jobs: MY_ENV_VAR: "YES" with: run: cargo codspeed run + mode: instrumentation token: ${{ secrets.CODSPEED_TOKEN }} compat-integration-test-walltime: @@ -116,9 +117,9 @@ jobs: uses: CodSpeedHQ/action@main env: MY_ENV_VAR: "YES" - CODSPEED_PERF_ENABLED: true with: run: cargo codspeed run + mode: walltime token: ${{ secrets.CODSPEED_TOKEN }} check: diff --git a/crates/cargo-codspeed/src/app.rs b/crates/cargo-codspeed/src/app.rs index a2578ac9..a1786bfd 100644 --- a/crates/cargo-codspeed/src/app.rs +++ b/crates/cargo-codspeed/src/app.rs @@ -3,7 +3,7 @@ use cargo_metadata::MetadataCommand; use clap::{Args, Parser, Subcommand}; use std::{ffi::OsString, process::exit}; -use crate::build::build_benches; +use crate::build::{build_benches, BuildConfig}; #[derive(Parser)] #[command(author, version, about, long_about = None)] @@ -21,58 +21,70 @@ struct Cli { command: Commands, } +const PACKAGE_HELP: &str = "Package Selection"; #[derive(Args)] pub(crate) struct PackageFilters { /// Select all packages in the workspace - #[arg(long)] + #[arg(long, help_heading = PACKAGE_HELP)] pub(crate) workspace: bool, - /// Exclude packages - #[arg(long)] + /// Package to exclude + #[arg(long, value_name = "SPEC", help_heading = PACKAGE_HELP)] pub(crate) exclude: Vec, - /// Package to select (builds all workspace package by default) - #[arg(short, long)] + /// Package to select + #[arg(short, long, value_name= "SPEC", help_heading = PACKAGE_HELP)] pub(crate) package: Vec, } #[derive(Args)] -pub(crate) struct Filters { - /// Optional list of benchmarks to build (builds all benchmarks by default) +pub(crate) struct BenchTargetFilters { + /// Select only the specified benchmark target (all benchmark targets by default) + #[arg(long, help_heading = TARGET_HELP)] pub(crate) bench: Option>, - #[command(flatten)] - pub(crate) package: PackageFilters, } +const FEATURE_HELP: &str = "Feature Selection"; +const COMPILATION_HELP: &str = "Compilation Options"; +const TARGET_HELP: &str = "Target Selection"; #[derive(Subcommand)] enum Commands { /// Build the benchmarks Build { #[command(flatten)] - filters: Filters, + package_filters: PackageFilters, /// Space or comma separated list of features to activate - #[arg(short = 'F', long)] + #[arg(short = 'F', long, help_heading = FEATURE_HELP)] features: Option, /// Activate all available features of all selected packages. - #[arg(long)] + #[arg(long, help_heading = FEATURE_HELP)] all_features: bool, /// Do not activate the `default` feature of the selected packages. - #[arg(long)] + #[arg(long, help_heading = FEATURE_HELP)] no_default_features: bool, /// Number of parallel jobs, defaults to # of CPUs. - #[arg(short, long)] + #[arg(short, long, help_heading = COMPILATION_HELP)] jobs: Option, /// Build the benchmarks with the specified profile - #[arg(long, default_value = "bench")] + #[arg(long, default_value = "bench", help_heading = COMPILATION_HELP)] profile: String, + + #[command(flatten)] + bench_target_filters: BenchTargetFilters, }, /// Run the previously built benchmarks Run { + /// If specified, only run benches containing this string in their names + benchname: Option, + + #[command(flatten)] + package_filters: PackageFilters, + #[command(flatten)] - filters: Filters, + bench_target_filters: BenchTargetFilters, }, } @@ -85,7 +97,8 @@ pub fn run(args: impl Iterator) -> Result<()> { let res = match cli.command { Commands::Build { - filters, + package_filters, + bench_target_filters, features, all_features, jobs, @@ -113,15 +126,28 @@ pub fn run(args: impl Iterator) -> Result<()> { features.map(|f| f.split([' ', ',']).map(|s| s.to_string()).collect_vec()); build_benches( &metadata, - filters, - features, - profile, - cli.quiet, - measurement_mode, - passthrough_flags, + BuildConfig { + package_filters, + bench_target_filters, + features, + profile, + quiet: cli.quiet, + measurement_mode, + passthrough_flags, + }, ) } - Commands::Run { filters } => run_benches(&metadata, filters, measurement_mode), + Commands::Run { + benchname, + package_filters, + bench_target_filters, + } => run_benches( + &metadata, + benchname, + package_filters, + bench_target_filters, + measurement_mode, + ), }; if let Err(e) = res { diff --git a/crates/cargo-codspeed/src/build.rs b/crates/cargo-codspeed/src/build.rs index 6bf9c117..8d44deee 100644 --- a/crates/cargo-codspeed/src/build.rs +++ b/crates/cargo-codspeed/src/build.rs @@ -1,5 +1,5 @@ use crate::{ - app::{Filters, PackageFilters}, + app::{BenchTargetFilters, PackageFilters}, helpers::{clear_dir, get_codspeed_target_dir}, measurement_mode::MeasurementMode, prelude::*, @@ -8,7 +8,8 @@ use cargo_metadata::{camino::Utf8PathBuf, Message, Metadata, TargetKind}; use std::process::{exit, Command, Stdio}; struct BuildOptions<'a> { - filters: Filters, + bench_target_filters: BenchTargetFilters, + package_filters: PackageFilters, features: &'a Option>, profile: &'a str, passthrough_flags: &'a Vec, @@ -20,6 +21,16 @@ struct BuiltBench { executable_path: Utf8PathBuf, } +pub struct BuildConfig { + pub package_filters: PackageFilters, + pub bench_target_filters: BenchTargetFilters, + pub features: Option>, + pub profile: String, + pub quiet: bool, + pub measurement_mode: MeasurementMode, + pub passthrough_flags: Vec, +} + impl BuildOptions<'_> { /// Builds the benchmarks by invoking cargo /// Returns a list of built benchmarks, with path to associated executables @@ -49,6 +60,19 @@ impl BuildOptions<'_> { ); let mut built_benches = Vec::new(); + + let package_names = self + .package_filters + .packages_from_flags(metadata) + .map_err(|e| { + // Avoid leaving an orphan cargo process, even if something went wrong + cargo.wait().expect("Could not get cargo's exist status"); + e + })? + .into_iter() + .map(|t| t.name.clone()) + .collect::>(); + for message in Message::parse_stream(reader) { match message.expect("Failed to parse message") { // Those messages will include build errors and warnings even if stderr also contain some of them @@ -66,19 +90,14 @@ impl BuildOptions<'_> { .find(|p| p.id == artifact.package_id) .expect("Could not find package"); - let bench_name = artifact.target.name; + let bench_target_name = artifact.target.name; - let add_bench_to_codspeed_dir = match &self.filters.bench { - Some(allowed_bench_names) => allowed_bench_names - .iter() - .any(|allowed_bench_name| bench_name.contains(allowed_bench_name)), - None => true, - }; + let add_bench_to_codspeed_dir = package_names.iter().contains(&package.name); if add_bench_to_codspeed_dir { built_benches.push(BuiltBench { package: package.name.clone(), - bench: bench_name, + bench: bench_target_name, executable_path: artifact .executable .expect("Unexpected missing executable path"), @@ -159,7 +178,15 @@ impl BuildOptions<'_> { /// This command explicitly ignores the `self.benches`: all benches are built fn build_command(&self, measurement_mode: MeasurementMode) -> Command { let mut cargo = Command::new("cargo"); - cargo.args(["build", "--benches"]); + cargo.arg("build"); + + if let Some(bench_target_filters) = &self.bench_target_filters.bench { + for bench_target_filter in bench_target_filters { + cargo.args(["--bench", bench_target_filter]); + } + } else { + cargo.args(["--benches"]); + } self.add_rust_flags(&mut cargo, measurement_mode); @@ -171,7 +198,7 @@ impl BuildOptions<'_> { cargo.arg("--profile").arg(self.profile); - self.filters.package.add_cargo_args(&mut cargo); + self.package_filters.add_cargo_args(&mut cargo); cargo } @@ -197,22 +224,15 @@ impl PackageFilters { } } -pub fn build_benches( - metadata: &Metadata, - filters: Filters, - features: Option>, - profile: String, - quiet: bool, - measurement_mode: MeasurementMode, - passthrough_flags: Vec, -) -> Result<()> { +pub fn build_benches(metadata: &Metadata, config: BuildConfig) -> Result<()> { let built_benches = BuildOptions { - filters, - features: &features, - profile: &profile, - passthrough_flags: &passthrough_flags, + bench_target_filters: config.bench_target_filters, + package_filters: config.package_filters, + features: &config.features, + profile: &config.profile, + passthrough_flags: &config.passthrough_flags, } - .build(metadata, quiet, measurement_mode)?; + .build(metadata, config.quiet, config.measurement_mode)?; if built_benches.is_empty() { bail!( @@ -221,7 +241,7 @@ pub fn build_benches( ); } - let codspeed_target_dir = get_codspeed_target_dir(metadata, measurement_mode); + let codspeed_target_dir = get_codspeed_target_dir(metadata, config.measurement_mode); let built_bench_count = built_benches.len(); // Create and clear packages codspeed target directories diff --git a/crates/cargo-codspeed/src/run.rs b/crates/cargo-codspeed/src/run.rs index 4dbd646f..aaa4914b 100644 --- a/crates/cargo-codspeed/src/run.rs +++ b/crates/cargo-codspeed/src/run.rs @@ -1,5 +1,5 @@ use crate::{ - app::{Filters, PackageFilters}, + app::{BenchTargetFilters, PackageFilters}, helpers::get_codspeed_target_dir, measurement_mode::MeasurementMode, prelude::*, @@ -17,18 +17,43 @@ use std::os::unix::process::ExitStatusExt; struct BenchToRun { bench_path: PathBuf, - bench_name: String, + bench_target_name: String, working_directory: PathBuf, package_name: String, } -impl Filters { +impl PackageFilters { + /// Logic taken from [cargo::ops::Packages](https://docs.rs/cargo/0.85.0/src/cargo/ops/cargo_compile/packages.rs.html#34-42) + pub(crate) fn packages_from_flags<'a>( + &self, + metadata: &'a Metadata, + ) -> Result> { + Ok( + match (self.workspace, self.exclude.len(), self.package.len()) { + (false, 0, 0) => metadata.workspace_default_packages(), + (false, 0, _) => metadata + .workspace_packages() + .into_iter() + .filter(|p| self.package.contains(&p.name)) + .collect(), + (false, _, _) => bail!("--exclude can only be used with --workspace"), + (true, 0, _) => metadata.workspace_packages(), + (true, _, _) => metadata + .workspace_packages() + .into_iter() + .filter(|p| !self.exclude.contains(&p.name)) + .collect(), + }, + ) + } + fn benches_to_run( &self, - codspeed_target_dir: PathBuf, metadata: &Metadata, + bench_target_filters: BenchTargetFilters, + codspeed_target_dir: PathBuf, ) -> Result> { - let packages = self.package.packages(metadata)?; + let packages = self.packages_from_flags(metadata)?; let mut benches = vec![]; for package in packages { @@ -41,18 +66,23 @@ impl Filters { for entry in read_dir { let entry = entry?; let bench_path = entry.path(); - let bench_name = bench_path + let bench_target_name = bench_path .file_name() .unwrap() .to_str() .unwrap() .to_string(); + if let Some(bench_target_filter) = &bench_target_filters.bench { + if !bench_target_filter.contains(&bench_target_name) { + continue; + } + } if !bench_path.is_dir() { benches.push(BenchToRun { package_name: package_name.clone(), working_directory: working_directory.into(), bench_path, - bench_name, + bench_target_name, }); } } @@ -63,41 +93,11 @@ impl Filters { } } -impl PackageFilters { - /// Logic taken from [cargo::ops::Packages](https://docs.rs/cargo/0.85.0/src/cargo/ops/cargo_compile/packages.rs.html#34-42) - fn packages<'a>(&self, metadata: &'a Metadata) -> Result> { - Ok( - match (self.workspace, self.exclude.len(), self.package.len()) { - (false, 0, 0) => metadata.workspace_default_packages(), - (false, 0, _) => metadata - .workspace_packages() - .into_iter() - .filter(|p| { - self.package - .iter() - .any(|allowed_package| p.name.contains(allowed_package)) - }) - .collect(), - (false, _, _) => bail!("--exclude can only be used with --workspace"), - (true, 0, _) => metadata.workspace_packages(), - (true, _, _) => metadata - .workspace_packages() - .into_iter() - .filter(|p| { - !self - .exclude - .iter() - .any(|denied_package| p.name.contains(denied_package)) - }) - .collect(), - }, - ) - } -} - pub fn run_benches( metadata: &Metadata, - filters: Filters, + bench_name_filter: Option, + package_filters: PackageFilters, + bench_target_filters: BenchTargetFilters, measurement_mode: MeasurementMode, ) -> Result<()> { let codspeed_target_dir = get_codspeed_target_dir(metadata, measurement_mode); @@ -105,43 +105,20 @@ pub fn run_benches( if measurement_mode == MeasurementMode::Walltime { WalltimeResults::clear(workspace_root)?; } - let benches = filters.benches_to_run(codspeed_target_dir, metadata)?; + let benches = + package_filters.benches_to_run(metadata, bench_target_filters, codspeed_target_dir)?; if benches.is_empty() { bail!("No benchmarks found. Run `cargo codspeed build` first."); } - let mut to_run = vec![]; - if let Some(allowed_bench_names) = filters.bench { - // Make sure all benchmarks are found - let mut not_found = vec![]; - for allowed_bench_name in allowed_bench_names.iter() { - let bench = benches - .iter() - .find(|b| b.bench_name.contains(allowed_bench_name)); - - if let Some(bench) = bench { - to_run.push(bench); - } else { - not_found.push(allowed_bench_name); - } - } + eprintln!("Collected {} benchmark suite(s) to run", benches.len()); - if !not_found.is_empty() { - bail!( - "The following benchmarks to run were not found: {}", - not_found.iter().join(", ") - ); - } - } else { - to_run = benches.iter().collect(); - } - eprintln!("Collected {} benchmark suite(s) to run", to_run.len()); - for bench in to_run.iter() { - let bench_name = &bench.bench_name; + for bench in benches.iter() { + let bench_target_name = &bench.bench_target_name; // workspace_root is needed since file! returns the path relatively to the workspace root // while CARGO_MANIFEST_DIR returns the path to the sub package let workspace_root = metadata.workspace_root.clone(); - eprintln!("Running {} {}", &bench.package_name, bench_name); + eprintln!("Running {} {}", &bench.package_name, bench_target_name); let mut command = std::process::Command::new(&bench.bench_path); command .env("CODSPEED_CARGO_WORKSPACE_ROOT", workspace_root) @@ -151,6 +128,10 @@ pub fn run_benches( command.arg("--bench"); // Walltime targets need this additional argument (inherited from running them with `cargo bench`) } + if let Some(bench_name_filter) = bench_name_filter.as_ref() { + command.arg(bench_name_filter); + } + command .status() .map_err(|e| anyhow!("failed to execute the benchmark process: {}", e)) @@ -175,9 +156,9 @@ pub fn run_benches( } } })?; - eprintln!("Done running {bench_name}"); + eprintln!("Done running {bench_target_name}"); } - eprintln!("Finished running {} benchmark suite(s)", to_run.len()); + eprintln!("Finished running {} benchmark suite(s)", benches.len()); if measurement_mode == MeasurementMode::Walltime { aggregate_raw_walltime_data(workspace_root)?; diff --git a/crates/cargo-codspeed/tests/simple-bencher.rs b/crates/cargo-codspeed/tests/simple-bencher.rs index f8cea99a..e193c61e 100644 --- a/crates/cargo-codspeed/tests/simple-bencher.rs +++ b/crates/cargo-codspeed/tests/simple-bencher.rs @@ -45,7 +45,7 @@ fn test_simple_build_single() { let dir = setup(DIR, Project::Simple); cargo_codspeed(&dir) .arg("build") - .arg("another_bencher_example") + .args(["--bench", "another_bencher_example"]) .assert() .success() .stderr(contains("Built 1 benchmark suite(s)")) @@ -58,12 +58,12 @@ fn test_simple_build_and_run_single() { let dir = setup(DIR, Project::Simple); cargo_codspeed(&dir) .arg("build") - .arg("another_bencher_example") + .args(["--bench", "another_bencher_example"]) .assert() .success(); cargo_codspeed(&dir) .arg("run") - .arg("another_bencher_example") + .args(["--bench", "another_bencher_example"]) .assert() .success() .stderr(contains("Finished running 1 benchmark suite(s)")) diff --git a/crates/cargo-codspeed/tests/simple-criterion.rs b/crates/cargo-codspeed/tests/simple-criterion.rs index d7084767..dc4b7dd7 100644 --- a/crates/cargo-codspeed/tests/simple-criterion.rs +++ b/crates/cargo-codspeed/tests/simple-criterion.rs @@ -45,7 +45,7 @@ fn test_criterion_build_single() { let dir = setup(DIR, Project::Simple); cargo_codspeed(&dir) .arg("build") - .arg("another_criterion_example") + .args(["--bench", "another_criterion_example"]) .assert() .success() .stderr(contains("Built 1 benchmark suite(s)")) @@ -58,12 +58,12 @@ fn test_criterion_build_and_run_single() { let dir = setup(DIR, Project::Simple); cargo_codspeed(&dir) .arg("build") - .arg("another_criterion_example") + .args(["--bench", "another_criterion_example"]) .assert() .success(); cargo_codspeed(&dir) .arg("run") - .arg("another_criterion_example") + .args(["--bench", "another_criterion_example"]) .assert() .success() .stderr(contains("Finished running 1 benchmark suite(s)")) @@ -71,6 +71,32 @@ fn test_criterion_build_and_run_single() { teardown(dir); } +#[test] +fn test_criterion_build_and_run_filtered_by_name() { + let dir = setup(DIR, Project::Simple); + cargo_codspeed(&dir).arg("build").assert().success(); + cargo_codspeed(&dir) + .arg("run") + .arg("fib 20") + .assert() + .success() + .stderr(contains("Finished running 2 benchmark suite(s)")); + teardown(dir); +} + +#[test] +fn test_criterion_build_and_run_filtered_by_partial_name() { + let dir = setup(DIR, Project::Simple); + cargo_codspeed(&dir).arg("build").assert().success(); + cargo_codspeed(&dir) + .arg("run") + .arg("bubble") + .assert() + .success() + .stderr(contains("Finished running 2 benchmark suite(s)")); + teardown(dir); +} + #[test] fn test_criterion_cargo_bench_no_run() { let dir = setup(DIR, Project::Simple); diff --git a/crates/cargo-codspeed/tests/simple-divan.rs b/crates/cargo-codspeed/tests/simple-divan.rs index ed7ac968..e7de4179 100644 --- a/crates/cargo-codspeed/tests/simple-divan.rs +++ b/crates/cargo-codspeed/tests/simple-divan.rs @@ -45,7 +45,7 @@ fn test_divan_build_single() { let dir = setup(DIR, Project::Simple); cargo_codspeed(&dir) .arg("build") - .arg("another_divan_example") + .args(["--bench", "another_divan_example"]) .assert() .success() .stderr(contains("Built 1 benchmark suite(s)")) @@ -58,12 +58,12 @@ fn test_divan_build_and_run_single() { let dir = setup(DIR, Project::Simple); cargo_codspeed(&dir) .arg("build") - .arg("another_divan_example") + .args(["--bench", "another_divan_example"]) .assert() .success(); cargo_codspeed(&dir) .arg("run") - .arg("another_divan_example") + .args(["--bench", "another_divan_example"]) .assert() .success() .stderr(contains("Finished running 1 benchmark suite(s)")) @@ -71,6 +71,32 @@ fn test_divan_build_and_run_single() { teardown(dir); } +#[test] +fn test_divan_build_and_run_filtered_by_name() { + let dir = setup(DIR, Project::Simple); + cargo_codspeed(&dir).arg("build").assert().success(); + cargo_codspeed(&dir) + .arg("run") + .arg("fib_20") + .assert() + .success() + .stderr(contains("Finished running 2 benchmark suite(s)")); + teardown(dir); +} + +#[test] +fn test_divan_build_and_run_filtered_by_partial_name() { + let dir = setup(DIR, Project::Simple); + cargo_codspeed(&dir).arg("build").assert().success(); + cargo_codspeed(&dir) + .arg("run") + .arg("bubble_sort") + .assert() + .success() + .stderr(contains("Finished running 2 benchmark suite(s)")); + teardown(dir); +} + #[test] fn test_divan_cargo_bench_no_run() { let dir = setup(DIR, Project::Simple);