From a3c91215b724d3d1540d7f54238a4b1ac0aec9bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bjo=CC=88rn=20Antonsson?= Date: Wed, 7 Aug 2024 11:32:38 +0200 Subject: [PATCH] Stabilize criterion benchmark results --- benchmark/run_benchmarks_ci.sh | 2 +- .../benches/normalization_utils.rs | 81 ++++++++++++------- .../benches/benchmarks/credit_cards_bench.rs | 47 ++++++----- .../benchmarks/redis_obfuscation_bench.rs | 15 ++-- .../benchmarks/replace_trace_tags_bench.rs | 10 ++- .../benchmarks/sql_obfuscation_bench.rs | 15 ++-- trace-utils/benches/deserialization.rs | 33 +++++--- 7 files changed, 124 insertions(+), 79 deletions(-) diff --git a/benchmark/run_benchmarks_ci.sh b/benchmark/run_benchmarks_ci.sh index d08f6a324..cb249c07c 100755 --- a/benchmark/run_benchmarks_ci.sh +++ b/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 diff --git a/trace-normalization/benches/normalization_utils.rs b/trace-normalization/benches/normalization_utils.rs index 022e31bf7..9bc69cab2 100644 --- a/trace-normalization/benches/normalization_utils.rs +++ b/trace-normalization/benches/normalization_utils.rs @@ -1,12 +1,18 @@ // 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::{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 +21,63 @@ 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, 500, "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, 500, "normalize_name", normalize_name); +} + +#[inline] +fn normalize_fnmut_string( + mut group: BenchmarkGroup, + cases: &[&str], + elements: u64, + 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)); + // 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(3)); + 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), + |mutable| { + (0..elements).for_each(|_| { + mutable.insert_str(0, case); + function(mutable); + }); + }, + BatchSize::SmallInput, ) }, ); } - group.finish() + group.finish(); } fn normalize_span_bench(c: &mut Criterion) { @@ -109,8 +128,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| normalize_trace(t), + BatchSize::LargeInput, ) }, ); diff --git a/trace-obfuscation/benches/benchmarks/credit_cards_bench.rs b/trace-obfuscation/benches/benchmarks/credit_cards_bench.rs index 945ec9ecc..b88d27a05 100644 --- a/trace-obfuscation/benches/benchmarks/credit_cards_bench.rs +++ b/trace-obfuscation/benches/benchmarks/credit_cards_bench.rs @@ -1,31 +1,31 @@ // Copyright 2023-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 +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 +36,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(|_| { + is_card_number(i, validate_luhn); + }) + }, + BatchSize::SmallInput, + ) }); } } diff --git a/trace-obfuscation/benches/benchmarks/redis_obfuscation_bench.rs b/trace-obfuscation/benches/benchmarks/redis_obfuscation_bench.rs index 08ae0b558..2864fbd73 100644 --- a/trace-obfuscation/benches/benchmarks/redis_obfuscation_bench.rs +++ b/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, + |res: &mut Vec| { + for c in cases { + res.push(black_box(redis::obfuscate_redis_string(c))); + } + }, + criterion::BatchSize::LargeInput, + ) }); } diff --git a/trace-obfuscation/benches/benchmarks/replace_trace_tags_bench.rs b/trace-obfuscation/benches/benchmarks/replace_trace_tags_bench.rs index 3875b756d..589fe8f51 100644 --- a/trace-obfuscation/benches/benchmarks/replace_trace_tags_bench.rs +++ b/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, + ) }); } diff --git a/trace-obfuscation/benches/benchmarks/sql_obfuscation_bench.rs b/trace-obfuscation/benches/benchmarks/sql_obfuscation_bench.rs index b0caa28b8..97e7c63d8 100644 --- a/trace-obfuscation/benches/benchmarks/sql_obfuscation_bench.rs +++ b/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, + |res: &mut Vec| { + for (input, _) in CASES { + res.push(black_box(obfuscate_sql_string(input))); + } + }, + criterion::BatchSize::LargeInput, + ) }); } diff --git a/trace-utils/benches/deserialization.rs b/trace-utils/benches/deserialization.rs index 0a252b07b..c4573c3d9 100644 --- a/trace-utils/benches/deserialization.rs +++ b/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 = 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 = 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, + ); }, ); }