From 94cfb4f612a62f3ed8ffa89a7e8394f121651b8e Mon Sep 17 00:00:00 2001 From: bm1549 Date: Fri, 10 Apr 2026 07:27:20 -0400 Subject: [PATCH 1/3] Optimize scope lifecycle by skipping work when listeners empty and profiling disabled tag: no release note tag: ai generated Co-Authored-By: Claude Opus 4.6 (1M context) --- .../trace/core/ScopeLifecycleBenchmark.java | 75 +++++++++++++++++++ .../core/scopemanager/ContinuableScope.java | 58 ++++++++------ .../scopemanager/ContinuableScopeManager.java | 34 ++++++--- 3 files changed, 133 insertions(+), 34 deletions(-) create mode 100644 dd-trace-core/src/jmh/java/datadog/trace/core/ScopeLifecycleBenchmark.java diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/ScopeLifecycleBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/ScopeLifecycleBenchmark.java new file mode 100644 index 00000000000..d3b600e2a34 --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/ScopeLifecycleBenchmark.java @@ -0,0 +1,75 @@ +package datadog.trace.core; + +import static java.util.concurrent.TimeUnit.MICROSECONDS; + +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +@State(Scope.Benchmark) +@Warmup(iterations = 3) +@Measurement(iterations = 5) +@BenchmarkMode(Mode.Throughput) +@Threads(8) +@OutputTimeUnit(MICROSECONDS) +@Fork(value = 1) +public class ScopeLifecycleBenchmark { + static final CoreTracer TRACER = CoreTracer.builder().build(); + + @State(Scope.Thread) + public static class ThreadState { + AgentSpan span; + AgentSpan childSpan; + + @Setup(Level.Iteration) + public void setup() { + span = TRACER.startSpan("benchmark", "parent"); + childSpan = TRACER.startSpan("benchmark", "child"); + } + + @TearDown(Level.Iteration) + public void tearDown() { + childSpan.finish(); + span.finish(); + } + } + + @Benchmark + public void activateAndClose(ThreadState state) { + AgentScope scope = TRACER.activateSpan(state.span); + scope.close(); + } + + @Benchmark + public void activateSameSpan(ThreadState state) { + AgentScope outer = TRACER.activateSpan(state.span); + AgentScope inner = TRACER.activateSpan(state.span); + inner.close(); + outer.close(); + } + + @Benchmark + public void nestedActivateAndClose(ThreadState state) { + AgentScope parentScope = TRACER.activateSpan(state.span); + AgentScope childScope = TRACER.activateSpan(state.childSpan); + childScope.close(); + parentScope.close(); + } + + @Benchmark + public AgentSpan activeSpanLookup() { + return TRACER.activeSpan(); + } +} diff --git a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java index 8451f914405..60cd1fbf483 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java @@ -80,19 +80,22 @@ void cleanup(final ScopeStack scopeStack) { * I would hope this becomes unnecessary. */ final void onProperClose() { - for (final ScopeListener listener : scopeManager.scopeListeners) { - try { - listener.afterScopeClosed(); - } catch (Exception e) { - ContinuableScopeManager.log.debug("ScopeListener threw exception in close()", e); + if (!scopeManager.scopeListeners.isEmpty()) { + for (final ScopeListener listener : scopeManager.scopeListeners) { + try { + listener.afterScopeClosed(); + } catch (Exception e) { + ContinuableScopeManager.log.debug("ScopeListener threw exception in close()", e); + } } } - - for (final ExtendedScopeListener listener : scopeManager.extendedScopeListeners) { - try { - listener.afterScopeClosed(); - } catch (Exception e) { - ContinuableScopeManager.log.debug("ScopeListener threw exception in close()", e); + if (!scopeManager.extendedScopeListeners.isEmpty()) { + for (final ExtendedScopeListener listener : scopeManager.extendedScopeListeners) { + try { + listener.afterScopeClosed(); + } catch (Exception e) { + ContinuableScopeManager.log.debug("ScopeListener threw exception in close()", e); + } } } } @@ -154,6 +157,9 @@ public boolean rollback() { } public final void beforeActivated() { + if (scopeState == Stateful.DEFAULT) { + return; + } AgentSpan span = span(); if (span == null) { return; @@ -167,24 +173,30 @@ public final void beforeActivated() { } public final void afterActivated() { + if (scopeManager.scopeListeners.isEmpty() && scopeManager.extendedScopeListeners.isEmpty()) { + return; + } AgentSpan span = span(); if (span == null) { return; } - for (final ScopeListener listener : scopeManager.scopeListeners) { - try { - listener.afterScopeActivated(); - } catch (Throwable e) { - ContinuableScopeManager.log.debug("ScopeListener threw exception in afterActivated()", e); + if (!scopeManager.scopeListeners.isEmpty()) { + for (final ScopeListener listener : scopeManager.scopeListeners) { + try { + listener.afterScopeActivated(); + } catch (Throwable e) { + ContinuableScopeManager.log.debug("ScopeListener threw exception in afterActivated()", e); + } } } - - for (final ExtendedScopeListener listener : scopeManager.extendedScopeListeners) { - try { - listener.afterScopeActivated(span.getTraceId(), span.getSpanId()); - } catch (Throwable e) { - ContinuableScopeManager.log.debug( - "ExtendedScopeListener threw exception in afterActivated()", e); + if (!scopeManager.extendedScopeListeners.isEmpty()) { + for (final ExtendedScopeListener listener : scopeManager.extendedScopeListeners) { + try { + listener.afterScopeActivated(span.getTraceId(), span.getSpanId()); + } catch (Throwable e) { + ContinuableScopeManager.log.debug( + "ExtendedScopeListener threw exception in afterActivated()", e); + } } } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java index f67d3b5d39f..5d997f87b39 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java @@ -58,6 +58,8 @@ public final class ContinuableScopeManager implements ContextManager { private final int depthLimit; final HealthMetrics healthMetrics; private final ProfilingContextIntegration profilingContextIntegration; + private final boolean profilingEnabled; + private final boolean hasDepthLimit; /** * Constructor with NOOP Profiling and HealthMetrics implementations. @@ -81,12 +83,15 @@ public ContinuableScopeManager( final ProfilingContextIntegration profilingContextIntegration, final HealthMetrics healthMetrics) { this.depthLimit = depthLimit == 0 ? Integer.MAX_VALUE : depthLimit; + this.hasDepthLimit = this.depthLimit < Integer.MAX_VALUE; this.strictMode = strictMode; this.scopeListeners = new CopyOnWriteArrayList<>(); this.extendedScopeListeners = new CopyOnWriteArrayList<>(); this.healthMetrics = healthMetrics; this.tlsScopeStack = new ScopeStackThreadLocal(profilingContextIntegration); this.profilingContextIntegration = profilingContextIntegration; + this.profilingEnabled = + !(profilingContextIntegration instanceof ProfilingContextIntegration.NoOp); ContextManager.register(this); } @@ -135,11 +140,13 @@ private AgentScope activate( } // DQH - This check could go before the check above, since depth limit checking is fast - final int currentDepth = scopeStack.depth(); - if (depthLimit <= currentDepth) { - healthMetrics.onScopeStackOverflow(); - log.debug("Scope depth limit exceeded ({}). Returning NoopScope.", currentDepth); - return noopScope(); + if (hasDepthLimit) { + final int currentDepth = scopeStack.depth(); + if (depthLimit <= currentDepth) { + healthMetrics.onScopeStackOverflow(); + log.debug("Scope depth limit exceeded ({}). Returning NoopScope.", currentDepth); + return noopScope(); + } } assert span != null; @@ -170,11 +177,13 @@ private AgentScope activate(final Context context) { } // DQH - This check could go before the check above, since depth limit checking is fast - final int currentDepth = scopeStack.depth(); - if (depthLimit <= currentDepth) { - healthMetrics.onScopeStackOverflow(); - log.debug("Scope depth limit exceeded ({}). Returning NoopScope.", currentDepth); - return noopScope(); + if (hasDepthLimit) { + final int currentDepth = scopeStack.depth(); + if (depthLimit <= currentDepth) { + healthMetrics.onScopeStackOverflow(); + log.debug("Scope depth limit exceeded ({}). Returning NoopScope.", currentDepth); + return noopScope(); + } } assert context != null; @@ -263,7 +272,7 @@ public AgentScope activateNext(final AgentSpan span) { ScopeStack scopeStack = scopeStack(); final int currentDepth = scopeStack.depth(); - if (depthLimit <= currentDepth) { + if (hasDepthLimit && depthLimit <= currentDepth) { healthMetrics.onScopeStackOverflow(); log.debug("Scope depth limit exceeded ({}). Returning NoopScope.", currentDepth); return noopScope(); @@ -341,6 +350,9 @@ private void addExtendedScopeListener(final ExtendedScopeListener listener) { } private Stateful createScopeState(Context context) { + if (!profilingEnabled) { + return Stateful.DEFAULT; + } // currently this just manages things the profiler has to do per scope, but could be expanded // to encapsulate other scope lifecycle activities // FIXME DDSpanContext is always a ProfilerContext anyway... From 1e13d53f58030ee7123dfa9f058b9468779a1882 Mon Sep 17 00:00:00 2001 From: bm1549 Date: Fri, 10 Apr 2026 08:50:20 -0400 Subject: [PATCH 2/3] Fix activeSpanLookup benchmark to exercise live scope stack Co-Authored-By: Claude Opus 4.6 (1M context) --- .../jmh/java/datadog/trace/core/ScopeLifecycleBenchmark.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/ScopeLifecycleBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/ScopeLifecycleBenchmark.java index d3b600e2a34..1a1de2bc795 100644 --- a/dd-trace-core/src/jmh/java/datadog/trace/core/ScopeLifecycleBenchmark.java +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/ScopeLifecycleBenchmark.java @@ -32,15 +32,18 @@ public class ScopeLifecycleBenchmark { public static class ThreadState { AgentSpan span; AgentSpan childSpan; + AgentScope activeScope; @Setup(Level.Iteration) public void setup() { span = TRACER.startSpan("benchmark", "parent"); childSpan = TRACER.startSpan("benchmark", "child"); + activeScope = TRACER.activateSpan(span); } @TearDown(Level.Iteration) public void tearDown() { + activeScope.close(); childSpan.finish(); span.finish(); } From f21e7d43dc30a00ce64f715e92a3917ac961fd14 Mon Sep 17 00:00:00 2001 From: bm1549 Date: Fri, 10 Apr 2026 10:14:16 -0400 Subject: [PATCH 3/3] Cache isEmpty() results in local variables to avoid redundant volatile reads Addresses review feedback from dougqh: the JIT will be conservative with volatile reads from CopyOnWriteArrayList.isEmpty() and won't hoist them, so we cache in locals ourselves. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../core/scopemanager/ContinuableScope.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java index 60cd1fbf483..d4e7e332586 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java @@ -80,7 +80,12 @@ void cleanup(final ScopeStack scopeStack) { * I would hope this becomes unnecessary. */ final void onProperClose() { - if (!scopeManager.scopeListeners.isEmpty()) { + boolean hasScopeListeners = !scopeManager.scopeListeners.isEmpty(); + boolean hasExtendedListeners = !scopeManager.extendedScopeListeners.isEmpty(); + if (!hasScopeListeners && !hasExtendedListeners) { + return; + } + if (hasScopeListeners) { for (final ScopeListener listener : scopeManager.scopeListeners) { try { listener.afterScopeClosed(); @@ -89,7 +94,7 @@ final void onProperClose() { } } } - if (!scopeManager.extendedScopeListeners.isEmpty()) { + if (hasExtendedListeners) { for (final ExtendedScopeListener listener : scopeManager.extendedScopeListeners) { try { listener.afterScopeClosed(); @@ -173,14 +178,16 @@ public final void beforeActivated() { } public final void afterActivated() { - if (scopeManager.scopeListeners.isEmpty() && scopeManager.extendedScopeListeners.isEmpty()) { + boolean hasScopeListeners = !scopeManager.scopeListeners.isEmpty(); + boolean hasExtendedListeners = !scopeManager.extendedScopeListeners.isEmpty(); + if (!hasScopeListeners && !hasExtendedListeners) { return; } AgentSpan span = span(); if (span == null) { return; } - if (!scopeManager.scopeListeners.isEmpty()) { + if (hasScopeListeners) { for (final ScopeListener listener : scopeManager.scopeListeners) { try { listener.afterScopeActivated(); @@ -189,7 +196,7 @@ public final void afterActivated() { } } } - if (!scopeManager.extendedScopeListeners.isEmpty()) { + if (hasExtendedListeners) { for (final ExtendedScopeListener listener : scopeManager.extendedScopeListeners) { try { listener.afterScopeActivated(span.getTraceId(), span.getSpanId());