From c49291481722dac7c5fcbc591aa274346796e39b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 27 May 2025 11:07:54 +0200 Subject: [PATCH 1/2] Rename `BenchmarkFilter` to `RuntimeBenchmarkFilter` --- collector/src/bin/collector.rs | 14 +++++++------- collector/src/runtime/benchmark.rs | 8 ++++---- collector/src/runtime/mod.rs | 8 ++++---- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/collector/src/bin/collector.rs b/collector/src/bin/collector.rs index c82929267..c5e310743 100644 --- a/collector/src/bin/collector.rs +++ b/collector/src/bin/collector.rs @@ -45,8 +45,8 @@ use collector::compile::execute::bencher::BenchProcessor; use collector::compile::execute::profiler::{ProfileProcessor, Profiler}; use collector::runtime::{ bench_runtime, get_runtime_benchmark_groups, prepare_runtime_benchmark_suite, - runtime_benchmark_dir, BenchmarkFilter, BenchmarkSuite, BenchmarkSuiteCompilation, - CargoIsolationMode, RuntimeProfiler, DEFAULT_RUNTIME_ITERATIONS, + runtime_benchmark_dir, BenchmarkSuite, BenchmarkSuiteCompilation, CargoIsolationMode, + RuntimeBenchmarkFilter, RuntimeProfiler, DEFAULT_RUNTIME_ITERATIONS, }; use collector::runtime::{profile_runtime, RuntimeCompilationOpts}; use collector::toolchain::{ @@ -105,12 +105,12 @@ struct CompileBenchmarkConfig { struct RuntimeBenchmarkConfig { runtime_suite: BenchmarkSuite, - filter: BenchmarkFilter, + filter: RuntimeBenchmarkFilter, iterations: u32, } impl RuntimeBenchmarkConfig { - fn new(suite: BenchmarkSuite, filter: BenchmarkFilter, iterations: u32) -> Self { + fn new(suite: BenchmarkSuite, filter: RuntimeBenchmarkFilter, iterations: u32) -> Self { Self { runtime_suite: suite.filter(&filter), filter, @@ -761,7 +761,7 @@ fn main_result() -> anyhow::Result { }; let config = RuntimeBenchmarkConfig::new( runtime_suite, - BenchmarkFilter::new(local.exclude, local.include), + RuntimeBenchmarkFilter::new(local.exclude, local.include), iterations, ); run_benchmarks(&mut rt, conn, shared, None, Some(config))?; @@ -1042,7 +1042,7 @@ fn main_result() -> anyhow::Result { let runtime_config = RuntimeBenchmarkConfig { runtime_suite, - filter: BenchmarkFilter::keep_all(), + filter: RuntimeBenchmarkFilter::keep_all(), iterations: DEFAULT_RUNTIME_ITERATIONS, }; let shared = SharedBenchmarkConfig { @@ -1745,7 +1745,7 @@ fn bench_published_artifact( }), Some(RuntimeBenchmarkConfig::new( runtime_suite, - BenchmarkFilter::keep_all(), + RuntimeBenchmarkFilter::keep_all(), DEFAULT_RUNTIME_ITERATIONS, )), ) diff --git a/collector/src/runtime/benchmark.rs b/collector/src/runtime/benchmark.rs index 3568b297d..7fc83a8d7 100644 --- a/collector/src/runtime/benchmark.rs +++ b/collector/src/runtime/benchmark.rs @@ -42,7 +42,7 @@ pub struct BenchmarkSuite { impl BenchmarkSuite { /// Returns a new suite containing only groups that contains at least a single benchmark /// that matches the filter. - pub fn filter(self, filter: &BenchmarkFilter) -> Self { + pub fn filter(self, filter: &RuntimeBenchmarkFilter) -> Self { let BenchmarkSuite { toolchain, groups, @@ -64,7 +64,7 @@ impl BenchmarkSuite { } } - pub fn filtered_benchmark_count(&self, filter: &BenchmarkFilter) -> u64 { + pub fn filtered_benchmark_count(&self, filter: &RuntimeBenchmarkFilter) -> u64 { self.benchmark_names() .filter(|benchmark| passes_filter(benchmark, &filter.exclude, &filter.include)) .count() as u64 @@ -86,12 +86,12 @@ impl BenchmarkSuite { } } -pub struct BenchmarkFilter { +pub struct RuntimeBenchmarkFilter { pub exclude: Vec, pub include: Vec, } -impl BenchmarkFilter { +impl RuntimeBenchmarkFilter { pub fn keep_all() -> Self { Self { exclude: vec![], diff --git a/collector/src/runtime/mod.rs b/collector/src/runtime/mod.rs index bfced0570..a18d04afe 100644 --- a/collector/src/runtime/mod.rs +++ b/collector/src/runtime/mod.rs @@ -9,8 +9,8 @@ use thousands::Separable; use benchlib::comm::messages::{BenchmarkMessage, BenchmarkResult, BenchmarkStats}; pub use benchmark::{ get_runtime_benchmark_groups, prepare_runtime_benchmark_suite, runtime_benchmark_dir, - BenchmarkFilter, BenchmarkGroup, BenchmarkGroupCrate, BenchmarkSuite, - BenchmarkSuiteCompilation, CargoIsolationMode, + BenchmarkGroup, BenchmarkGroupCrate, BenchmarkSuite, BenchmarkSuiteCompilation, + CargoIsolationMode, RuntimeBenchmarkFilter, }; use database::{ArtifactIdNumber, CollectionId, Connection}; @@ -33,7 +33,7 @@ pub async fn bench_runtime( conn: &mut dyn Connection, suite: BenchmarkSuite, collector: &CollectorCtx, - filter: BenchmarkFilter, + filter: RuntimeBenchmarkFilter, iterations: u32, ) -> anyhow::Result<()> { let filtered = suite.filtered_benchmark_count(&filter); @@ -203,7 +203,7 @@ async fn record_stats( /// a set of runtime benchmarks and print `BenchmarkMessage`s encoded as JSON, one per line. fn execute_runtime_benchmark_binary( binary: &Path, - filter: &BenchmarkFilter, + filter: &RuntimeBenchmarkFilter, iterations: u32, ) -> anyhow::Result>> { let mut command = prepare_command(binary); From 25108f38ce7d7a5533ac4fa2b924e1868bd3478b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 27 May 2025 11:12:44 +0200 Subject: [PATCH 2/2] Add `--exact-match` CLI argument for exact matching benchmark names To avoid problems with prefixes/suffixes in programmatic usage of the collector. --- collector/README.md | 3 ++ collector/src/bin/collector.rs | 61 ++++++++++++++--------- collector/src/compile/benchmark/mod.rs | 68 ++++++++++++++++++++------ 3 files changed, 95 insertions(+), 37 deletions(-) diff --git a/collector/README.md b/collector/README.md index df3231b5a..61c79aba3 100644 --- a/collector/README.md +++ b/collector/README.md @@ -135,6 +135,9 @@ The following options alter the behaviour of the `bench_local` subcommand. dedicated to artifact sizes (ending with `-tiny`). - `--id ` the identifier that will be used to identify the results in the database. +- `--exact-match `: comma-separated list of benchmark names that should be + executed. The names have to match exactly. Cannot be combined with + `--include`/`--exclude`/`--exclude-suffix`. - `--include `: the inverse of `--exclude`. The argument is a comma-separated list of benchmark prefixes. When this option is specified, a benchmark is included in the run only if its name matches one of the given diff --git a/collector/src/bin/collector.rs b/collector/src/bin/collector.rs index c5e310743..a91186dcd 100644 --- a/collector/src/bin/collector.rs +++ b/collector/src/bin/collector.rs @@ -40,6 +40,7 @@ use collector::compile::benchmark::scenario::Scenario; use collector::compile::benchmark::target::Target; use collector::compile::benchmark::{ compile_benchmark_dir, get_compile_benchmarks, ArtifactType, Benchmark, BenchmarkName, + CompileBenchmarkFilter, }; use collector::compile::execute::bencher::BenchProcessor; use collector::compile::execute::profiler::{ProfileProcessor, Profiler}; @@ -337,6 +338,16 @@ struct LocalOptions { #[arg(long, value_delimiter = ',')] include: Vec, + /// Include only benchmarks in this comma-separated list + #[arg( + long, + value_delimiter = ',', + conflicts_with("include"), + conflicts_with("exclude"), + conflicts_with("exclude_suffix") + )] + exact_match: Vec, + /// Include only benchmarks belonging to the given categories. #[arg(long, value_parser = EnumArgParser::::default(), default_value = "Primary,Secondary")] category: MultiEnumValue, @@ -688,6 +699,25 @@ enum DownloadSubcommand { }, } +impl<'a> From<&'a LocalOptions> for CompileBenchmarkFilter<'a> { + fn from(value: &'a LocalOptions) -> Self { + if !value.exact_match.is_empty() { + Self::Exact(&value.exact_match) + } else if !value.include.is_empty() + || !value.exclude.is_empty() + || !value.exclude_suffix.is_empty() + { + Self::Fuzzy { + include: &value.include, + exclude: &value.exclude, + exclude_suffix: &value.exclude_suffix, + } + } else { + Self::All + } + } +} + fn main_result() -> anyhow::Result { env_logger::init(); @@ -884,12 +914,7 @@ fn main_result() -> anyhow::Result { target_triple, )?; - let mut benchmarks = get_compile_benchmarks( - &compile_benchmark_dir, - &local.include, - &local.exclude, - &local.exclude_suffix, - )?; + let mut benchmarks = get_compile_benchmarks(&compile_benchmark_dir, (&local).into())?; benchmarks.retain(|b| local.category.0.contains(&b.category())); let artifact_id = ArtifactId::Commit(Commit { @@ -1006,9 +1031,11 @@ fn main_result() -> anyhow::Result { let mut benchmarks = get_compile_benchmarks( &compile_benchmark_dir, - &split_args(include), - &split_args(exclude), - &[], + CompileBenchmarkFilter::Fuzzy { + include: &split_args(include), + exclude: &split_args(exclude), + exclude_suffix: &[], + }, )?; benchmarks.retain(|b| b.category().is_primary_or_secondary()); @@ -1102,12 +1129,7 @@ fn main_result() -> anyhow::Result { let scenarios = &opts.scenarios.0; let backends = &opts.codegen_backends.0; - let mut benchmarks = get_compile_benchmarks( - &compile_benchmark_dir, - &local.include, - &local.exclude, - &local.exclude_suffix, - )?; + let mut benchmarks = get_compile_benchmarks(&compile_benchmark_dir, (&local).into())?; benchmarks.retain(|b| local.category.0.contains(&b.category())); let mut errors = BenchmarkErrors::new(); @@ -1315,12 +1337,7 @@ fn binary_stats_compile( Profile::Opt => CargoProfile::Release, _ => return Err(anyhow::anyhow!("Only Debug and Opt profiles are supported")), }; - let benchmarks = get_compile_benchmarks( - &compile_benchmark_dir(), - &local.include, - &local.exclude, - &local.exclude_suffix, - )?; + let benchmarks = get_compile_benchmarks(&compile_benchmark_dir(), (&local).into())?; for benchmark in benchmarks { println!("Stats for benchmark `{}`", benchmark.name); println!("{}", "-".repeat(20)); @@ -1713,7 +1730,7 @@ fn bench_published_artifact( }; // Exclude benchmarks that don't work with a stable compiler. - let mut compile_benchmarks = get_compile_benchmarks(dirs.compile, &[], &[], &[])?; + let mut compile_benchmarks = get_compile_benchmarks(dirs.compile, CompileBenchmarkFilter::All)?; compile_benchmarks.retain(|b| b.category().is_stable()); let runtime_suite = rt.block_on(load_runtime_benchmarks( diff --git a/collector/src/compile/benchmark/mod.rs b/collector/src/compile/benchmark/mod.rs index 80cc1c5d3..2deb05883 100644 --- a/collector/src/compile/benchmark/mod.rs +++ b/collector/src/compile/benchmark/mod.rs @@ -466,14 +466,22 @@ pub fn compile_benchmark_dir() -> PathBuf { PathBuf::from("collector/compile-benchmarks") } +pub enum CompileBenchmarkFilter<'a> { + All, + /// Select benchmarks exactly matching the given benchmark names. + Exact(&'a [String]), + /// Select benchmarks matching the given prefixes/suffixes. + Fuzzy { + include: &'a [String], + exclude: &'a [String], + exclude_suffix: &'a [String], + }, +} + pub fn get_compile_benchmarks( benchmark_dir: &Path, - include: &[String], - exclude: &[String], - exclude_suffix: &[String], + filter: CompileBenchmarkFilter<'_>, ) -> anyhow::Result> { - let mut benchmarks = Vec::new(); - let mut paths = Vec::new(); for entry in std::fs::read_dir(benchmark_dir) .with_context(|| format!("failed to list benchmark dir '{}'", benchmark_dir.display()))? @@ -493,6 +501,40 @@ pub fn get_compile_benchmarks( paths.push((path, name)); } + let mut benchmarks = match filter { + CompileBenchmarkFilter::All => paths + .into_iter() + .map(|(path, name)| Benchmark::new(name, path)) + .collect::>>()?, + CompileBenchmarkFilter::Exact(names) => paths + .into_iter() + .filter(|(_, name)| names.contains(name)) + .map(|(path, name)| Benchmark::new(name, path)) + .collect::>>()?, + CompileBenchmarkFilter::Fuzzy { + include, + exclude, + exclude_suffix, + } => select_benchmarks_fuzzy(paths, include, exclude, exclude_suffix)?, + }; + + benchmarks.sort_by_key(|benchmark| benchmark.name.clone()); + + if benchmarks.is_empty() { + eprintln!("Warning: no benchmarks selected! Try less strict filters."); + } + + Ok(benchmarks) +} + +fn select_benchmarks_fuzzy( + paths: Vec<(PathBuf, String)>, + include: &[String], + exclude: &[String], + exclude_suffix: &[String], +) -> anyhow::Result> { + let mut benchmarks = Vec::new(); + // For each --include/--exclude entry, we count how many times it's used, // to enable `check_for_unused` below. fn to_hashmap(xyz: &[String]) -> HashMap<&str, usize> { @@ -551,12 +593,6 @@ Expected zero or more entries or substrings from list: {:?}."#, check_for_unused("exclude", excludes)?; check_for_unused("exclude-suffix", exclude_suffixes)?; - benchmarks.sort_by_key(|benchmark| benchmark.name.clone()); - - if benchmarks.is_empty() { - eprintln!("Warning: no benchmarks selected! Try less strict filters."); - } - Ok(benchmarks) } @@ -578,7 +614,7 @@ fn substring_matches( #[cfg(test)] mod tests { - use crate::compile::benchmark::get_compile_benchmarks; + use crate::compile::benchmark::{get_compile_benchmarks, CompileBenchmarkFilter}; use std::path::Path; #[test] @@ -586,9 +622,11 @@ mod tests { // Check that we can deserialize all perf-config.json files in the compile benchmark // directory. let root = env!("CARGO_MANIFEST_DIR"); - let benchmarks = - get_compile_benchmarks(&Path::new(root).join("compile-benchmarks"), &[], &[], &[]) - .unwrap(); + let benchmarks = get_compile_benchmarks( + &Path::new(root).join("compile-benchmarks"), + CompileBenchmarkFilter::All, + ) + .unwrap(); assert!(!benchmarks.is_empty()); } }