diff --git a/crates/codspeed/src/utils.rs b/crates/codspeed/src/utils.rs index 472c1f84..4dd06fde 100644 --- a/crates/codspeed/src/utils.rs +++ b/crates/codspeed/src/utils.rs @@ -32,6 +32,20 @@ where } } +/// Builds a benchmark URI by joining the non-empty segments with `::`. +/// +/// Segments can be empty when `criterion_group!`/`criterion_main!` are bypassed (custom main): +/// `current_file` and `macro_group` are not set in that case and must not produce empty +/// `::` parts (e.g. `::::my_group::my_bench`). +pub fn build_uri(segments: &[&str]) -> String { + segments + .iter() + .filter(|s| !s.is_empty()) + .copied() + .collect::>() + .join("::") +} + /// Fixes spaces around `::` created by stringify!($function). pub fn get_formated_function_path(function_path: impl Into) -> String { let function_path = function_path.into(); @@ -111,6 +125,28 @@ mod tests { assert_eq!(relative_path, symlink.canonicalize().unwrap()); } + /// COD-2324: empty segments (no file/macro group when `criterion_group!` is bypassed) + /// must not produce URIs like `::::my_group::my_bench`. + #[test] + fn test_build_uri_skips_empty_segments() { + assert_eq!( + build_uri(&["", "", "my_group::my_bench"]), + "my_group::my_bench" + ); + assert_eq!( + build_uri(&["benches/custom_main.rs", "", "my_group::my_bench"]), + "benches/custom_main.rs::my_group::my_bench" + ); + assert_eq!( + build_uri(&[ + "benches/bench.rs", + "benches::bench_fn", + "my_group::my_bench" + ]), + "benches/bench.rs::benches::bench_fn::my_group::my_bench" + ); + } + #[test] fn test_get_formated_function_path() { let input = "std :: vec :: Vec :: new"; diff --git a/crates/criterion_compat/Cargo.toml b/crates/criterion_compat/Cargo.toml index e34315d6..dd1d710c 100644 --- a/crates/criterion_compat/Cargo.toml +++ b/crates/criterion_compat/Cargo.toml @@ -68,3 +68,7 @@ harness = false [[bench]] name = "test_benches" harness = false + +[[bench]] +name = "custom_main" +harness = false diff --git a/crates/criterion_compat/benches/custom_main.rs b/crates/criterion_compat/benches/custom_main.rs new file mode 100644 index 00000000..24db79b4 --- /dev/null +++ b/crates/criterion_compat/benches/custom_main.rs @@ -0,0 +1,42 @@ +/// Test bench for custom main patterns (COD-2324). +/// Verifies URI formatting when users bypass criterion_group!/criterion_main!. +use codspeed_criterion_compat::{criterion_group, BenchmarkId, Criterion}; + +fn bench_with_group(c: &mut Criterion) { + let mut group = c.benchmark_group("my_group"); + group.bench_function("my_bench", |b| b.iter(|| 2 + 2)); + group.bench_function(BenchmarkId::new("parameterized", 42), |b| b.iter(|| 2 + 2)); + group.finish(); +} + +criterion_group!(benches, bench_with_group); + +#[cfg(codspeed)] +fn main() { + // Pattern A: Using new_instrumented() but calling bench functions directly (not through criterion_group!) + // Since criterion_group! is bypassed, there is no macro context (group/function name), + // so the URI only contains the file, benchmark group, and benchmark name. + // + // Expected URIs: + // - crates/criterion_compat/benches/custom_main.rs::my_group::my_bench + // - crates/criterion_compat/benches/custom_main.rs::my_group::parameterized[42] + let mut criterion = Criterion::new_instrumented(); + bench_with_group(&mut criterion); + + // Pattern B: Calling through criterion_group!-generated function (should work correctly) + // The macro captures the criterion_group! name (`benches`) and the bench function name + // (`bench_with_group`), so both are part of the URI. + // + // Expected URIs: + // - crates/criterion_compat/benches/custom_main.rs::benches::bench_with_group::my_group::my_bench + // - crates/criterion_compat/benches/custom_main.rs::benches::bench_with_group::my_group::parameterized[42] + let mut criterion2 = Criterion::new_instrumented(); + benches(&mut criterion2); +} + +#[cfg(not(codspeed))] +fn main() { + // Without codspeed, just run through upstream criterion directly + let mut criterion = Criterion::default().configure_from_args(); + bench_with_group(&mut criterion); +} diff --git a/crates/criterion_compat/criterion_fork/src/analysis/mod.rs b/crates/criterion_compat/criterion_fork/src/analysis/mod.rs index 979a0106..5b82df80 100644 --- a/crates/criterion_compat/criterion_fork/src/analysis/mod.rs +++ b/crates/criterion_compat/criterion_fork/src/analysis/mod.rs @@ -279,12 +279,11 @@ mod codspeed { } let git_relative_file_path = codspeed::utils::get_git_relative_path(&c.current_file); - let uri = format!( - "{}::{}::{}", - git_relative_file_path.to_string_lossy(), + let uri = codspeed::utils::build_uri(&[ + &git_relative_file_path.to_string_lossy(), &c.macro_group, - bench_name - ); + &bench_name, + ]); (uri, bench_name) } diff --git a/crates/criterion_compat/criterion_fork/src/lib.rs b/crates/criterion_compat/criterion_fork/src/lib.rs index 9a7a4ebc..81c9cfe8 100644 --- a/crates/criterion_compat/criterion_fork/src/lib.rs +++ b/crates/criterion_compat/criterion_fork/src/lib.rs @@ -398,6 +398,25 @@ mod codspeed { pub fn set_macro_group(&mut self, macro_group: impl Into) { self.macro_group = macro_group.into(); } + + /// Ensures `current_file` is set, for CodSpeed URI generation. + /// + /// `criterion_group!` sets it explicitly via `set_current_file`. When the macro is + /// bypassed (custom main, COD-2324), derive it from the caller location instead. + #[track_caller] + pub(crate) fn ensure_current_file(&mut self) { + if !self.current_file.is_empty() { + return; + } + let caller = std::panic::Location::caller(); + self.current_file = match std::env::var("CODSPEED_CARGO_WORKSPACE_ROOT") { + Ok(workspace_root) => std::path::PathBuf::from(workspace_root) + .join(caller.file()) + .to_string_lossy() + .into_owned(), + Err(_) => caller.file().to_string(), + }; + } } } @@ -1201,7 +1220,9 @@ https://bheisler.github.io/criterion.rs/book/faq.html /// ``` /// # Panics: /// Panics if the group name is empty + #[track_caller] pub fn benchmark_group>(&mut self, group_name: S) -> BenchmarkGroup<'_, M> { + self.ensure_current_file(); let group_name = group_name.into(); assert!(!group_name.is_empty(), "Group name must not be empty."); @@ -1238,6 +1259,7 @@ where /// criterion_group!(benches, bench); /// criterion_main!(benches); /// ``` + #[track_caller] pub fn bench_function(&mut self, id: &str, f: F) -> &mut Criterion where F: FnMut(&mut Bencher<'_, M>), @@ -1270,6 +1292,7 @@ where /// criterion_group!(benches, bench); /// criterion_main!(benches); /// ``` + #[track_caller] pub fn bench_with_input(&mut self, id: BenchmarkId, input: &I, f: F) -> &mut Criterion where F: FnMut(&mut Bencher<'_, M>, &I), diff --git a/crates/criterion_compat/src/compat/criterion.rs b/crates/criterion_compat/src/compat/criterion.rs index a8b85c1a..51e14472 100644 --- a/crates/criterion_compat/src/compat/criterion.rs +++ b/crates/criterion_compat/src/compat/criterion.rs @@ -92,6 +92,7 @@ impl Criterion { self.macro_group = macro_group.into(); } + #[track_caller] pub fn bench_function(&mut self, id: &str, f: F) -> &mut Criterion where F: FnMut(&mut Bencher), @@ -101,6 +102,7 @@ impl Criterion { self } + #[track_caller] pub fn bench_with_input(&mut self, id: BenchmarkId, input: &I, f: F) -> &mut Criterion where F: FnMut(&mut Bencher, &I), @@ -118,9 +120,30 @@ impl Criterion { self } + #[track_caller] pub fn benchmark_group>(&mut self, group_name: S) -> BenchmarkGroup { + self.ensure_current_file(); BenchmarkGroup::::new(self, group_name.into()) } + + /// Ensures `current_file` is set, for CodSpeed URI generation. + /// + /// `criterion_group!` sets it explicitly via `set_current_file`. When the macro is + /// bypassed (custom main, COD-2324), derive it from the caller location instead. + #[track_caller] + fn ensure_current_file(&mut self) { + if !self.current_file.is_empty() { + return; + } + let caller = std::panic::Location::caller(); + self.current_file = match std::env::var("CODSPEED_CARGO_WORKSPACE_ROOT") { + Ok(workspace_root) => std::path::PathBuf::from(workspace_root) + .join(caller.file()) + .to_string_lossy() + .into_owned(), + Err(_) => caller.file().to_string(), + }; + } } // Dummy methods diff --git a/crates/criterion_compat/src/compat/group.rs b/crates/criterion_compat/src/compat/group.rs index 90e52877..0ab50fb4 100644 --- a/crates/criterion_compat/src/compat/group.rs +++ b/crates/criterion_compat/src/compat/group.rs @@ -1,7 +1,10 @@ use std::marker::PhantomData; use std::{cell::RefCell, rc::Rc, time::Duration}; -use codspeed::{codspeed::CodSpeed, utils::get_git_relative_path}; +use codspeed::{ + codspeed::CodSpeed, + utils::{build_uri, get_git_relative_path}, +}; use criterion::measurement::WallTime; use criterion::{measurement::Measurement, PlotConfiguration, SamplingMode, Throughput}; @@ -63,12 +66,11 @@ impl<'a, M: Measurement> BenchmarkGroup<'a, M> { I: ?Sized, { let git_relative_file_path = get_git_relative_path(&self.current_file); - let mut uri = format!( - "{}::{}::{}", - git_relative_file_path.to_string_lossy(), - self.macro_group, - self.group_name, - ); + let mut uri = build_uri(&[ + &git_relative_file_path.to_string_lossy(), + &self.macro_group, + &self.group_name, + ]); if let Some(function_name) = id.function_name { uri = format!("{uri}::{function_name}"); }