Skip to content

Commit 4666c89

Browse files
bm1549devflow.devflow-routing-intake
bm1549
and
devflow.devflow-routing-intake
authored
Optimize scope lifecycle hot path (#11079)
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) <noreply@anthropic.com> Fix activeSpanLookup benchmark to exercise live scope stack Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> 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) <noreply@anthropic.com> Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 81bb305 commit 4666c89

3 files changed

Lines changed: 143 additions & 34 deletions

File tree

‎dd-trace-core/src/jmh/java/datadog/trace/core/ScopeLifecycleBenchmark.java‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package datadog.trace.core;
2+
3+
import static java.util.concurrent.TimeUnit.MICROSECONDS;
4+
5+
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
6+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
7+
import org.openjdk.jmh.annotations.Benchmark;
8+
import org.openjdk.jmh.annotations.BenchmarkMode;
9+
import org.openjdk.jmh.annotations.Fork;
10+
import org.openjdk.jmh.annotations.Level;
11+
import org.openjdk.jmh.annotations.Measurement;
12+
import org.openjdk.jmh.annotations.Mode;
13+
import org.openjdk.jmh.annotations.OutputTimeUnit;
14+
import org.openjdk.jmh.annotations.Scope;
15+
import org.openjdk.jmh.annotations.Setup;
16+
import org.openjdk.jmh.annotations.State;
17+
import org.openjdk.jmh.annotations.TearDown;
18+
import org.openjdk.jmh.annotations.Threads;
19+
import org.openjdk.jmh.annotations.Warmup;
20+
21+
@State(Scope.Benchmark)
22+
@Warmup(iterations = 3)
23+
@Measurement(iterations = 5)
24+
@BenchmarkMode(Mode.Throughput)
25+
@Threads(8)
26+
@OutputTimeUnit(MICROSECONDS)
27+
@Fork(value = 1)
28+
public class ScopeLifecycleBenchmark {
29+
static final CoreTracer TRACER = CoreTracer.builder().build();
30+
31+
@State(Scope.Thread)
32+
public static class ThreadState {
33+
AgentSpan span;
34+
AgentSpan childSpan;
35+
AgentScope activeScope;
36+
37+
@Setup(Level.Iteration)
38+
public void setup() {
39+
span = TRACER.startSpan("benchmark", "parent");
40+
childSpan = TRACER.startSpan("benchmark", "child");
41+
activeScope = TRACER.activateSpan(span);
42+
}
43+
44+
@TearDown(Level.Iteration)
45+
public void tearDown() {
46+
activeScope.close();
47+
childSpan.finish();
48+
span.finish();
49+
}
50+
}
51+
52+
@Benchmark
53+
public void activateAndClose(ThreadState state) {
54+
AgentScope scope = TRACER.activateSpan(state.span);
55+
scope.close();
56+
}
57+
58+
@Benchmark
59+
public void activateSameSpan(ThreadState state) {
60+
AgentScope outer = TRACER.activateSpan(state.span);
61+
AgentScope inner = TRACER.activateSpan(state.span);
62+
inner.close();
63+
outer.close();
64+
}
65+
66+
@Benchmark
67+
public void nestedActivateAndClose(ThreadState state) {
68+
AgentScope parentScope = TRACER.activateSpan(state.span);
69+
AgentScope childScope = TRACER.activateSpan(state.childSpan);
70+
childScope.close();
71+
parentScope.close();
72+
}
73+
74+
@Benchmark
75+
public AgentSpan activeSpanLookup() {
76+
return TRACER.activeSpan();
77+
}
78+
}

