Optimize scope lifecycle hot path

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

@@ -0,0 +1,78 @@
+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;
+    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();
+    }
+  }
+
+  @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();
+  }
+}

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

@@ -80,19 +80,27 @@ 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);
+    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();
+        } 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 (hasExtendedListeners) {
+      for (final ExtendedScopeListener listener : scopeManager.extendedScopeListeners) {
+        try {
+          listener.afterScopeClosed();
+        } catch (Exception e) {
+          ContinuableScopeManager.log.debug("ScopeListener threw exception in close()", e);
+        }
       }
     }
   }
@@ -154,6 +162,9 @@ public boolean rollback() {
   }
 
   public final void beforeActivated() {
+    if (scopeState == Stateful.DEFAULT) {
+      return;
+    }
     AgentSpan span = span();
     if (span == null) {
       return;
@@ -167,24 +178,32 @@ public final void beforeActivated() {
   }
 
   public final void afterActivated() {
+    boolean hasScopeListeners = !scopeManager.scopeListeners.isEmpty();
+    boolean hasExtendedListeners = !scopeManager.extendedScopeListeners.isEmpty();
+    if (!hasScopeListeners && !hasExtendedListeners) {
+      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 (hasScopeListeners) {
+      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 (hasExtendedListeners) {
+      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);
+        }
       }
     }
   }

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...