Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions crates/codspeed/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,37 @@ 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::<Vec<_>>()
.join("::")
}

/// Resolves the caller's source file path for URI generation, anchored to the
/// workspace root when running under the CodSpeed runner.
///
/// `#[track_caller]` so the location resolves to the original benchmark call site
/// rather than this helper.
#[track_caller]
pub fn caller_file_path() -> String {
let caller = std::panic::Location::caller();
match std::env::var("CODSPEED_CARGO_WORKSPACE_ROOT") {
Ok(workspace_root) => PathBuf::from(workspace_root)
.join(caller.file())
.to_string_lossy()
.into_owned(),
Err(_) => caller.file().to_string(),
}
}

/// Fixes spaces around `::` created by stringify!($function).
pub fn get_formated_function_path(function_path: impl Into<String>) -> String {
let function_path = function_path.into();
Expand Down Expand Up @@ -111,6 +142,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";
Expand Down
4 changes: 4 additions & 0 deletions crates/criterion_compat/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,7 @@ harness = false
[[bench]]
name = "test_benches"
harness = false

[[bench]]
name = "custom_main"
harness = false
42 changes: 42 additions & 0 deletions crates/criterion_compat/benches/custom_main.rs
Original file line number Diff line number Diff line change
@@ -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);
Comment thread
not-matthias marked this conversation as resolved.
}

#[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);
}
9 changes: 4 additions & 5 deletions crates/criterion_compat/criterion_fork/src/analysis/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
16 changes: 16 additions & 0 deletions crates/criterion_compat/criterion_fork/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,18 @@ mod codspeed {
pub fn set_macro_group(&mut self, macro_group: impl Into<String>) {
self.macro_group = macro_group.into();
}

/// Ensures `current_file` is set, for CodSpeed URI generation.
///
/// When set explicitly via `set_current_file`, it is kept as-is; otherwise it is
/// derived from the caller location.
#[track_caller]
pub(crate) fn ensure_current_file(&mut self) {
if !self.current_file.is_empty() {
return;
}
self.current_file = codspeed::utils::caller_file_path();
}
}
}

Expand Down Expand Up @@ -1201,7 +1213,9 @@ https://bheisler.github.io/criterion.rs/book/faq.html
/// ```
/// # Panics:
/// Panics if the group name is empty
#[track_caller]
pub fn benchmark_group<S: Into<String>>(&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.");

Expand Down Expand Up @@ -1238,6 +1252,7 @@ where
/// criterion_group!(benches, bench);
/// criterion_main!(benches);
/// ```
#[track_caller]
pub fn bench_function<F>(&mut self, id: &str, f: F) -> &mut Criterion<M>
where
F: FnMut(&mut Bencher<'_, M>),
Expand Down Expand Up @@ -1270,6 +1285,7 @@ where
/// criterion_group!(benches, bench);
/// criterion_main!(benches);
/// ```
#[track_caller]
pub fn bench_with_input<F, I>(&mut self, id: BenchmarkId, input: &I, f: F) -> &mut Criterion<M>
where
F: FnMut(&mut Bencher<'_, M>, &I),
Expand Down
16 changes: 16 additions & 0 deletions crates/criterion_compat/src/compat/criterion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ impl<M: Measurement> Criterion<M> {
self.macro_group = macro_group.into();
}

#[track_caller]
pub fn bench_function<F>(&mut self, id: &str, f: F) -> &mut Criterion<M>
where
F: FnMut(&mut Bencher),
Expand All @@ -101,6 +102,7 @@ impl<M: Measurement> Criterion<M> {
self
}

#[track_caller]
pub fn bench_with_input<F, I>(&mut self, id: BenchmarkId, input: &I, f: F) -> &mut Criterion<M>
where
F: FnMut(&mut Bencher, &I),
Expand All @@ -118,9 +120,23 @@ impl<M: Measurement> Criterion<M> {
self
}

#[track_caller]
pub fn benchmark_group<S: Into<String>>(&mut self, group_name: S) -> BenchmarkGroup<M> {
self.ensure_current_file();
BenchmarkGroup::<M>::new(self, group_name.into())
}

/// Ensures `current_file` is set, for CodSpeed URI generation.
///
/// When set explicitly via `set_current_file`, it is kept as-is; otherwise it is
/// derived from the caller location.
#[track_caller]
fn ensure_current_file(&mut self) {
if !self.current_file.is_empty() {
return;
}
self.current_file = codspeed::utils::caller_file_path();
}
Comment thread
not-matthias marked this conversation as resolved.
}

// Dummy methods
Expand Down
16 changes: 9 additions & 7 deletions crates/criterion_compat/src/compat/group.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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}");
}
Expand Down
Loading