‎dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java‎

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,27 @@ void cleanup(final ScopeStack scopeStack) {
8080
* I would hope this becomes unnecessary.
8181
*/
8282
final void onProperClose() {
83-
for (final ScopeListener listener : scopeManager.scopeListeners) {
84-
try {
85-
listener.afterScopeClosed();
86-
} catch (Exception e) {
87-
ContinuableScopeManager.log.debug("ScopeListener threw exception in close()", e);
83+
boolean hasScopeListeners = !scopeManager.scopeListeners.isEmpty();
84+
boolean hasExtendedListeners = !scopeManager.extendedScopeListeners.isEmpty();
85+
if (!hasScopeListeners && !hasExtendedListeners) {
86+
return;
87+
}
88+
if (hasScopeListeners) {
89+
for (final ScopeListener listener : scopeManager.scopeListeners) {
90+
try {
91+
listener.afterScopeClosed();
92+
} catch (Exception e) {
93+
ContinuableScopeManager.log.debug("ScopeListener threw exception in close()", e);
94+
}
8895
}
8996
}
90-
91-
for (final ExtendedScopeListener listener : scopeManager.extendedScopeListeners) {
92-
try {
93-
listener.afterScopeClosed();
94-
} catch (Exception e) {
95-
ContinuableScopeManager.log.debug("ScopeListener threw exception in close()", e);
97+
if (hasExtendedListeners) {
98+
for (final ExtendedScopeListener listener : scopeManager.extendedScopeListeners) {
99+
try {
100+
listener.afterScopeClosed();
101+
} catch (Exception e) {
102+
ContinuableScopeManager.log.debug("ScopeListener threw exception in close()", e);
103+
}
96104
}
97105
}
98106
}
@@ -154,6 +162,9 @@ public boolean rollback() {
154162
}
155163

156164
public final void beforeActivated() {
165+
if (scopeState == Stateful.DEFAULT) {
166+
return;
167+
}
157168
AgentSpan span = span();
158169
if (span == null) {
159170
return;
@@ -167,24 +178,32 @@ public final void beforeActivated() {
167178
}
168179

169180
public final void afterActivated() {
181+
boolean hasScopeListeners = !scopeManager.scopeListeners.isEmpty();
182+
boolean hasExtendedListeners = !scopeManager.extendedScopeListeners.isEmpty();
183+
if (!hasScopeListeners && !hasExtendedListeners) {
184+
return;
185+
}
170186
AgentSpan span = span();
171187
if (span == null) {
172188
return;
173189
}
174-
for (final ScopeListener listener : scopeManager.scopeListeners) {
175-
try {
176-
listener.afterScopeActivated();
177-
} catch (Throwable e) {
178-
ContinuableScopeManager.log.debug("ScopeListener threw exception in afterActivated()", e);
190+
if (hasScopeListeners) {
191+
for (final ScopeListener listener : scopeManager.scopeListeners) {
192+
try {
193+
listener.afterScopeActivated();
194+
} catch (Throwable e) {
195+
ContinuableScopeManager.log.debug("ScopeListener threw exception in afterActivated()", e);
196+
}
179197
}
180198
}
181-
182-
for (final ExtendedScopeListener listener : scopeManager.extendedScopeListeners) {
183-
try {
184-
listener.afterScopeActivated(span.getTraceId(), span.getSpanId());
185-
} catch (Throwable e) {
186-
ContinuableScopeManager.log.debug(
187-
"ExtendedScopeListener threw exception in afterActivated()", e);
199+
if (hasExtendedListeners) {
200+
for (final ExtendedScopeListener listener : scopeManager.extendedScopeListeners) {
201+
try {
202+
listener.afterScopeActivated(span.getTraceId(), span.getSpanId());
203+
} catch (Throwable e) {
204+
ContinuableScopeManager.log.debug(
205+
"ExtendedScopeListener threw exception in afterActivated()", e);
206+
}
188207
}
189208
}
190209
}

‎dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ public final class ContinuableScopeManager implements ContextManager {
5858
private final int depthLimit;
5959
final HealthMetrics healthMetrics;
6060
private final ProfilingContextIntegration profilingContextIntegration;
61+
private final boolean profilingEnabled;
62+
private final boolean hasDepthLimit;
6163

6264
/**
6365
* Constructor with NOOP Profiling and HealthMetrics implementations.
@@ -81,12 +83,15 @@ public ContinuableScopeManager(
8183
final ProfilingContextIntegration profilingContextIntegration,
8284
final HealthMetrics healthMetrics) {
8385
this.depthLimit = depthLimit == 0 ? Integer.MAX_VALUE : depthLimit;
86+
this.hasDepthLimit = this.depthLimit < Integer.MAX_VALUE;
8487
this.strictMode = strictMode;
8588
this.scopeListeners = new CopyOnWriteArrayList<>();
8689
this.extendedScopeListeners = new CopyOnWriteArrayList<>();
8790
this.healthMetrics = healthMetrics;
8891
this.tlsScopeStack = new ScopeStackThreadLocal(profilingContextIntegration);
8992
this.profilingContextIntegration = profilingContextIntegration;
93+
this.profilingEnabled =
94+
!(profilingContextIntegration instanceof ProfilingContextIntegration.NoOp);
9095

9196
ContextManager.register(this);
9297
}
@@ -135,11 +140,13 @@ private AgentScope activate(
135140
}
136141

137142
// DQH - This check could go before the check above, since depth limit checking is fast
138-
final int currentDepth = scopeStack.depth();
139-
if (depthLimit <= currentDepth) {
140-
healthMetrics.onScopeStackOverflow();
141-
log.debug("Scope depth limit exceeded ({}). Returning NoopScope.", currentDepth);
142-
return noopScope();
143+
if (hasDepthLimit) {
144+
final int currentDepth = scopeStack.depth();
145+
if (depthLimit <= currentDepth) {
146+
healthMetrics.onScopeStackOverflow();
147+
log.debug("Scope depth limit exceeded ({}). Returning NoopScope.", currentDepth);
148+
return noopScope();
149+
}
143150
}
144151

145152
assert span != null;
@@ -170,11 +177,13 @@ private AgentScope activate(final Context context) {
170177
}
171178

172179
// DQH - This check could go before the check above, since depth limit checking is fast
173-
final int currentDepth = scopeStack.depth();
174-
if (depthLimit <= currentDepth) {
175-
healthMetrics.onScopeStackOverflow();
176-
log.debug("Scope depth limit exceeded ({}). Returning NoopScope.", currentDepth);
177-
return noopScope();
180+
if (hasDepthLimit) {
181+
final int currentDepth = scopeStack.depth();
182+
if (depthLimit <= currentDepth) {
183+
healthMetrics.onScopeStackOverflow();
184+
log.debug("Scope depth limit exceeded ({}). Returning NoopScope.", currentDepth);
185+
return noopScope();
186+
}
178187
}
179188

180189
assert context != null;
@@ -263,7 +272,7 @@ public AgentScope activateNext(final AgentSpan span) {
263272
ScopeStack scopeStack = scopeStack();
264273

265274
final int currentDepth = scopeStack.depth();
266-
if (depthLimit <= currentDepth) {
275+
if (hasDepthLimit && depthLimit <= currentDepth) {
267276
healthMetrics.onScopeStackOverflow();
268277
log.debug("Scope depth limit exceeded ({}). Returning NoopScope.", currentDepth);
269278
return noopScope();
@@ -341,6 +350,9 @@ private void addExtendedScopeListener(final ExtendedScopeListener listener) {
341350
}
342351

343352
private Stateful createScopeState(Context context) {
353+
if (!profilingEnabled) {
354+
return Stateful.DEFAULT;
355+
}
344356
// currently this just manages things the profiler has to do per scope, but could be expanded
345357
// to encapsulate other scope lifecycle activities
346358
// FIXME DDSpanContext is always a ProfilerContext anyway...

0 commit comments

Comments
 (0)