Added memory optimisations for GetLastActiveSpan (#2642)

getsentry · Sep 27, 2023 · 5cb67de · 5cb67de
1 parent cecb99d
commit 5cb67de
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -36,6 +36,7 @@ API Changes:
 - Added distributed tracing without performance for Azure Function Workers ([#2630](https://github.com/getsentry/sentry-dotnet/pull/2630))
 - The SDK now provides and overload of `ContinueTrace` that accepts headers as `string` ([#2601](https://github.com/getsentry/sentry-dotnet/pull/2601))
 - Sentry tracing middleware now gets configured automatically ([#2602](https://github.com/getsentry/sentry-dotnet/pull/2602))
+- Added memory optimisations for GetLastActiveSpan ([#2642](https://github.com/getsentry/sentry-dotnet/pull/2642))
 
 ### Fixes
 

diff --git a/...t.Artifacts/results/Sentry.Benchmarks.LastActiveSpanBenchmarks-report-github.md b/...t.Artifacts/results/Sentry.Benchmarks.LastActiveSpanBenchmarks-report-github.md
@@ -0,0 +1,17 @@
+``` ini
+
+BenchmarkDotNet=v0.13.5, OS=macOS Ventura 13.5.2 (22G91) [Darwin 22.6.0]
+Apple M1, 1 CPU, 8 logical and 8 physical cores
+.NET SDK=7.0.306
+  [Host]   : .NET 6.0.19 (6.0.1923.31806), Arm64 RyuJIT AdvSIMD
+  ShortRun : .NET 6.0.19 (6.0.1923.31806), Arm64 RyuJIT AdvSIMD
+
+Job=ShortRun  IterationCount=3  LaunchCount=1  
+WarmupCount=3  
+
+```
+|                          Method | SpanCount |      Mean |       Error |    StdDev |    Gen0 |    Gen1 | Allocated |
+|-------------------------------- |---------- |----------:|------------:|----------:|--------:|--------:|----------:|
+| **&#39;Create spans for scope access&#39;** |         **1** |  **32.34 μs** |    **26.58 μs** |  **1.457 μs** |  **4.7607** |  **1.5259** |  **16.68 KB** |
+| **&#39;Create spans for scope access&#39;** |        **10** | **101.28 μs** |   **239.09 μs** | **13.105 μs** |  **9.1553** |  **2.5635** |  **39.21 KB** |
+| **&#39;Create spans for scope access&#39;** |       **100** | **628.84 μs** | **1,267.56 μs** | **69.479 μs** | **59.5703** | **18.5547** | **266.14 KB** |
diff --git a/benchmarks/Sentry.Benchmarks/LastActiveSpanBenchmarks.cs b/benchmarks/Sentry.Benchmarks/LastActiveSpanBenchmarks.cs
@@ -0,0 +1,57 @@
+using BenchmarkDotNet.Attributes;
+using Perfolizer.Mathematics.Randomization;
+
+namespace Sentry.Benchmarks;
+
+public class LastActiveSpanBenchmarks
+{
+    private const string Operation = "Operation";
+    private const string Name = "Name";
+
+    [Params(1, 10, 100)]
+    public int SpanCount;
+
+    private IDisposable _sdk;
+
+    [GlobalSetup(Target = nameof(CreateScopedSpans))]
+    public void EnabledSdk() => _sdk = SentrySdk.Init(Constants.ValidDsn);
+
+    [GlobalCleanup(Target = nameof(CreateScopedSpans))]
+    public void DisableDsk() => _sdk.Dispose();
+
+    [Benchmark(Description = "Create spans for scope access")]
+    public void CreateScopedSpans()
+    {
+        var transaction = SentrySdk.StartTransaction(Name, Operation);
+        SentrySdk.WithScope(scope =>
+        {
+            scope.Transaction = transaction;
+            for (var i = 0; i < SpanCount; i++)
+            {
+                // Simulates a scenario where TransactionTracer.GetLastActiveSpan will be called frequently
+                // See: https://github.com/getsentry/sentry-dotnet/blob/c2a31b4ead03da388c2db7fe07f290354aa51b9d/src/Sentry/Scope.cs#L567C1-L567C68
+                CallOneFunction(i);
+            }
+        });
+        transaction.Finish();
+    }
+
+    private void CallOneFunction(int i)
+    {
+        var span = SentrySdk.GetSpan()!.StartChild($"One Function {i}");
+        ThatCallsAnother(i);
+        span.Finish();
+    }
+
+    private void ThatCallsAnother(int i)
+    {
+        var span = SentrySdk.GetSpan()!.StartChild($"Another Function {i}");
+        AndAnother($"Alternate Description {i}");
+        span.Finish();
+    }
+
+    private void AndAnother(string description)
+    {
+        SentrySdk.ConfigureScope(scope => scope.Span!.Description = description);
+    }
+}
diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs
@@ -313,9 +313,48 @@ private void AddChildSpan(SpanTracer span)
         if (!isOutOfLimit)
         {
             _spans.Add(span);
+            _activeSpanTracker.Push(span);
         }
     }
 
+    private class LastActiveSpanTracker
+    {
+        private readonly object _lock = new object();
+
+        private readonly Lazy<Stack<ISpan>> _trackedSpans = new();
+        private Stack<ISpan> TrackedSpans => _trackedSpans.Value;
+
+        public void Push(ISpan span)
+        {
+            lock(_lock)
+            {
+                TrackedSpans.Push(span);
+            }
+        }
+
+        public ISpan? PeekActive()
+        {
+            lock(_lock)
+            {
+                while (TrackedSpans.Count > 0)
+                {
+                    // Stop tracking inactive spans
+                    var span = TrackedSpans.Peek();
+                    if (!span.IsFinished)
+                    {
+                        return span;
+                    }
+                    TrackedSpans.Pop();
+                }
+                return null;
+            }
+        }
+    }
+    private readonly LastActiveSpanTracker _activeSpanTracker = new LastActiveSpanTracker();
+
+    /// <inheritdoc />
+    public ISpan? GetLastActiveSpan() => _activeSpanTracker.PeekActive();
+
     /// <inheritdoc />
     public void Finish()
     {
@@ -375,11 +414,6 @@ public void Finish(Exception exception, SpanStatus status)
     public void Finish(Exception exception) =>
         Finish(exception, SpanStatusConverter.FromException(exception));
 
-    /// <inheritdoc />
-    public ISpan? GetLastActiveSpan() =>
-        // We need to sort by timestamp because the order of ConcurrentBag<T> is not deterministic
-        Spans.OrderByDescending(x => x.StartTimestamp).FirstOrDefault(s => !s.IsFinished);
-
     /// <inheritdoc />
     public SentryTraceHeader GetTraceHeader() => new(TraceId, SpanId, IsSampled);
 }