From 3ec23f2c403d824d3202d7352f3718e4d8d421a3 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Thu, 6 Feb 2025 16:23:35 +0100 Subject: [PATCH 1/8] ci: add divan to instrumented integration test --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6a63b09f..e36a69a1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,6 +52,9 @@ jobs: - run: cargo codspeed build -p codspeed-bencher-compat - run: cargo codspeed build --features async_futures -p codspeed-criterion-compat + - run: cargo codspeed build -p codspeed-divan-compat + - run: cargo codspeed build -p codspeed-divan-compat-examples + - name: Run the benchmarks uses: CodSpeedHQ/action@main with: @@ -69,6 +72,7 @@ jobs: - run: cargo install --path crates/cargo-codspeed --locked - run: cargo codspeed build -p codspeed-divan-compat + - run: cargo codspeed build -p codspeed-divan-compat-examples - name: Run the benchmarks uses: CodSpeedHQ/action@main From 9d2067c8063cae84485aab6365e7498346d4f968 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Thu, 6 Feb 2025 18:38:06 +0100 Subject: [PATCH 2/8] feat(divan_compat): share instance of codspeed helper across benches --- crates/divan_compat/src/compat/bench/mod.rs | 10 +++++----- crates/divan_compat/src/compat/mod.rs | 8 ++++++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/divan_compat/src/compat/bench/mod.rs b/crates/divan_compat/src/compat/bench/mod.rs index af38c874..9931587c 100644 --- a/crates/divan_compat/src/compat/bench/mod.rs +++ b/crates/divan_compat/src/compat/bench/mod.rs @@ -11,7 +11,7 @@ pub use self::{ }; use codspeed::codspeed::CodSpeed; -use std::{cell::RefCell, rc::Rc}; +use std::cell::RefCell; /// Using this in place of `()` for `GenI` prevents `Bencher::with_inputs` from /// working with `()` unintentionally. @@ -23,18 +23,18 @@ pub struct BencherConfig { } pub struct Bencher<'a, 'b, C = BencherConfig> { - pub(crate) codspeed: Rc>, + pub(crate) codspeed: &'a RefCell, pub(crate) uri: String, pub(crate) config: C, - pub(crate) _marker: std::marker::PhantomData<&'a &'b ()>, + pub(crate) _marker: std::marker::PhantomData<&'b ()>, } #[allow(clippy::needless_lifetimes)] impl<'a, 'b> Bencher<'a, 'b> { - pub(crate) fn new(uri: String) -> Self { + pub(crate) fn new(codspeed: &'a RefCell, uri: String) -> Self { Self { - codspeed: Rc::new(RefCell::new(CodSpeed::new())), config: BencherConfig { gen_input: Unit }, + codspeed, uri, _marker: std::marker::PhantomData, } diff --git a/crates/divan_compat/src/compat/mod.rs b/crates/divan_compat/src/compat/mod.rs index 85a8ec39..08de2d53 100644 --- a/crates/divan_compat/src/compat/mod.rs +++ b/crates/divan_compat/src/compat/mod.rs @@ -14,7 +14,10 @@ mod entry; mod uri; mod util; +use std::{cell::RefCell, rc::Rc}; + pub use bench::*; +use codspeed::codspeed::CodSpeed; pub fn main() { // Outlined steps of original divan::main and their equivalent in codspeed instrumented mode @@ -31,6 +34,7 @@ pub fn main() { // filtering is managed by the `cargo-codspeed` wrappers before we reach this point. // 4. Scan the tree and execute benchmarks + let codspeed = Rc::new(RefCell::new(CodSpeed::new())); for entry in bench_entries.iter() { let entry_uri = uri::generate(entry.meta.display_name, &entry.meta); @@ -42,14 +46,14 @@ pub fn main() { } match entry.bench { entry::BenchEntryRunner::Plain(bench_fn) => { - bench_fn(bench::Bencher::new(entry_uri)); + bench_fn(bench::Bencher::new(&codspeed, entry_uri)); } entry::BenchEntryRunner::Args(bench_runner) => { let bench_runner = bench_runner(); for (arg_index, arg_name) in bench_runner.arg_names().iter().enumerate() { let entry_name_with_arg = format!("{}::{}", entry_uri, arg_name); - let bencher = bench::Bencher::new(entry_name_with_arg); + let bencher = bench::Bencher::new(&codspeed, entry_name_with_arg); bench_runner.bench(bencher, arg_index); } From 25351941da22e7ead22dba10a1e795bb9b635270 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Thu, 6 Feb 2025 16:31:03 +0100 Subject: [PATCH 3/8] feat(divan_compat): put args between brackets in bench URI --- crates/divan_compat/src/compat/mod.rs | 2 +- crates/divan_compat/src/compat/uri.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/divan_compat/src/compat/mod.rs b/crates/divan_compat/src/compat/mod.rs index 08de2d53..6523103c 100644 --- a/crates/divan_compat/src/compat/mod.rs +++ b/crates/divan_compat/src/compat/mod.rs @@ -52,7 +52,7 @@ pub fn main() { let bench_runner = bench_runner(); for (arg_index, arg_name) in bench_runner.arg_names().iter().enumerate() { - let entry_name_with_arg = format!("{}::{}", entry_uri, arg_name); + let entry_name_with_arg = uri::append_arg(&entry_uri, arg_name); let bencher = bench::Bencher::new(&codspeed, entry_name_with_arg); bench_runner.bench(bencher, arg_index); diff --git a/crates/divan_compat/src/compat/uri.rs b/crates/divan_compat/src/compat/uri.rs index 01a2c5cb..ddf5a0ba 100644 --- a/crates/divan_compat/src/compat/uri.rs +++ b/crates/divan_compat/src/compat/uri.rs @@ -17,3 +17,7 @@ pub(crate) fn generate( uri } + +pub(crate) fn append_arg(uri: &str, arg: &str) -> String { + format!("{uri}[{arg}]") +} From 7a849b9a3edb4e90b0737bd89f940c97bb9b9004 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Thu, 6 Feb 2025 18:00:35 +0100 Subject: [PATCH 4/8] feat(divan_fork): manage type and args in bench names --- crates/divan_compat/divan_fork/src/divan.rs | 90 +++++++++++++++++---- 1 file changed, 74 insertions(+), 16 deletions(-) diff --git a/crates/divan_compat/divan_fork/src/divan.rs b/crates/divan_compat/divan_fork/src/divan.rs index 291288fb..6eb79a9e 100644 --- a/crates/divan_compat/divan_fork/src/divan.rs +++ b/crates/divan_compat/divan_fork/src/divan.rs @@ -311,21 +311,9 @@ impl Divan { if should_compute_stats { let stats = bench_context.compute_stats(); { - // WARNING: Keep in sync with `codspeed-divan-compat::uri::generate` - // Not worth doing the work of actually using the same code since this fork - // is temporary - let name = bench_entry.display_name().to_string(); - let file = bench_entry.meta().location.file; - let mut module_path = bench_entry - .meta() - .module_path_components() - .skip(1) - .collect::>() - .join("::"); - if !module_path.is_empty() { - module_path.push_str("::"); - } - let uri = format!("{file}::{module_path}{name}"); + let CodspeedBench { bench_name, uri } = + generate_codspeed_bench_name(&bench_entry, bench_display_name); + let iter_per_round = bench_context.samples.sample_size; let times_ns: Vec<_> = bench_context .samples @@ -336,7 +324,7 @@ impl Divan { let max_time_ns = options.max_time.map(|t| t.as_nanos()); ::codspeed::walltime::collect_raw_walltime_results( "divan", - name, + bench_name, uri, iter_per_round, max_time_ns, @@ -383,6 +371,76 @@ impl Divan { } } +struct CodspeedBench { + bench_name: String, + uri: String, +} + +/// Generates a Codspeed benchmark name and URI +/// +/// WARNING: Keep in sync with `codspeed-divan-compat::uri::generate` +/// Not worth doing the work of actually using the same code since this fork +/// is temporary +fn generate_codspeed_bench_name( + bench_entry: &AnyBenchEntry, + closure_bench_display_name: &str, +) -> CodspeedBench { + let bench_function_name = bench_entry.meta().display_name; + + let (bench_type_name, bench_arg_name) = { + let bench_function_or_type_name = bench_entry.display_name().to_string(); + + let type_name = if bench_function_or_type_name == bench_function_name { + None + } else { + Some(bench_function_or_type_name) + }; + + let arg_name = match type_name.as_ref() { + None => { + if closure_bench_display_name == bench_function_name { + None + } else { + Some(closure_bench_display_name) + } + } + Some(type_name) => { + if closure_bench_display_name == type_name { + None + } else { + Some(closure_bench_display_name) + } + } + }; + + (type_name, arg_name) + }; + + let mut bench_name = bench_function_name.to_string(); + + match (bench_type_name, bench_arg_name) { + (None, None) => {} + (Some(type_name), None) => { + bench_name.push_str(format!("[{type_name}]").as_str()); + } + (None, Some(arg_name)) => { + bench_name.push_str(format!("[{arg_name}]").as_str()); + } + (Some(type_name), Some(arg_name)) => { + bench_name.push_str(format!("[{type_name}, {arg_name}]").as_str()); + } + } + + let file = bench_entry.meta().location.file; + let mut module_path = + bench_entry.meta().module_path_components().skip(1).collect::>().join("::"); + if !module_path.is_empty() { + module_path.push_str("::"); + } + let uri = format!("{file}::{module_path}{bench_name}"); + CodspeedBench { bench_name, uri } +} + /// Makes `Divan::skip_regex` input polymorphic. pub trait SkipRegex { fn skip_regex(self, divan: &mut Divan); From d1130a4b06016c1037457a4b8c6451095d958dd3 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 10 Feb 2025 14:03:28 +0100 Subject: [PATCH 5/8] feat(divan_compat_macro): parse types argument --- Cargo.lock | 7 +- Cargo.toml | 1 + crates/cargo-codspeed/Cargo.toml | 2 +- crates/divan_compat/examples/benches/math.rs | 2 +- crates/divan_compat/macros/Cargo.toml | 2 + crates/divan_compat/macros/src/args.rs | 107 +++++++++++++++++++ crates/divan_compat/macros/src/lib.rs | 83 +++++--------- 7 files changed, 143 insertions(+), 61 deletions(-) create mode 100644 crates/divan_compat/macros/src/args.rs diff --git a/Cargo.lock b/Cargo.lock index 15502d82..0fb3221b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -461,7 +461,7 @@ dependencies = [ "codspeed", "fs_extra", "glob", - "itertools 0.13.0", + "itertools 0.14.0", "predicates", "serde", "serde_json", @@ -634,6 +634,7 @@ name = "codspeed-divan-compat-macros" version = "2.8.0-alpha.2" dependencies = [ "divan-macros", + "itertools 0.14.0", "proc-macro-crate", "proc-macro2", "quote", @@ -1123,9 +1124,9 @@ dependencies = [ [[package]] name = "itertools" -version = "0.13.0" +version = "0.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "413ee7dfc52ee1a4949ceeb7dbc8a33f2d6c088194d9f922fb8318faf1f01186" +checksum = "2b192c782037fadd9cfa75548310488aabdbf3d2da73885b31bd0abd03351285" dependencies = [ "either", ] diff --git a/Cargo.toml b/Cargo.toml index f5b9cd50..6b907974 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,5 +13,6 @@ members = [ resolver = "2" [workspace.dependencies] +itertools = "0.14.0" serde = { version = "1.0.217", features = ["derive"] } serde_json = "1.0.138" diff --git a/crates/cargo-codspeed/Cargo.toml b/crates/cargo-codspeed/Cargo.toml index 54fb9283..5b5d83d2 100644 --- a/crates/cargo-codspeed/Cargo.toml +++ b/crates/cargo-codspeed/Cargo.toml @@ -22,7 +22,7 @@ cargo_metadata = "0.19.1" clap = { version = "=4.5.17", features = ["derive", "env"] } termcolor = "1.4" anyhow = "1.0.86" -itertools = "0.13.0" +itertools = { workspace = true } anstyle = "1.0.8" serde = { workspace = true } serde_json = { workspace = true } diff --git a/crates/divan_compat/examples/benches/math.rs b/crates/divan_compat/examples/benches/math.rs index 6015e9d8..7a802497 100644 --- a/crates/divan_compat/examples/benches/math.rs +++ b/crates/divan_compat/examples/benches/math.rs @@ -17,7 +17,7 @@ fn sub() -> i32 { black_box(2) - black_box(1) } -#[divan::bench] +#[divan::bench(max_time = 1)] fn mul() -> i32 { black_box(2) * black_box(1) } diff --git a/crates/divan_compat/macros/Cargo.toml b/crates/divan_compat/macros/Cargo.toml index 413ed2c8..f3abc6a2 100644 --- a/crates/divan_compat/macros/Cargo.toml +++ b/crates/divan_compat/macros/Cargo.toml @@ -22,6 +22,7 @@ proc-macro = true [dependencies] divan-macros = { version = "=0.1.17" } +itertools = { workspace = true } proc-macro-crate = "3.2.0" proc-macro2 = "1" quote = { version = "1", default-features = false } @@ -32,4 +33,5 @@ syn = { version = "^2.0.18", default-features = false, features = [ "parsing", "printing", "proc-macro", + "extra-traits", ] } diff --git a/crates/divan_compat/macros/src/args.rs b/crates/divan_compat/macros/src/args.rs new file mode 100644 index 00000000..da32978c --- /dev/null +++ b/crates/divan_compat/macros/src/args.rs @@ -0,0 +1,107 @@ +use itertools::Itertools; +use proc_macro::TokenStream; +use quote::{quote, ToTokens}; +use syn::{ + parse::{Parse, Parser}, + Expr, Meta, MetaNameValue, Token, Type, +}; + +/// Values from parsed options shared between `#[divan::bench]` and +/// `#[divan::bench_group]`. +/// +/// The `crate` option is not included because it is only needed to get proper +/// access to `__private`. +#[derive(Default)] +pub(crate) struct AttrOptions { + pub(crate) types: Option, + pub(crate) crate_: bool, + pub(crate) other_args: Vec, +} + +#[allow(unreachable_code)] +impl AttrOptions { + pub fn parse(tokens: TokenStream) -> Result { + let mut attr_options = Self::default(); + + let attr_parser = syn::meta::parser(|meta| { + let Some(ident) = meta.path.get_ident() else { + return Err(meta.error("Unexpected attribute")); + }; + + let ident_name = ident.to_string(); + let ident_name = ident_name.strip_prefix("r#").unwrap_or(&ident_name); + + match ident_name { + // Divan accepts type syntax that is not parseable into syn::Meta out of the box, + // so we parse and rebuild the arguments manually. + "types" => { + attr_options.types = Some(meta.value()?.parse()?); + } + "crate" => { + attr_options.crate_ = true; + meta.value()?.parse::()?; // Discard the value + } + "min_time" | "max_time" | "sample_size" | "sample_count" | "skip_ext_time" => { + // These arguments are ignored for codspeed runs + meta.value()?.parse::()?; // Discard the value + } + _ => { + let path = meta.path.clone(); + let parsed_meta = if meta.input.is_empty() { + Meta::Path(path) + } else { + let value: syn::Expr = meta.value()?.parse()?; + Meta::NameValue(MetaNameValue { + path, + eq_token: Default::default(), + value: Expr::Verbatim(value.into_token_stream()), + }) + }; + + attr_options.other_args.push(parsed_meta); + } + } + + Ok(()) + }); + + match attr_parser.parse(tokens) { + Ok(()) => {} + Err(error) => return Err(error.into_compile_error().into()), + } + + Ok(attr_options) + } +} + +/// Generic types over which to instantiate benchmark functions. +pub(crate) enum GenericTypes { + /// List of types, e.g. `[i32, String, ()]`. + List(Vec), +} + +impl Parse for GenericTypes { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + let content; + syn::bracketed!(content in input); + + Ok(Self::List( + content + .parse_terminated(Type::parse, Token![,])? + .into_iter() + .map(|ty| ty.into_token_stream()) + .collect(), + )) + } +} + +impl ToTokens for GenericTypes { + fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { + match self { + Self::List(list) => { + let type_tokens = list.iter().cloned().map_into::(); + tokens.extend(quote! { [ #(#type_tokens),* ] }); + } + } + } +} diff --git a/crates/divan_compat/macros/src/lib.rs b/crates/divan_compat/macros/src/lib.rs index e903e4c1..d8213504 100644 --- a/crates/divan_compat/macros/src/lib.rs +++ b/crates/divan_compat/macros/src/lib.rs @@ -1,65 +1,25 @@ +mod args; + +use args::AttrOptions; use proc_macro::TokenStream; use proc_macro_crate::{crate_name, FoundCrate}; -use quote::{format_ident, quote}; -use syn::{ - parse::Parse, - parse_macro_input, - punctuated::Punctuated, - ItemFn, - Meta::{self, NameValue}, - MetaNameValue, Token, -}; - -struct MyBenchArgs { - args: Punctuated, -} - -impl Parse for MyBenchArgs { - fn parse(input: syn::parse::ParseStream) -> syn::Result { - Ok(Self { - args: Punctuated::parse_terminated(input)?, - }) - } -} +use quote::{format_ident, quote, ToTokens}; +use syn::{parse_macro_input, Expr, ItemFn, Meta}; #[proc_macro_attribute] pub fn bench_compat(attr: TokenStream, item: TokenStream) -> TokenStream { - let parsed_args = parse_macro_input!(attr as MyBenchArgs); let input = parse_macro_input!(item as ItemFn); - let mut filtered_args = Vec::new(); - - for arg in parsed_args.args { - match &arg { - NameValue(MetaNameValue { path, .. }) => { - if path.is_ident("crate") { - return quote! { - compile_error!("`crate` argument is not supported with codspeed_divan_compat"); - }. - into(); - } - - if path.is_ident("types") { - return quote! { - compile_error!("`type` argument is not yet supported with codspeed_divan_compat"); - } - .into(); - } - - if path.is_ident("min_time") - || path.is_ident("max_time") - || path.is_ident("sample_size") - || path.is_ident("sample_count") - || path.is_ident("skip_ext_time") - { - // These arguments are ignored in instrumented mode - continue; - } + let attr_options = match AttrOptions::parse(attr) { + Ok(attr_options) => attr_options, + Err(error) => return error, + }; - filtered_args.push(arg); - } - _ => filtered_args.push(arg), + if attr_options.crate_ { + return quote! { + compile_error!("`crate` argument is yet supported with codspeed_divan_compat"); } + .into(); } let codspeed_divan_crate_ident = format_ident!( @@ -72,10 +32,21 @@ pub fn bench_compat(attr: TokenStream, item: TokenStream) -> TokenStream { .unwrap_or("codspeed_divan_compat".to_string()) ); - filtered_args.push(syn::parse_quote!(crate = ::#codspeed_divan_crate_ident)); - // Important: keep macro name in sync with re-exported macro name in divan-compat lib + let mut transfered_args = attr_options.other_args; + + transfered_args.push(syn::parse_quote!(crate = ::#codspeed_divan_crate_ident)); + + if let Some(types) = attr_options.types { + transfered_args.push(Meta::NameValue(syn::MetaNameValue { + path: syn::parse_quote!(types), + eq_token: Default::default(), + value: Expr::Verbatim(types.into_token_stream()), + })); + } + + // WARN: keep macro name in sync with re-exported macro name in divan-compat lib let expanded = quote! { - #[::#codspeed_divan_crate_ident::bench_original(#(#filtered_args),*)] + #[::#codspeed_divan_crate_ident::bench_original(#(#transfered_args),*)] #input }; From 39c4a0f26940a79f84938262a301b5790ad738b2 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 10 Feb 2025 17:06:46 +0100 Subject: [PATCH 6/8] feat: manage type generics in codspeed instrumentation --- crates/divan_compat/examples/benches/math.rs | 1 - .../divan_compat/src/compat/entry/generic.rs | 137 ++++++++++++ .../src/compat/{entry.rs => entry/mod.rs} | 65 ++++++ crates/divan_compat/src/compat/mod.rs | 43 +++- crates/divan_compat/src/compat/uri.rs | 208 +++++++++++++++++- 5 files changed, 431 insertions(+), 23 deletions(-) create mode 100644 crates/divan_compat/src/compat/entry/generic.rs rename crates/divan_compat/src/compat/{entry.rs => entry/mod.rs} (70%) diff --git a/crates/divan_compat/examples/benches/math.rs b/crates/divan_compat/examples/benches/math.rs index 7a802497..b963fdfb 100644 --- a/crates/divan_compat/examples/benches/math.rs +++ b/crates/divan_compat/examples/benches/math.rs @@ -91,7 +91,6 @@ mod fibonacci { // Will be ignored in instrumented mode as we do not support type generics yet // O(n) - #[cfg(not(codspeed))] #[divan::bench( types = [BTreeMap, HashMap], args = VALUES, diff --git a/crates/divan_compat/src/compat/entry/generic.rs b/crates/divan_compat/src/compat/entry/generic.rs new file mode 100644 index 00000000..efee8b38 --- /dev/null +++ b/crates/divan_compat/src/compat/entry/generic.rs @@ -0,0 +1,137 @@ +use std::{ + any::{Any, TypeId}, + cmp::Ordering, + mem::ManuallyDrop, + sync::OnceLock, +}; + +use super::{BenchEntryRunner, GroupEntry}; + +// use crate::util::sort::natural_cmp; + +/// Compile-time entry for a generic benchmark function, generated by +/// `#[divan::bench]`. +/// +/// Unlike `BenchEntry`, this is for a specific generic type or `const`. +/// +/// Although this type contains trivially-`Copy` data, it *should not* implement +/// `Clone` because the memory address of each instance is used to determine the +/// relative order in `GroupEntry.generic_benches` when sorting benchmarks by +/// location. +pub struct GenericBenchEntry { + /// The associated group, for entry metadata. + pub group: &'static GroupEntry, + + /// The benchmarking function. + pub bench: BenchEntryRunner, + + /// A generic type. + pub ty: Option, + + /// A `const` value and associated data. + pub const_value: Option, +} + +impl GenericBenchEntry { + pub(crate) fn display_name(&self) -> &str { + match (&self.ty, &self.const_value) { + (_, Some(const_value)) => const_value.name(), + (Some(ty), None) => ty.display_name(), + (None, None) => unreachable!(), + } + } +} + +/// Generic type instantiation. +pub struct EntryType { + /// [`std::any::type_name`]. + get_type_name: fn() -> &'static str, + + /// [`std::any::TypeId::of`]. + #[allow(dead_code)] + get_type_id: fn() -> TypeId, +} + +impl EntryType { + /// Creates an instance for the given type. + pub const fn new() -> Self { + Self { + get_type_name: std::any::type_name::, + get_type_id: TypeId::of::, + } + } + + pub(crate) fn raw_name(&self) -> &'static str { + (self.get_type_name)() + } + + pub(crate) fn display_name(&self) -> &'static str { + let mut type_name = self.raw_name(); + + // Remove module components in type name. + while let Some((prev, next)) = type_name.split_once("::") { + // Do not go past generic type boundary. + if prev.contains('<') { + break; + } + type_name = next; + } + + type_name + } +} + +/// A reference to a `const` as a `&'static T`. +#[allow(dead_code)] +pub struct EntryConst { + /// `&'static T`. + value: *const (), + + /// [`PartialOrd::partial_cmp`]. + partial_cmp: unsafe fn(*const (), *const ()) -> Option, + + /// [`ToString::to_string`]. + to_string: unsafe fn(*const ()) -> String, + + /// Cached `to_string` result. + cached_string: ManuallyDrop>, +} + +// SAFETY: `T: Send + Sync`. +unsafe impl Send for EntryConst {} +unsafe impl Sync for EntryConst {} + +#[allow(dead_code)] +impl EntryConst { + /// Creates entry data for a `const` values. + pub const fn new(value: &'static T) -> Self + where + T: PartialOrd + ToString + Send + Sync, + { + unsafe fn partial_cmp(a: *const (), b: *const ()) -> Option { + T::partial_cmp(&*a.cast(), &*b.cast()) + } + + unsafe fn to_string(value: *const ()) -> String { + T::to_string(&*value.cast()) + } + + Self { + value: value as *const T as *const (), + partial_cmp: partial_cmp::, + to_string: to_string::, + cached_string: ManuallyDrop::new(OnceLock::new()), + } + } + + /// [`ToString::to_string`]. + #[inline] + pub(crate) fn name(&self) -> &str { + self.cached_string.get_or_init(|| { + // SAFETY: The function is guaranteed to call `T::to_string`. + let string = unsafe { (self.to_string)(self.value) }; + + Box::leak(string.into_boxed_str()) + }) + } +} diff --git a/crates/divan_compat/src/compat/entry.rs b/crates/divan_compat/src/compat/entry/mod.rs similarity index 70% rename from crates/divan_compat/src/compat/entry.rs rename to crates/divan_compat/src/compat/entry/mod.rs index 48166a96..2a4273ba 100644 --- a/crates/divan_compat/src/compat/entry.rs +++ b/crates/divan_compat/src/compat/entry/mod.rs @@ -1,5 +1,8 @@ //! Handpicked stubs from [divan::entry](https://github.com/nvzqz/divan/blob/main/src/entry/mod.rs) //! Necessary to be able to use the [divan::bench](https://docs.rs/divan/0.1.17/divan/attr.bench.html) macro without changing it too much + +mod generic; + use std::{ ptr, sync::{ @@ -13,12 +16,17 @@ use super::{ BenchArgsRunner, }; +pub use generic::{EntryType, GenericBenchEntry}; + /// Benchmark entries generated by `#[divan::bench]`. /// /// Note: generic-type benchmark entries are instead stored in `GROUP_ENTRIES` /// in `generic_benches`. pub static BENCH_ENTRIES: EntryList = EntryList::root(); +/// Group entries generated by `#[divan::bench_group]`. +pub static GROUP_ENTRIES: EntryList = EntryList::root(); + /// Determines how the benchmark entry is run. #[derive(Clone, Copy)] pub enum BenchEntryRunner { @@ -37,6 +45,36 @@ pub struct BenchEntry { pub bench: BenchEntryRunner, } +/// Compile-time entry for a benchmark group, generated by +/// `#[divan::bench_group]` or a generic-type `#[divan::bench]`. +pub struct GroupEntry { + /// Entry metadata. + pub meta: EntryMeta, + + /// Generic `#[divan::bench]` entries. + /// + /// This is two-dimensional to make code generation simpler. The outer + /// dimension corresponds to types and the inner dimension corresponds to + /// constants. + pub generic_benches: Option<&'static [&'static [GenericBenchEntry]]>, +} + +impl GroupEntry { + pub(crate) fn generic_benches_iter(&self) -> impl Iterator { + self.generic_benches + .unwrap_or_default() + .iter() + .flat_map(|benches| benches.iter()) + } +} + +/// `BenchEntry` or `GenericBenchEntry`. +#[derive(Clone, Copy)] +pub(crate) enum AnyBenchEntry<'a> { + Bench(&'a BenchEntry), + GenericBench(&'a GenericBenchEntry), +} + /// Metadata common to `#[divan::bench]` and `#[divan::bench_group]`. pub struct EntryMeta { /// The entry's display name. @@ -152,3 +190,30 @@ impl EntryList { } } } + +impl<'a> AnyBenchEntry<'a> { + /// Returns this entry's benchmark runner. + #[inline] + pub fn bench_runner(self) -> &'a BenchEntryRunner { + match self { + Self::Bench(BenchEntry { bench, .. }) + | Self::GenericBench(GenericBenchEntry { bench, .. }) => bench, + } + } + + #[inline] + pub fn meta(self) -> &'a EntryMeta { + match self { + Self::Bench(entry) => &entry.meta, + Self::GenericBench(entry) => &entry.group.meta, + } + } + + #[inline] + pub fn display_name(self) -> &'a str { + match self { + Self::Bench(entry) => entry.meta.display_name, + Self::GenericBench(entry) => entry.display_name(), + } + } +} diff --git a/crates/divan_compat/src/compat/mod.rs b/crates/divan_compat/src/compat/mod.rs index 6523103c..a7bd782b 100644 --- a/crates/divan_compat/src/compat/mod.rs +++ b/crates/divan_compat/src/compat/mod.rs @@ -3,7 +3,10 @@ pub mod __private { pub use super::{ bench::{BenchArgs, BenchOptions}, - entry::{BenchEntry, BenchEntryRunner, EntryList, EntryLocation, EntryMeta, BENCH_ENTRIES}, + entry::{ + BenchEntry, BenchEntryRunner, EntryList, EntryLocation, EntryMeta, EntryType, + GenericBenchEntry, GroupEntry, BENCH_ENTRIES, GROUP_ENTRIES, + }, }; pub use divan::__private::{Arg, ToStringHelper}; @@ -18,12 +21,25 @@ use std::{cell::RefCell, rc::Rc}; pub use bench::*; use codspeed::codspeed::CodSpeed; +use entry::AnyBenchEntry; pub fn main() { // Outlined steps of original divan::main and their equivalent in codspeed instrumented mode // 1. Get registered entries - // TODO: Manage bench groups - let bench_entries = &entry::BENCH_ENTRIES; + let group_entries = &entry::GROUP_ENTRIES; + + let generic_bench_entries = group_entries.iter().flat_map(|group| { + group + .generic_benches_iter() + .map(AnyBenchEntry::GenericBench) + }); + + let bench_entries = entry::BENCH_ENTRIES + .iter() + .map(AnyBenchEntry::Bench) + .chain(generic_bench_entries); + + // TODO: Manage non generic bench groups // 2. Build an execution tree // No need, we do not manage detailed tree printing like original divan, and we extract @@ -35,25 +51,30 @@ pub fn main() { // 4. Scan the tree and execute benchmarks let codspeed = Rc::new(RefCell::new(CodSpeed::new())); - for entry in bench_entries.iter() { - let entry_uri = uri::generate(entry.meta.display_name, &entry.meta); + for entry in bench_entries { + let runner = entry.bench_runner(); + let meta = entry.meta(); - if let Some(options) = &entry.meta.bench_options.as_ref() { + if let Some(options) = &meta.bench_options { if let Some(true) = options.ignore { - println!("Skipped: {}", entry_uri); + let uri = uri::generate(&entry, entry.display_name()); + println!("Skipped: {}", uri); continue; } } - match entry.bench { + match runner { entry::BenchEntryRunner::Plain(bench_fn) => { - bench_fn(bench::Bencher::new(&codspeed, entry_uri)); + let uri = uri::generate(&entry, entry.display_name()); + + bench_fn(bench::Bencher::new(&codspeed, uri)); } entry::BenchEntryRunner::Args(bench_runner) => { let bench_runner = bench_runner(); for (arg_index, arg_name) in bench_runner.arg_names().iter().enumerate() { - let entry_name_with_arg = uri::append_arg(&entry_uri, arg_name); - let bencher = bench::Bencher::new(&codspeed, entry_name_with_arg); + let uri = uri::generate(&entry, arg_name); + + let bencher = bench::Bencher::new(&codspeed, uri); bench_runner.bench(bencher, arg_index); } diff --git a/crates/divan_compat/src/compat/uri.rs b/crates/divan_compat/src/compat/uri.rs index ddf5a0ba..e9ece793 100644 --- a/crates/divan_compat/src/compat/uri.rs +++ b/crates/divan_compat/src/compat/uri.rs @@ -1,11 +1,76 @@ -use crate::__private::EntryMeta; - -pub(crate) fn generate( - bench_display_name: impl std::fmt::Display, - bench_meta: &EntryMeta, -) -> String { - let file = bench_meta.location.file; - let mut module_path = bench_meta +use super::AnyBenchEntry; + +/// Generate the codspeed URI for a benchmark entry. +/// The format is `"{file}::{module_path}::{bench_name}"`. +/// +/// # Bench Name Computation +/// There are three elements to consider: +/// - The static entry name from metadata, i.e., the name of the benchmarked function. +/// - The type (since benchmarks can be generic). +/// - The arguments (as you can statically specify a list of inputs for a benchmark). +/// +/// Depending on the nesting, you need to check three places: +/// - `entry.meta().display_name` +/// - `entry.display_name()` +/// - `closure_bench_display_name`, computed by divan when calling the closure that runs the bench +/// +/// From these three elements, we derive the codspeed bench name `function_name[type?, arg?]`: +/// - In the simple case (no generic, no args via macro), all three are equivalent. +/// - With an arg and no type, the first two are equal to the function name. +/// - With no arg and a type, the last two are equal to the type name. +/// - With both an arg and a type, all three have distinct values: the function name, the arg, and the type, respectively. +pub(crate) fn generate(bench_entry: &AnyBenchEntry, closure_bench_display_name: &str) -> String { + let bench_function_name = bench_entry.meta().display_name; + + let (bench_type_name, bench_arg_name) = { + let bench_function_or_type_name = bench_entry.display_name().to_string(); + + let type_name = if bench_function_or_type_name == bench_function_name { + None + } else { + Some(bench_function_or_type_name) + }; + + let arg_name = match type_name.as_ref() { + None => { + if closure_bench_display_name == bench_function_name { + None + } else { + Some(closure_bench_display_name) + } + } + Some(type_name) => { + if closure_bench_display_name == type_name { + None + } else { + Some(closure_bench_display_name) + } + } + }; + + (type_name, arg_name) + }; + + let mut bench_name = bench_function_name.to_string(); + + match (bench_type_name, bench_arg_name) { + (None, None) => {} + (Some(type_name), None) => { + bench_name.push_str(format!("[{type_name}]").as_str()); + } + (None, Some(arg_name)) => { + bench_name.push_str(format!("[{arg_name}]").as_str()); + } + (Some(type_name), Some(arg_name)) => { + bench_name.push_str(format!("[{type_name}, {arg_name}]").as_str()); + } + } + + let file = bench_entry.meta().location.file; + // In the context of a bench, the top level module will be a repetition of the file name, we + // chose to skip it + let mut module_path = bench_entry + .meta() .module_path_components() .skip(1) .collect::>() @@ -13,11 +78,132 @@ pub(crate) fn generate( if !module_path.is_empty() { module_path.push_str("::"); } - let uri = format!("{file}::{module_path}{bench_display_name}"); + let uri = format!("{file}::{module_path}{bench_name}"); uri } -pub(crate) fn append_arg(uri: &str, arg: &str) -> String { - format!("{uri}[{arg}]") +#[cfg(test)] +mod tests { + use crate::__private::*; + + use super::*; + + #[test] + fn test_generate_simple_case() { + let meta = EntryMeta { + display_name: "bench_function", + raw_name: "bench_function", + module_path: "test::module", + location: EntryLocation { + file: "foo.rs", + ..Default::default() + }, + bench_options: None, + }; + let bench_entry = BenchEntry { + meta, + bench: BenchEntryRunner::Plain(|_| {}), + }; + let closure_bench_display_name = "bench_function"; + let uri = generate( + &AnyBenchEntry::Bench(&bench_entry), + closure_bench_display_name, + ); + assert_eq!(uri, "foo.rs::module::bench_function"); + } + + #[test] + fn test_generate_with_arg() { + let meta = EntryMeta { + display_name: "bench_function", + raw_name: "bench_function", + module_path: "test::module", + location: EntryLocation { + file: "foo.rs", + ..Default::default() + }, + bench_options: None, + }; + let bench_entry = BenchEntry { + meta, + bench: BenchEntryRunner::Plain(|_| {}), + }; + let closure_bench_display_name = "ArgName"; + let uri = generate( + &AnyBenchEntry::Bench(&bench_entry), + closure_bench_display_name, + ); + assert_eq!(uri, "foo.rs::module::bench_function[ArgName]"); + } + + #[test] + fn test_generate_no_module_path() { + let meta = EntryMeta { + display_name: "bench_function", + raw_name: "bench_function", + module_path: "test", + location: EntryLocation { + file: "bar.rs", + ..Default::default() + }, + bench_options: None, + }; + let bench_entry = BenchEntry { + meta, + bench: BenchEntryRunner::Plain(|_| {}), + }; + let closure_bench_display_name = "bench_function"; + let uri = generate( + &AnyBenchEntry::Bench(&bench_entry), + closure_bench_display_name, + ); + assert_eq!(uri, "bar.rs::bench_function"); + } + + #[allow(non_upper_case_globals)] + static mock_group_entry: GroupEntry = GroupEntry { + meta: EntryMeta { + display_name: "bench_function", + raw_name: "bench_function", + module_path: "test::module", + location: EntryLocation { + file: "main.rs", + line: 0, + col: 0, + }, + bench_options: None, + }, + generic_benches: None, + }; + #[test] + fn test_generate_with_type() { + // Without arg + let hashmap_bench_entry = GenericBenchEntry { + group: &mock_group_entry, + bench: BenchEntryRunner::Plain(|_| {}), + ty: Some(EntryType::new::>()), + const_value: None, + }; + let entry = AnyBenchEntry::GenericBench(&hashmap_bench_entry); + let uri = generate(&entry, entry.display_name()); + assert_eq!(uri, "main.rs::module::bench_function[HashMap<&str, f64>]"); + } + + #[test] + fn test_generate_with_type_and_arg() { + let vec_bench_entry = GenericBenchEntry { + group: &mock_group_entry, + bench: BenchEntryRunner::Plain(|_| {}), + ty: Some(EntryType::new::>()), + const_value: None, + }; + + let closure_bench_display_name = "ArgName"; + let uri = generate( + &AnyBenchEntry::GenericBench(&vec_bench_entry), + closure_bench_display_name, + ); + assert_eq!(uri, "main.rs::module::bench_function[Vec, ArgName]"); + } } From 968e2621c7c83c721f198ad3b55554e4089c835a Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Tue, 11 Feb 2025 14:22:13 +0100 Subject: [PATCH 7/8] fix: remove discrepancies between original divan API and compat layer --- crates/divan_compat/benches/basic_example.rs | 11 +++++++++ crates/divan_compat/src/compat/bench/mod.rs | 25 +++++++++++--------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/crates/divan_compat/benches/basic_example.rs b/crates/divan_compat/benches/basic_example.rs index f74c3dc1..a93925af 100644 --- a/crates/divan_compat/benches/basic_example.rs +++ b/crates/divan_compat/benches/basic_example.rs @@ -1,3 +1,5 @@ +use codspeed_divan_compat::Bencher; + fn fibo(n: i32) -> i32 { let mut a = 0; let mut b = 1; @@ -21,6 +23,15 @@ fn fibo_10() -> i32 { codspeed_divan_compat::black_box(fibo(10)) } +#[codspeed_divan_compat::bench] +fn mut_borrow(bencher: Bencher) { + let mut bytes = Vec::::new(); + + bencher.bench_local(|| { + bytes.push(42); + }); +} + fn main() { codspeed_divan_compat::main(); } diff --git a/crates/divan_compat/src/compat/bench/mod.rs b/crates/divan_compat/src/compat/bench/mod.rs index 9931587c..48910ed1 100644 --- a/crates/divan_compat/src/compat/bench/mod.rs +++ b/crates/divan_compat/src/compat/bench/mod.rs @@ -19,7 +19,7 @@ use std::cell::RefCell; pub struct Unit; pub struct BencherConfig { - gen_input: GenI, + gen_input: RefCell, } pub struct Bencher<'a, 'b, C = BencherConfig> { @@ -29,11 +29,12 @@ pub struct Bencher<'a, 'b, C = BencherConfig> { pub(crate) _marker: std::marker::PhantomData<&'b ()>, } -#[allow(clippy::needless_lifetimes)] impl<'a, 'b> Bencher<'a, 'b> { pub(crate) fn new(codspeed: &'a RefCell, uri: String) -> Self { Self { - config: BencherConfig { gen_input: Unit }, + config: BencherConfig { + gen_input: RefCell::new(Unit), + }, codspeed, uri, _marker: std::marker::PhantomData, @@ -42,7 +43,9 @@ impl<'a, 'b> Bencher<'a, 'b> { pub fn with_inputs(self, gen_input: G) -> Bencher<'a, 'b, BencherConfig> { Bencher { - config: BencherConfig { gen_input }, + config: BencherConfig { + gen_input: RefCell::new(gen_input), + }, codspeed: self.codspeed, uri: self.uri, _marker: self._marker, @@ -58,9 +61,9 @@ impl<'a, 'b> Bencher<'a, 'b> { self.with_inputs(|| ()).bench_values(|_| benched()) } - pub fn bench_local(self, benched: B) + pub fn bench_local(self, mut benched: B) where - B: Fn() -> O, + B: FnMut() -> O, { self.with_inputs(|| ()).bench_local_values(|_| benched()) } @@ -86,24 +89,24 @@ where self.bench_local_refs(benched) } - pub fn bench_local_values(mut self, benched: B) + pub fn bench_local_values(self, mut benched: B) where - B: Fn(I) -> O, + B: FnMut(I) -> O, { let mut codspeed = self.codspeed.borrow_mut(); - let gen_input = &mut self.config.gen_input; + let mut gen_input = self.config.gen_input.borrow_mut(); let input = gen_input(); codspeed.start_benchmark(self.uri.as_str()); divan::black_box(benched(input)); codspeed.end_benchmark(); } - pub fn bench_local_refs(mut self, mut benched: B) + pub fn bench_local_refs(self, mut benched: B) where B: FnMut(&mut I) -> O, { let mut codspeed = self.codspeed.borrow_mut(); - let gen_input = &mut self.config.gen_input; + let mut gen_input = self.config.gen_input.borrow_mut(); let mut input = gen_input(); codspeed.start_benchmark(self.uri.as_str()); From 3a1c877d5b11d133a12d9b7eaddb7f98bb56b871 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Tue, 11 Feb 2025 15:52:21 +0100 Subject: [PATCH 8/8] refactor(divan_fork): keep all codspeed code within one function --- crates/divan_compat/divan_fork/src/divan.rs | 154 ++++++++++---------- 1 file changed, 76 insertions(+), 78 deletions(-) diff --git a/crates/divan_compat/divan_fork/src/divan.rs b/crates/divan_compat/divan_fork/src/divan.rs index 6eb79a9e..942a0e5c 100644 --- a/crates/divan_compat/divan_fork/src/divan.rs +++ b/crates/divan_compat/divan_fork/src/divan.rs @@ -310,27 +310,11 @@ impl Divan { if should_compute_stats { let stats = bench_context.compute_stats(); - { - let CodspeedBench { bench_name, uri } = - generate_codspeed_bench_name(&bench_entry, bench_display_name); - - let iter_per_round = bench_context.samples.sample_size; - let times_ns: Vec<_> = bench_context - .samples - .time_samples - .iter() - .map(|s| s.duration.picos / 1_000) - .collect(); - let max_time_ns = options.max_time.map(|t| t.as_nanos()); - ::codspeed::walltime::collect_raw_walltime_results( - "divan", - bench_name, - uri, - iter_per_round, - max_time_ns, - times_ns, - ); - }; + codspeed::collect_walltime_results( + &bench_context, + &bench_entry, + bench_display_name, + ); tree_painter.borrow_mut().finish_leaf( is_last_thread_count, &stats, @@ -371,74 +355,88 @@ impl Divan { } } -struct CodspeedBench { - bench_name: String, - uri: String, -} +mod codspeed { + use crate::bench::BenchContext; + use crate::entry::AnyBenchEntry; -/// Generates a Codspeed benchmark name and URI -/// -/// WARNING: Keep in sync with `codspeed-divan-compat::uri::generate` -/// Not worth doing the work of actually using the same code since this fork -/// is temporary -fn generate_codspeed_bench_name( - bench_entry: &AnyBenchEntry, - closure_bench_display_name: &str, -) -> CodspeedBench { - let bench_function_name = bench_entry.meta().display_name; - - let (bench_type_name, bench_arg_name) = { - let bench_function_or_type_name = bench_entry.display_name().to_string(); - - let type_name = if bench_function_or_type_name == bench_function_name { - None - } else { - Some(bench_function_or_type_name) - }; + pub(crate) fn collect_walltime_results( + bench_context: &BenchContext, + bench_entry: &AnyBenchEntry, + closure_bench_display_name: &str, + ) { + // WARNING: Keep URI generation in sync with `codspeed-divan-compat::uri::generate` + // Not worth doing the work of actually using the same code since this fork is temporary + let (bench_name, uri) = { + let bench_function_name = bench_entry.meta().display_name; + + let (bench_type_name, bench_arg_name) = { + let bench_function_or_type_name = bench_entry.display_name().to_string(); - let arg_name = match type_name.as_ref() { - None => { - if closure_bench_display_name == bench_function_name { + let type_name = if bench_function_or_type_name == bench_function_name { None } else { - Some(closure_bench_display_name) + Some(bench_function_or_type_name) + }; + + let arg_name = match type_name.as_ref() { + None => { + if closure_bench_display_name == bench_function_name { + None + } else { + Some(closure_bench_display_name) + } + } + Some(type_name) => { + if closure_bench_display_name == type_name { + None + } else { + Some(closure_bench_display_name) + } + } + }; + + (type_name, arg_name) + }; + + let mut bench_name = bench_function_name.to_string(); + + match (bench_type_name, bench_arg_name) { + (None, None) => {} + (Some(type_name), None) => { + bench_name.push_str(format!("[{type_name}]").as_str()); } - } - Some(type_name) => { - if closure_bench_display_name == type_name { - None - } else { - Some(closure_bench_display_name) + (None, Some(arg_name)) => { + bench_name.push_str(format!("[{arg_name}]").as_str()); + } + (Some(type_name), Some(arg_name)) => { + bench_name.push_str(format!("[{type_name}, {arg_name}]").as_str()); } } - }; - - (type_name, arg_name) - }; - let mut bench_name = bench_function_name.to_string(); + let file = bench_entry.meta().location.file; + let mut module_path = + bench_entry.meta().module_path_components().skip(1).collect::>().join("::"); + if !module_path.is_empty() { + module_path.push_str("::"); + } + let uri = format!("{file}::{module_path}{bench_name}"); + (bench_name, uri) + }; - match (bench_type_name, bench_arg_name) { - (None, None) => {} - (Some(type_name), None) => { - bench_name.push_str(format!("[{type_name}]").as_str()); - } - (None, Some(arg_name)) => { - bench_name.push_str(format!("[{arg_name}]").as_str()); - } - (Some(type_name), Some(arg_name)) => { - bench_name.push_str(format!("[{type_name}, {arg_name}]").as_str()); - } - } + let iter_per_round = bench_context.samples.sample_size; + let times_ns: Vec<_> = + bench_context.samples.time_samples.iter().map(|s| s.duration.picos / 1_000).collect(); + let max_time_ns = bench_context.options.max_time.map(|t| t.as_nanos()); - let file = bench_entry.meta().location.file; - let mut module_path = - bench_entry.meta().module_path_components().skip(1).collect::>().join("::"); - if !module_path.is_empty() { - module_path.push_str("::"); + ::codspeed::walltime::collect_raw_walltime_results( + "divan", + bench_name, + uri, + iter_per_round, + max_time_ns, + times_ns, + ); } - let uri = format!("{file}::{module_path}{bench_name}"); - CodspeedBench { bench_name, uri } } /// Makes `Divan::skip_regex` input polymorphic.