Stabilize criterion benchmark results

benchmark/run_benchmarks_ci.sh

@@ -22,7 +22,7 @@ pushd "${PROJECT_DIR}" > /dev/null
 
 # Run benchmarks
 message "Running benchmarks"
-cargo bench --workspace -- --sample-size=200
+cargo bench --workspace -- --warm-up-time 1 --measurement-time 5 --sample-size=200
 message "Finished running benchmarks"
 
 # Copy the benchmark results to the output directory

trace-normalization/benches/normalization_utils.rs

@@ -1,12 +1,19 @@
 // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/
 // SPDX-License-Identifier: Apache-2.0
 
-use criterion::{criterion_group, criterion_main, BatchSize, BenchmarkId, Criterion};
+use criterion::measurement::WallTime;
+use criterion::Throughput::Elements;
+use criterion::{
+    criterion_group, criterion_main, BatchSize, BenchmarkGroup, BenchmarkId, Criterion,
+};
+use datadog_trace_normalization::normalize_utils::{normalize_name, normalize_service};
+use datadog_trace_normalization::normalizer::normalize_trace;
 use datadog_trace_protobuf::pb;
-use std::collections::HashMap;
+use std::hint::black_box;
+use std::{collections::HashMap, time::Duration};
 
 fn normalize_service_bench(c: &mut Criterion) {
-    let mut group = c.benchmark_group("normalization/normalize_service");
+    let group = c.benchmark_group("normalization/normalize_service");
     let cases = &[
             "",
             "test_ASCII",
@@ -15,50 +22,67 @@ fn normalize_service_bench(c: &mut Criterion) {
             "A00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 000000000000",
         ];
 
-    for case in cases {
-        group.bench_with_input(
-            BenchmarkId::new(
-                "normalize_service",
-                if case.is_empty() {
-                    "[empty string]"
-                } else {
-                    case
-                },
-            ),
-            *case,
-            |b, case| {
-                b.iter_batched_ref(
-                    || case.to_owned(),
-                    datadog_trace_normalization::normalize_utils::normalize_service,
-                    BatchSize::NumBatches(100000),
-                )
-            },
-        );
-    }
-    group.finish()
+    normalize_fnmut_string(group, cases, 1000, "normalize_service", normalize_service);
 }
 
 fn normalize_name_bench(c: &mut Criterion) {
-    let mut group = c.benchmark_group("normalization/normalize_name");
+    let group = c.benchmark_group("normalization/normalize_name");
     let cases = &[
         "good",
         "bad-name",
         "Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.Too-Long-.",
     ];
+    normalize_fnmut_string(group, cases, 1000, "normalize_name", normalize_name);
+}
+
+#[inline]
+fn normalize_fnmut_string<F>(
+    mut group: BenchmarkGroup<WallTime>,
+    cases: &[&str],
+    elements: usize,
+    function_name: &str,
+    mut function: F,
+) where
+    F: FnMut(&mut String),
+{
+    // Measure over a number of calls to minimize impact of OS noise
+    group.throughput(Elements(elements as u64));
+    // We only need to measure for a small time since the function is very fast
+    group.warm_up_time(Duration::from_secs(1));
+    group.measurement_time(Duration::from_secs(2));
+    group.sample_size(200);
+    group.sampling_mode(criterion::SamplingMode::Flat);
+
     for case in cases {
         group.bench_with_input(
-            BenchmarkId::new("normalize_name", case),
+            BenchmarkId::new(
+                function_name,
+                if case.is_empty() {
+                    "[empty string]"
+                } else {
+                    case
+                },
+            ),
             *case,
             |b, case| {
                 b.iter_batched_ref(
-                    || case.to_owned(),
-                    datadog_trace_normalization::normalize_utils::normalize_name,
-                    BatchSize::NumIterations(100000),
+                    || {
+                        let mut strings = Vec::with_capacity(elements);
+                        (0..elements).for_each(|_| strings.push(case.to_owned()));
+                        strings
+                    },
+                    |strings| {
+                        #[allow(clippy::unit_arg)]
+                        strings.iter_mut().for_each(|string| {
+                            black_box(function(black_box(string)));
+                        });
+                    },
+                    BatchSize::LargeInput,
                 )
             },
         );
     }
-    group.finish()
+    group.finish();
 }
 
 fn normalize_span_bench(c: &mut Criterion) {
@@ -109,8 +133,8 @@ fn normalize_span_bench(c: &mut Criterion) {
         |b, case| {
             b.iter_batched_ref(
                 || case.to_owned(),
-                |s| datadog_trace_normalization::normalizer::normalize_trace(s),
-                BatchSize::SmallInput,
+                |t| black_box(normalize_trace(black_box(t))),
+                BatchSize::LargeInput,
             )
         },
     );

trace-obfuscation/benches/benchmarks/credit_cards_bench.rs

@@ -1,31 +1,32 @@
 // Copyright 2023-Present Datadog, Inc. https://www.datadoghq.com/
 // SPDX-License-Identifier: Apache-2.0
 
+use std::hint::black_box;
+use std::time::Duration;
+
 use criterion::Throughput::Elements;
-use criterion::{criterion_group, BenchmarkId, Criterion};
+use criterion::{criterion_group, BatchSize, BenchmarkId, Criterion};
 use datadog_trace_obfuscation::credit_cards::is_card_number;
 
 pub fn is_card_number_bench(c: &mut Criterion) {
-    let mut group = c.benchmark_group("credit_card");
-    let ccs = [
-        "378282246310005",
-        "  378282246310005",
-        "  3782-8224-6310-005 ",
-        "37828224631000521389798", // valid but too long
-        "37828224631",             // valid but too short
-        "x371413321323331",        // invalid characters
-        "",
-    ];
-    for c in ccs.iter() {
-        group.throughput(Elements(1));
-        group.bench_with_input(BenchmarkId::new("is_card_number", c), c, |b, i| {
-            b.iter(|| is_card_number(i, true))
-        });
-    }
+    bench_is_card_number(c, "is_card_number", true);
 }
 
 fn is_card_number_no_luhn_bench(c: &mut Criterion) {
+    bench_is_card_number(c, "is_card_number_no_luhn", false);
+}
+
+#[inline]
+fn bench_is_card_number(c: &mut Criterion, function_name: &str, validate_luhn: bool) {
     let mut group = c.benchmark_group("credit_card");
+    // Measure over a number of calls to minimize impact of OS noise
+    let elements = 1000;
+    group.throughput(Elements(elements));
+    // We only need to measure for a small time since the function is very fast
+    group.warm_up_time(Duration::from_secs(1));
+    group.measurement_time(Duration::from_secs(2));
+    group.sampling_mode(criterion::SamplingMode::Flat);
+    group.sample_size(200);
     let ccs = [
         "378282246310005",
         "  378282246310005",
@@ -36,9 +37,16 @@ fn is_card_number_no_luhn_bench(c: &mut Criterion) {
         "",
     ];
     for c in ccs.iter() {
-        group.throughput(Elements(1));
-        group.bench_with_input(BenchmarkId::new("is_card_number_no_luhn", c), c, |b, i| {
-            b.iter(|| is_card_number(i, false))
+        group.bench_with_input(BenchmarkId::new(function_name, c), c, |b, i| {
+            b.iter_batched(
+                || {},
+                |_| {
+                    (0..elements).for_each(|_| {
+                        black_box(is_card_number(black_box(i), black_box(validate_luhn)));
+                    })
+                },
+                BatchSize::SmallInput,
+            )
         });
     }
 }

trace-obfuscation/benches/benchmarks/redis_obfuscation_bench.rs

@@ -96,11 +96,16 @@ SET k v
     ];
 
     group.bench_function("obfuscate_redis_string", |b| {
-        b.iter(|| {
-            for c in cases {
-                black_box(redis::obfuscate_redis_string(c));
-            }
-        })
+        b.iter_batched_ref(
+            // Keep the String instances around to avoid measuring the deallocation cost
+            || Vec::with_capacity(cases.len()) as Vec<String>,
+            |res: &mut Vec<String>| {
+                for c in cases {
+                    res.push(black_box(redis::obfuscate_redis_string(c)));
+                }
+            },
+            criterion::BatchSize::LargeInput,
+        )
     });
 }
 

trace-obfuscation/benches/benchmarks/replace_trace_tags_bench.rs

@@ -48,11 +48,13 @@ fn criterion_benchmark(c: &mut Criterion) {
         span_links: vec![],
     };
 
-    let mut trace = [span_1];
+    let trace = [span_1];
     group.bench_function("replace_trace_tags", |b| {
-        b.iter(|| {
-            replacer::replace_trace_tags(black_box(&mut trace), black_box(rules));
-        })
+        b.iter_batched_ref(
+            || trace.to_owned(),
+            |t| replacer::replace_trace_tags(black_box(t), black_box(rules)),
+            criterion::BatchSize::LargeInput,
+        )
     });
 }
 

trace-obfuscation/benches/benchmarks/sql_obfuscation_bench.rs

@@ -7,11 +7,16 @@ use datadog_trace_obfuscation::sql::obfuscate_sql_string;
 fn sql_obfuscation(c: &mut Criterion) {
     let mut group = c.benchmark_group("sql");
     group.bench_function("obfuscate_sql_string", |b| {
-        b.iter(|| {
-            for (input, _) in CASES {
-                black_box(obfuscate_sql_string(input));
-            }
-        })
+        b.iter_batched_ref(
+            // Keep the String instances around to avoid measuring the deallocation cost
+            || Vec::with_capacity(CASES.len()) as Vec<String>,
+            |res: &mut Vec<String>| {
+                for (input, _) in CASES {
+                    res.push(black_box(obfuscate_sql_string(input)));
+                }
+            },
+            criterion::BatchSize::LargeInput,
+        )
     });
 }
 

trace-utils/benches/deserialization.rs

@@ -1,7 +1,7 @@
 // Copyright 2024-Present Datadog, Inc. https://www.datadoghq.com/
 // SPDX-License-Identifier: Apache-2.0
 
-use criterion::{criterion_group, Criterion};
+use criterion::{black_box, criterion_group, Criterion};
 use datadog_trace_utils::tracer_header_tags::TracerHeaderTags;
 use datadog_trace_utils::tracer_payload::{
     DefaultTraceChunkProcessor, TraceEncoding, TracerPayloadCollection, TracerPayloadParams,
@@ -44,18 +44,25 @@ pub fn deserialize_msgpack_to_internal(c: &mut Criterion) {
     c.bench_function(
         "benching deserializing traces from msgpack to their internal representation ",
         |b| {
-            b.iter(|| {
-                let result: anyhow::Result<TracerPayloadCollection> = TracerPayloadParams::new(
-                    &data,
-                    tracer_header_tags,
-                    &mut DefaultTraceChunkProcessor,
-                    false,
-                    TraceEncoding::V04,
-                )
-                .try_into();
-
-                assert!(result.is_ok())
-            })
+            b.iter_batched(
+                || &data,
+                |data| {
+                    let result: anyhow::Result<TracerPayloadCollection> = black_box(
+                        TracerPayloadParams::new(
+                            data,
+                            tracer_header_tags,
+                            &mut DefaultTraceChunkProcessor,
+                            false,
+                            TraceEncoding::V04,
+                        )
+                        .try_into(),
+                    );
+                    assert!(result.is_ok());
+                    // Return the result to avoid measuring the deallocation time
+                    result
+                },
+                criterion::BatchSize::LargeInput,
+            );
         },
     );
 }