From 1b86d3bba3bff57e50d1d65278d1e5b68cbfc541 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Wed, 8 Jan 2025 20:37:58 +0000 Subject: [PATCH 1/4] Update benchmarks --- opentelemetry-sdk/benches/metrics_counter.rs | 82 +++++++++++++++++-- .../benches/metrics_histogram.rs | 76 +++++++++++++++-- 2 files changed, 143 insertions(+), 15 deletions(-) diff --git a/opentelemetry-sdk/benches/metrics_counter.rs b/opentelemetry-sdk/benches/metrics_counter.rs index b4517d379b..8512a9509c 100644 --- a/opentelemetry-sdk/benches/metrics_counter.rs +++ b/opentelemetry-sdk/benches/metrics_counter.rs @@ -1,16 +1,17 @@ /* The benchmark results: criterion = "0.5.1" - rustc 1.82.0 (f6e511eec 2024-10-15) - OS: Ubuntu 22.04.3 LTS (5.15.167.4-microsoft-standard-WSL2) - Hardware: AMD EPYC 7763 64-Core Processor - 2.44 GHz, 16vCPUs, + rustc 1.83.0 (90b35a623 2024-11-26) + OS: Ubuntu 22.04.4 LTS (5.15.167.4-microsoft-standard-WSL2) + Hardware: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz 2.79 GHz RAM: 64.0 GB - | Test | Average time| - |--------------------------------|-------------| - | Counter_Add_Sorted | 172 ns | - | Counter_Add_Unsorted | 183 ns | - | Counter_Overflow | 562 ns | - | ThreadLocal_Random_Generator_5 | 37 ns | + | Test | Average time| + |-------------------------------------------------------|-------------| + | Counter_Add_Sorted | 160 ns | + | Counter_Add_Unsorted | 164 ns | + | Counter_Add_Sorted_With_Non_Static_Values | 439 ns | + | Counter_Overflow | 562 ns | + | ThreadLocal_Random_Generator_5 | 37 ns | */ use criterion::{criterion_group, criterion_main, BatchSize, Criterion}; @@ -51,6 +52,7 @@ fn create_counter(name: &'static str) -> Counter { fn criterion_benchmark(c: &mut Criterion) { counter_add_sorted(c); counter_add_unsorted(c); + counter_add_sorted_with_non_static_values(c); counter_overflow(c); random_generator(c); } @@ -127,6 +129,68 @@ fn counter_add_unsorted(c: &mut Criterion) { }); } +fn counter_add_sorted_with_non_static_values(c: &mut Criterion) { + let counter = create_counter("Counter_Add_Sorted_With_Non_Static_Values"); + c.bench_function("Counter_Add_Sorted_With_Non_Static_Values", |b| { + b.iter_batched( + || { + ( + [ + "value1".to_owned(), + "value2".to_owned(), + "value3".to_owned(), + "value4".to_owned(), + "value5".to_owned(), + "value6".to_owned(), + "value7".to_owned(), + "value8".to_owned(), + "value9".to_owned(), + "value10".to_owned(), + ], + // 4*4*10*10 = 1600 time series. + CURRENT_RNG.with(|rng| { + let mut rng = rng.borrow_mut(); + [ + rng.gen_range(0..4), + rng.gen_range(0..4), + rng.gen_range(0..10), + rng.gen_range(0..10), + ] + }), + ) + }, + |(attribute_values, rands)| { + let index_first_attribute = rands[0]; + let index_second_attribute = rands[1]; + let index_third_attribute = rands[2]; + let index_fourth_attribute = rands[3]; + counter.add( + 1, + &[ + KeyValue::new( + "attribute1", + attribute_values[index_first_attribute].as_str().to_owned(), + ), + KeyValue::new( + "attribute2", + attribute_values[index_second_attribute].as_str().to_owned(), + ), + KeyValue::new( + "attribute3", + attribute_values[index_third_attribute].as_str().to_owned(), + ), + KeyValue::new( + "attribute4", + attribute_values[index_fourth_attribute].as_str().to_owned(), + ), + ], + ); + }, + BatchSize::SmallInput, + ); + }); +} + fn counter_overflow(c: &mut Criterion) { let counter = create_counter("Counter_Overflow"); // Cause overflow. diff --git a/opentelemetry-sdk/benches/metrics_histogram.rs b/opentelemetry-sdk/benches/metrics_histogram.rs index c6d5751dd6..2d5f0cb1b8 100644 --- a/opentelemetry-sdk/benches/metrics_histogram.rs +++ b/opentelemetry-sdk/benches/metrics_histogram.rs @@ -1,17 +1,18 @@ /* The benchmark results: criterion = "0.5.1" - rustc 1.82.0 (f6e511eec 2024-10-15) + rustc 1.83.0 (90b35a623 2024-11-26) OS: Ubuntu 22.04.4 LTS (5.15.167.4-microsoft-standard-WSL2) - Hardware: AMD EPYC 7763 64-Core Processor - 2.44 GHz, 16vCPUs, + Hardware: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz 2.79 GHz RAM: 64.0 GB - | Test | Average time| - |--------------------------------|-------------| - | Histogram_Record | 225.04 ns | + | Test | Average time| + |-------------------------------------------------------|-------------| + | Histogram_Record | 206.35 ns | + | Histogram_Record_With_Non_Static_Values | 483.58 ns | */ -use criterion::{criterion_group, criterion_main, Criterion}; +use criterion::{criterion_group, criterion_main, BatchSize, Criterion}; use opentelemetry::{ metrics::{Histogram, MeterProvider as _}, KeyValue, @@ -48,6 +49,7 @@ fn create_histogram(name: &'static str) -> Histogram { fn criterion_benchmark(c: &mut Criterion) { histogram_record(c); + histogram_record_with_non_static_values(c); } fn histogram_record(c: &mut Criterion) { @@ -81,6 +83,68 @@ fn histogram_record(c: &mut Criterion) { }); } +fn histogram_record_with_non_static_values(c: &mut Criterion) { + let histogram = create_histogram("Histogram_Record_With_Non_Static_Values"); + c.bench_function("Histogram_Record_With_Non_Static_Values", |b| { + b.iter_batched( + || { + ( + [ + "value1".to_owned(), + "value2".to_owned(), + "value3".to_owned(), + "value4".to_owned(), + "value5".to_owned(), + "value6".to_owned(), + "value7".to_owned(), + "value8".to_owned(), + "value9".to_owned(), + "value10".to_owned(), + ], + // 4*4*10*10 = 1600 time series. + CURRENT_RNG.with(|rng| { + let mut rng = rng.borrow_mut(); + [ + rng.gen_range(0..4), + rng.gen_range(0..4), + rng.gen_range(0..10), + rng.gen_range(0..10), + ] + }), + ) + }, + |(attribute_values, rands)| { + let index_first_attribute = rands[0]; + let index_second_attribute = rands[1]; + let index_third_attribute = rands[2]; + let index_fourth_attribute = rands[3]; + histogram.record( + 1, + &[ + KeyValue::new( + "attribute1", + attribute_values[index_first_attribute].as_str().to_owned(), + ), + KeyValue::new( + "attribute2", + attribute_values[index_second_attribute].as_str().to_owned(), + ), + KeyValue::new( + "attribute3", + attribute_values[index_third_attribute].as_str().to_owned(), + ), + KeyValue::new( + "attribute4", + attribute_values[index_fourth_attribute].as_str().to_owned(), + ), + ], + ); + }, + BatchSize::SmallInput, + ); + }); +} + #[cfg(not(target_os = "windows"))] criterion_group! { name = benches; From 6f3892d5192bd64f6e926149e6bb907599f4f324 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Wed, 8 Jan 2025 21:15:16 +0000 Subject: [PATCH 2/4] Fix benchmark --- opentelemetry-sdk/benches/metrics_counter.rs | 44 ++++----- .../benches/metrics_histogram.rs | 99 +++++++++---------- 2 files changed, 63 insertions(+), 80 deletions(-) diff --git a/opentelemetry-sdk/benches/metrics_counter.rs b/opentelemetry-sdk/benches/metrics_counter.rs index 8512a9509c..bdcdca3484 100644 --- a/opentelemetry-sdk/benches/metrics_counter.rs +++ b/opentelemetry-sdk/benches/metrics_counter.rs @@ -9,7 +9,7 @@ |-------------------------------------------------------|-------------| | Counter_Add_Sorted | 160 ns | | Counter_Add_Unsorted | 164 ns | - | Counter_Add_Sorted_With_Non_Static_Values | 439 ns | + | Counter_Add_Sorted_With_Non_Static_Values | 238 ns | | Counter_Overflow | 562 ns | | ThreadLocal_Random_Generator_5 | 37 ns | */ @@ -52,7 +52,11 @@ fn create_counter(name: &'static str) -> Counter { fn criterion_benchmark(c: &mut Criterion) { counter_add_sorted(c); counter_add_unsorted(c); - counter_add_sorted_with_non_static_values(c); + + let attribute_values = ["value1".to_owned(), "value2".to_owned(), "value3".to_owned(), "value4".to_owned(), "value5".to_owned(), "value6".to_owned(), "value7".to_owned(), "value8".to_owned(), "value9".to_owned(), "value10".to_owned()]; + + counter_add_sorted_with_non_static_values(c, attribute_values); + counter_overflow(c); random_generator(c); } @@ -129,37 +133,23 @@ fn counter_add_unsorted(c: &mut Criterion) { }); } -fn counter_add_sorted_with_non_static_values(c: &mut Criterion) { +fn counter_add_sorted_with_non_static_values(c: &mut Criterion, attribute_values: [String; 10]) { let counter = create_counter("Counter_Add_Sorted_With_Non_Static_Values"); c.bench_function("Counter_Add_Sorted_With_Non_Static_Values", |b| { b.iter_batched( || { - ( + // 4*4*10*10 = 1600 time series. + CURRENT_RNG.with(|rng| { + let mut rng = rng.borrow_mut(); [ - "value1".to_owned(), - "value2".to_owned(), - "value3".to_owned(), - "value4".to_owned(), - "value5".to_owned(), - "value6".to_owned(), - "value7".to_owned(), - "value8".to_owned(), - "value9".to_owned(), - "value10".to_owned(), - ], - // 4*4*10*10 = 1600 time series. - CURRENT_RNG.with(|rng| { - let mut rng = rng.borrow_mut(); - [ - rng.gen_range(0..4), - rng.gen_range(0..4), - rng.gen_range(0..10), - rng.gen_range(0..10), - ] - }), - ) + rng.gen_range(0..4), + rng.gen_range(0..4), + rng.gen_range(0..10), + rng.gen_range(0..10), + ] + }) }, - |(attribute_values, rands)| { + |rands| { let index_first_attribute = rands[0]; let index_second_attribute = rands[1]; let index_third_attribute = rands[2]; diff --git a/opentelemetry-sdk/benches/metrics_histogram.rs b/opentelemetry-sdk/benches/metrics_histogram.rs index 2d5f0cb1b8..5694a06630 100644 --- a/opentelemetry-sdk/benches/metrics_histogram.rs +++ b/opentelemetry-sdk/benches/metrics_histogram.rs @@ -7,8 +7,8 @@ RAM: 64.0 GB | Test | Average time| |-------------------------------------------------------|-------------| - | Histogram_Record | 206.35 ns | - | Histogram_Record_With_Non_Static_Values | 483.58 ns | + | Histogram_Record | 186.24 ns | + | Histogram_Record_With_Non_Static_Values | 264.70 ns | */ @@ -49,71 +49,64 @@ fn create_histogram(name: &'static str) -> Histogram { fn criterion_benchmark(c: &mut Criterion) { histogram_record(c); - histogram_record_with_non_static_values(c); + + let attribute_values = ["value1".to_owned(), "value2".to_owned(), "value3".to_owned(), "value4".to_owned(), "value5".to_owned(), "value6".to_owned(), "value7".to_owned(), "value8".to_owned(), "value9".to_owned(), "value10".to_owned()]; + histogram_record_with_non_static_values(c, attribute_values); } fn histogram_record(c: &mut Criterion) { let histogram = create_histogram("Histogram_Record"); c.bench_function("Histogram_Record", |b| { - b.iter(|| { - // 4*4*10*10 = 1600 time series. - let rands = CURRENT_RNG.with(|rng| { - let mut rng = rng.borrow_mut(); - [ - rng.gen_range(0..4), - rng.gen_range(0..4), - rng.gen_range(0..10), - rng.gen_range(0..10), - ] - }); - let index_first_attribute = rands[0]; - let index_second_attribute = rands[1]; - let index_third_attribute = rands[2]; - let index_fourth_attribute = rands[3]; - histogram.record( - 1, - &[ - KeyValue::new("attribute1", ATTRIBUTE_VALUES[index_first_attribute]), - KeyValue::new("attribute2", ATTRIBUTE_VALUES[index_second_attribute]), - KeyValue::new("attribute3", ATTRIBUTE_VALUES[index_third_attribute]), - KeyValue::new("attribute4", ATTRIBUTE_VALUES[index_fourth_attribute]), - ], - ); - }); + b.iter_batched( + || { + // 4*4*10*10 = 1600 time series. + CURRENT_RNG.with(|rng| { + let mut rng = rng.borrow_mut(); + [ + rng.gen_range(0..4), + rng.gen_range(0..4), + rng.gen_range(0..10), + rng.gen_range(0..10), + ] + }) + }, + |rands| { + let index_first_attribute = rands[0]; + let index_second_attribute = rands[1]; + let index_third_attribute = rands[2]; + let index_fourth_attribute = rands[3]; + histogram.record( + 1, + &[ + KeyValue::new("attribute1", ATTRIBUTE_VALUES[index_first_attribute]), + KeyValue::new("attribute2", ATTRIBUTE_VALUES[index_second_attribute]), + KeyValue::new("attribute3", ATTRIBUTE_VALUES[index_third_attribute]), + KeyValue::new("attribute4", ATTRIBUTE_VALUES[index_fourth_attribute]), + ], + ); + }, + BatchSize::SmallInput, + ); }); } -fn histogram_record_with_non_static_values(c: &mut Criterion) { +fn histogram_record_with_non_static_values(c: &mut Criterion, attribute_values: [String; 10]) { let histogram = create_histogram("Histogram_Record_With_Non_Static_Values"); c.bench_function("Histogram_Record_With_Non_Static_Values", |b| { b.iter_batched( || { - ( + // 4*4*10*10 = 1600 time series. + CURRENT_RNG.with(|rng| { + let mut rng = rng.borrow_mut(); [ - "value1".to_owned(), - "value2".to_owned(), - "value3".to_owned(), - "value4".to_owned(), - "value5".to_owned(), - "value6".to_owned(), - "value7".to_owned(), - "value8".to_owned(), - "value9".to_owned(), - "value10".to_owned(), - ], - // 4*4*10*10 = 1600 time series. - CURRENT_RNG.with(|rng| { - let mut rng = rng.borrow_mut(); - [ - rng.gen_range(0..4), - rng.gen_range(0..4), - rng.gen_range(0..10), - rng.gen_range(0..10), - ] - }), - ) + rng.gen_range(0..4), + rng.gen_range(0..4), + rng.gen_range(0..10), + rng.gen_range(0..10), + ] + }) }, - |(attribute_values, rands)| { + |rands| { let index_first_attribute = rands[0]; let index_second_attribute = rands[1]; let index_third_attribute = rands[2]; From e6ca3a740ef4e71506bea71b577b2793d0f33929 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Wed, 8 Jan 2025 21:22:37 +0000 Subject: [PATCH 3/4] Code changes --- opentelemetry-sdk/benches/metrics_counter.rs | 13 ++++++++++++- opentelemetry-sdk/benches/metrics_histogram.rs | 13 ++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/benches/metrics_counter.rs b/opentelemetry-sdk/benches/metrics_counter.rs index bdcdca3484..2b1442452d 100644 --- a/opentelemetry-sdk/benches/metrics_counter.rs +++ b/opentelemetry-sdk/benches/metrics_counter.rs @@ -53,7 +53,18 @@ fn criterion_benchmark(c: &mut Criterion) { counter_add_sorted(c); counter_add_unsorted(c); - let attribute_values = ["value1".to_owned(), "value2".to_owned(), "value3".to_owned(), "value4".to_owned(), "value5".to_owned(), "value6".to_owned(), "value7".to_owned(), "value8".to_owned(), "value9".to_owned(), "value10".to_owned()]; + let attribute_values = [ + "value1".to_owned(), + "value2".to_owned(), + "value3".to_owned(), + "value4".to_owned(), + "value5".to_owned(), + "value6".to_owned(), + "value7".to_owned(), + "value8".to_owned(), + "value9".to_owned(), + "value10".to_owned(), + ]; counter_add_sorted_with_non_static_values(c, attribute_values); diff --git a/opentelemetry-sdk/benches/metrics_histogram.rs b/opentelemetry-sdk/benches/metrics_histogram.rs index 5694a06630..c5c798ae7c 100644 --- a/opentelemetry-sdk/benches/metrics_histogram.rs +++ b/opentelemetry-sdk/benches/metrics_histogram.rs @@ -50,7 +50,18 @@ fn create_histogram(name: &'static str) -> Histogram { fn criterion_benchmark(c: &mut Criterion) { histogram_record(c); - let attribute_values = ["value1".to_owned(), "value2".to_owned(), "value3".to_owned(), "value4".to_owned(), "value5".to_owned(), "value6".to_owned(), "value7".to_owned(), "value8".to_owned(), "value9".to_owned(), "value10".to_owned()]; + let attribute_values = [ + "value1".to_owned(), + "value2".to_owned(), + "value3".to_owned(), + "value4".to_owned(), + "value5".to_owned(), + "value6".to_owned(), + "value7".to_owned(), + "value8".to_owned(), + "value9".to_owned(), + "value10".to_owned(), + ]; histogram_record_with_non_static_values(c, attribute_values); } From 5f0e3d4a47f88403511151d5a990aeb29d663b83 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Wed, 8 Jan 2025 23:20:41 +0000 Subject: [PATCH 4/4] Address PR comments --- opentelemetry-sdk/benches/metrics_counter.rs | 17 +++++------------ opentelemetry-sdk/benches/metrics_histogram.rs | 18 ++++++------------ 2 files changed, 11 insertions(+), 24 deletions(-) diff --git a/opentelemetry-sdk/benches/metrics_counter.rs b/opentelemetry-sdk/benches/metrics_counter.rs index 2b1442452d..3817fad144 100644 --- a/opentelemetry-sdk/benches/metrics_counter.rs +++ b/opentelemetry-sdk/benches/metrics_counter.rs @@ -53,18 +53,11 @@ fn criterion_benchmark(c: &mut Criterion) { counter_add_sorted(c); counter_add_unsorted(c); - let attribute_values = [ - "value1".to_owned(), - "value2".to_owned(), - "value3".to_owned(), - "value4".to_owned(), - "value5".to_owned(), - "value6".to_owned(), - "value7".to_owned(), - "value8".to_owned(), - "value9".to_owned(), - "value10".to_owned(), - ]; + let attribute_values: [String; 10] = (1..=10) + .map(|i| format!("value{}", i)) + .collect::>() + .try_into() + .expect("Expected a Vec of length 10"); counter_add_sorted_with_non_static_values(c, attribute_values); diff --git a/opentelemetry-sdk/benches/metrics_histogram.rs b/opentelemetry-sdk/benches/metrics_histogram.rs index c5c798ae7c..8a0b3d8125 100644 --- a/opentelemetry-sdk/benches/metrics_histogram.rs +++ b/opentelemetry-sdk/benches/metrics_histogram.rs @@ -50,18 +50,12 @@ fn create_histogram(name: &'static str) -> Histogram { fn criterion_benchmark(c: &mut Criterion) { histogram_record(c); - let attribute_values = [ - "value1".to_owned(), - "value2".to_owned(), - "value3".to_owned(), - "value4".to_owned(), - "value5".to_owned(), - "value6".to_owned(), - "value7".to_owned(), - "value8".to_owned(), - "value9".to_owned(), - "value10".to_owned(), - ]; + let attribute_values: [String; 10] = (1..=10) + .map(|i| format!("value{}", i)) + .collect::>() + .try_into() + .expect("Expected a Vec of length 10"); + histogram_record_with_non_static_values(c, attribute_values); }