chore: Fix small nits on benchmarks and remove throughput (#2713)

open-telemetry · Feb 26, 2025 · f9ccdff · f9ccdff
1 parent 261ac75
commit f9ccdff
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 54 deletions.
diff --git a/opentelemetry-sdk/benches/context.rs b/opentelemetry-sdk/benches/context.rs
@@ -1,4 +1,4 @@
-use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion, Throughput};
+use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion};
 use opentelemetry::{
     global::BoxedTracer,
     trace::{
@@ -17,7 +17,6 @@ use std::fmt::Display;
 
 fn criterion_benchmark(c: &mut Criterion) {
     let mut group = c.benchmark_group("context");
-    group.throughput(Throughput::Elements(1));
     for env in [
         Environment::InContext,
         Environment::NoContext,

diff --git a/opentelemetry/benches/context_attach.rs b/opentelemetry/benches/context_attach.rs
@@ -1,14 +1,14 @@
 use criterion::{
     black_box, criterion_group, criterion_main, measurement::WallTime, BenchmarkGroup, BenchmarkId,
-    Criterion, Throughput,
+    Criterion,
 };
 use opentelemetry::{
     trace::{SpanContext, TraceContextExt},
     Context,
 };
 
 // Run this benchmark with:
-// cargo bench --bench current_context
+// cargo bench --bench context_attach
 
 fn criterion_benchmark(c: &mut Criterion) {
     let span_context = Context::new().with_remote_span_context(SpanContext::empty_context());
@@ -29,80 +29,68 @@ fn single_cx_scope(
     context_type: &str,
     context: &Context,
 ) {
-    let _restore = Context::current().attach();
-    group.throughput(Throughput::Elements(1)).bench_function(
-        BenchmarkId::new("single_cx_scope", context_type),
-        |b| {
-            b.iter_batched(
-                || context.clone(),
-                |cx| {
-                    single_cx(cx);
-                },
-                criterion::BatchSize::SmallInput,
-            );
-        },
-    );
+    group.bench_function(BenchmarkId::new("single_cx", context_type), |b| {
+        b.iter_batched(
+            || context.clone(),
+            |cx| {
+                single_cx(cx);
+            },
+            criterion::BatchSize::SmallInput,
+        );
+    });
 }
 
 #[inline(never)]
 fn single_cx(cx: Context) {
-    let cx = black_box(cx.attach());
+    let _cx_guard = black_box(cx.attach());
     let _ = black_box(dummy_work());
-    drop(cx);
 }
 
 fn nested_cx_scope(group: &mut BenchmarkGroup<'_, WallTime>, cx_type: &str, context: &Context) {
-    let _restore = Context::current().attach();
-    group.throughput(Throughput::Elements(1)).bench_function(
-        BenchmarkId::new("nested_cx_scope", cx_type),
-        |b| {
-            b.iter_batched(
-                || (context.clone(), context.clone()),
-                |(cx1, cx2)| {
-                    nested_cx(cx1, cx2);
-                },
-                criterion::BatchSize::SmallInput,
-            );
-        },
-    );
+    group.bench_function(BenchmarkId::new("nested_cx", cx_type), |b| {
+        b.iter_batched(
+            || (context.clone(), context.clone()),
+            |(cx1, cx2)| {
+                nested_cx(cx1, cx2);
+            },
+            criterion::BatchSize::SmallInput,
+        );
+    });
 }
 
 #[inline(never)]
 fn nested_cx(cx1: Context, cx2: Context) {
-    let outer = black_box(cx1.attach());
-    let inner = black_box(cx2.attach());
+    let _outer_cx_guard = black_box(cx1.attach());
+    let _inner_cx_guard = black_box(cx2.attach());
     let _ = black_box(dummy_work());
-    drop(inner);
-    drop(outer);
 }
 
 fn overlapping_cx_scope(
     group: &mut BenchmarkGroup<'_, WallTime>,
     cx_type: &str,
     context: &Context,
 ) {
-    let _restore = Context::current().attach();
-    group.throughput(Throughput::Elements(1)).bench_function(
-        BenchmarkId::new("overlapping_cx_scope", cx_type),
-        |b| {
-            b.iter_batched(
-                || (context.clone(), context.clone()),
-                |(cx1, cx2)| {
-                    overlapping_cx(cx1, cx2);
-                },
-                criterion::BatchSize::SmallInput,
-            );
-        },
-    );
+    // This is to ensure that the context is restored after the benchmark,
+    // see https://github.com/open-telemetry/opentelemetry-rust/issues/1887
+    let _restore_cx_guard = Context::current().attach();
+    group.bench_function(BenchmarkId::new("out_of_order_cx_drop", cx_type), |b| {
+        b.iter_batched(
+            || (context.clone(), context.clone()),
+            |(cx1, cx2)| {
+                out_of_order_cx_drop(cx1, cx2);
+            },
+            criterion::BatchSize::SmallInput,
+        );
+    });
 }
 
 #[inline(never)]
-fn overlapping_cx(cx1: Context, cx2: Context) {
-    let outer = cx1.attach();
-    let inner = cx2.attach();
+fn out_of_order_cx_drop(cx1: Context, cx2: Context) {
+    let outer_cx_guard = cx1.attach();
+    let inner_cx_guard = cx2.attach();
     let _ = black_box(dummy_work());
-    drop(outer);
-    drop(inner);
+    drop(outer_cx_guard);
+    drop(inner_cx_guard);
 }
 
 #[inline(never)]