From b80b1ae58ba2fde9e1ca30813e0789af8fba7d9b Mon Sep 17 00:00:00 2001 From: Dudi Keleti Date: Fri, 8 Nov 2024 00:42:35 +0100 Subject: [PATCH] [Dynamic Instrumentation] DEBUG-3088 Add object pool (#6105) ## Summary of changes Add object pool (IPoolable) to use in probe processor and snapshot creator. ## Implementation details Object pool with size limitation. --- .../TrackedStackFrameNode.cs | 4 +- .../Expressions/MethodScopeMembers.cs | 43 ++-- .../MethodScopeMembersParameters.cs | 9 + .../Debugger/Expressions/ProbeProcessor.cs | 2 +- .../Debugger/Helpers/IPoolable.cs | 15 ++ .../Debugger/Helpers/ObjectPool.cs | 51 +++++ .../Debugger/RateLimiting/ProbeRateLimiter.cs | 10 - .../Snapshots/DebuggerSnapshotCreator.cs | 12 +- .../DebuggerExpressionLanguageTests.cs | 4 +- .../Debugger/ObjectPoolTests.cs | 203 ++++++++++++++++++ 10 files changed, 320 insertions(+), 33 deletions(-) create mode 100644 tracer/src/Datadog.Trace/Debugger/Expressions/MethodScopeMembersParameters.cs create mode 100644 tracer/src/Datadog.Trace/Debugger/Helpers/IPoolable.cs create mode 100644 tracer/src/Datadog.Trace/Debugger/Helpers/ObjectPool.cs create mode 100644 tracer/test/Datadog.Trace.Tests/Debugger/ObjectPoolTests.cs diff --git a/tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/TrackedStackFrameNode.cs b/tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/TrackedStackFrameNode.cs index 8dc8cef7fd8e..7afccd2d91ca 100644 --- a/tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/TrackedStackFrameNode.cs +++ b/tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/TrackedStackFrameNode.cs @@ -124,7 +124,7 @@ private string CreateSnapshot() if (members == null) { - members = new MethodScopeMembers(0, 0); + members = new MethodScopeMembers(new MethodScopeMembersParameters(0, 0)); } var limitInfo = new CaptureLimitInfo( @@ -164,7 +164,7 @@ internal void AddScopeMember(string name, Type type, T value, ScopeMemberKind { if (Members == null) { - Members = new MethodScopeMembers(0, 0); + Members = new MethodScopeMembers(new MethodScopeMembersParameters(0, 0)); } type = (type.IsGenericTypeDefinition ? value?.GetType() : type) ?? type; diff --git a/tracer/src/Datadog.Trace/Debugger/Expressions/MethodScopeMembers.cs b/tracer/src/Datadog.Trace/Debugger/Expressions/MethodScopeMembers.cs index 9f9fdb4bbbff..78bc087d88ae 100644 --- a/tracer/src/Datadog.Trace/Debugger/Expressions/MethodScopeMembers.cs +++ b/tracer/src/Datadog.Trace/Debugger/Expressions/MethodScopeMembers.cs @@ -9,24 +9,17 @@ namespace Datadog.Trace.Debugger.Expressions; -internal class MethodScopeMembers +internal class MethodScopeMembers : IPoolable { - private readonly int _initialSize; private int _index; - internal MethodScopeMembers(int numberOfLocals, int numberOfArguments) + public MethodScopeMembers() { - _initialSize = numberOfLocals + numberOfArguments; - if (_initialSize == 0) - { - _initialSize = 1; - } + } - Members = ArrayPool.Shared.Rent(_initialSize); - Array.Clear(Members, 0, Members.Length); - Exception = null; - Return = default; - InvocationTarget = default; + internal MethodScopeMembers(MethodScopeMembersParameters parameters) + { + Set(parameters); } internal ScopeMember[] Members { get; private set; } @@ -34,7 +27,7 @@ internal MethodScopeMembers(int numberOfLocals, int numberOfArguments) internal Exception Exception { get; set; } // food for thought: - // we can save Return and InvocationTarget as T if we will change the native side so we will have MethodDebuggerState instead MethodDebuggerState. + // we can save Return and InvocationTarget as T if we will change the native side, so we will have MethodDebuggerState instead MethodDebuggerState. internal ScopeMember Return { get; set; } internal ScopeMember InvocationTarget { get; set; } @@ -59,4 +52,26 @@ internal void Dispose() ArrayPool.Shared.Return(Members); } } + + public void Set(MethodScopeMembersParameters parameters) + { + var initialSize = parameters.NumberOfLocals + parameters.NumberOfArguments; + if (initialSize == 0) + { + initialSize = 1; + } + + Members = ArrayPool.Shared.Rent(initialSize * 2); + Array.Clear(Members, 0, Members.Length); + Exception = null; + Duration = default; + Return = default; + InvocationTarget = default; + _index = 0; + } + + public void Reset() + { + Dispose(); + } } diff --git a/tracer/src/Datadog.Trace/Debugger/Expressions/MethodScopeMembersParameters.cs b/tracer/src/Datadog.Trace/Debugger/Expressions/MethodScopeMembersParameters.cs new file mode 100644 index 000000000000..76cdc196e417 --- /dev/null +++ b/tracer/src/Datadog.Trace/Debugger/Expressions/MethodScopeMembersParameters.cs @@ -0,0 +1,9 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +namespace Datadog.Trace.Debugger.Expressions; + +#nullable enable +internal record struct MethodScopeMembersParameters(int NumberOfLocals, int NumberOfArguments); diff --git a/tracer/src/Datadog.Trace/Debugger/Expressions/ProbeProcessor.cs b/tracer/src/Datadog.Trace/Debugger/Expressions/ProbeProcessor.cs index 96aacaae9aad..b03596daa5cc 100644 --- a/tracer/src/Datadog.Trace/Debugger/Expressions/ProbeProcessor.cs +++ b/tracer/src/Datadog.Trace/Debugger/Expressions/ProbeProcessor.cs @@ -160,7 +160,7 @@ private ProbeExpressionEvaluator GetOrCreateEvaluator() public bool ShouldProcess(in ProbeData probeData) { - return HasCondition() || probeData.Sampler.Sample(); + return HasCondition() || (probeData.Sampler.Sample()); } public bool Process(ref CaptureInfo info, IDebuggerSnapshotCreator inSnapshotCreator, in ProbeData probeData) diff --git a/tracer/src/Datadog.Trace/Debugger/Helpers/IPoolable.cs b/tracer/src/Datadog.Trace/Debugger/Helpers/IPoolable.cs new file mode 100644 index 000000000000..a51d5577a7bf --- /dev/null +++ b/tracer/src/Datadog.Trace/Debugger/Helpers/IPoolable.cs @@ -0,0 +1,15 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#nullable enable +namespace Datadog.Trace.Debugger.Helpers +{ + internal interface IPoolable + { + void Set(TSetParameters parameters); + + void Reset(); + } +} diff --git a/tracer/src/Datadog.Trace/Debugger/Helpers/ObjectPool.cs b/tracer/src/Datadog.Trace/Debugger/Helpers/ObjectPool.cs new file mode 100644 index 000000000000..6ee567906e54 --- /dev/null +++ b/tracer/src/Datadog.Trace/Debugger/Helpers/ObjectPool.cs @@ -0,0 +1,51 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#nullable enable +using System; +using System.Collections.Concurrent; + +namespace Datadog.Trace.Debugger.Helpers +{ + internal class ObjectPool + where T : class, IPoolable, new() + { + private readonly ConcurrentBag _objects; + private readonly Func _objectFactory; + private readonly int _maxSize; + + public ObjectPool(Func? objectFactory = null, int maxSize = 100) + { + if (maxSize <= 0) + { + throw new ArgumentException("Maximum pool size must be greater than zero.", nameof(maxSize)); + } + + _objects = new ConcurrentBag(); + _objectFactory = objectFactory ?? (() => new T()); + _maxSize = maxSize; + } + + public int Count => _objects.Count; + + public T? Get() => _objects.TryTake(out var item) ? item : _objectFactory(); + + public T? Get(TSetParameters parameters) + { + var item = _objects.TryTake(out var obj) ? obj : _objectFactory(); + item?.Set(parameters); + return item; + } + + public void Return(T? item) + { + item?.Reset(); + if (item != null && _objects.Count < _maxSize) + { + _objects.Add(item); + } + } + } +} diff --git a/tracer/src/Datadog.Trace/Debugger/RateLimiting/ProbeRateLimiter.cs b/tracer/src/Datadog.Trace/Debugger/RateLimiting/ProbeRateLimiter.cs index 090e1428f393..673dae6673a7 100644 --- a/tracer/src/Datadog.Trace/Debugger/RateLimiting/ProbeRateLimiter.cs +++ b/tracer/src/Datadog.Trace/Debugger/RateLimiting/ProbeRateLimiter.cs @@ -13,7 +13,6 @@ namespace Datadog.Trace.Debugger.RateLimiting internal class ProbeRateLimiter { private const int DefaultSamplesPerSecond = 1; - private const int DefaultGlobalSamplesPerSecond = DefaultSamplesPerSecond * 100; private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(ProbeRateLimiter)); @@ -23,8 +22,6 @@ internal class ProbeRateLimiter private static ProbeRateLimiter _instance; - private readonly AdaptiveSampler _globalSampler = CreateSampler(DefaultGlobalSamplesPerSecond); - private readonly ConcurrentDictionary _samplers = new(); internal static ProbeRateLimiter Instance @@ -51,13 +48,6 @@ public bool TryAddSampler(string probeId, IAdaptiveSampler sampler) return _samplers.TryAdd(probeId, sampler); } - public bool Sample(string probeId) - { - // Rate limiter is engaged at ~1 probe per second (1 probes per 1s time window) - var probeSampler = _samplers.GetOrAdd(probeId, _ => CreateSampler(1)); - return probeSampler.Sample() && _globalSampler.Sample(); - } - public void SetRate(string probeId, int samplesPerSecond) { // We currently don't support updating the probe rate limit, and that is fine diff --git a/tracer/src/Datadog.Trace/Debugger/Snapshots/DebuggerSnapshotCreator.cs b/tracer/src/Datadog.Trace/Debugger/Snapshots/DebuggerSnapshotCreator.cs index 51563eeb89be..5c89732e6bc0 100644 --- a/tracer/src/Datadog.Trace/Debugger/Snapshots/DebuggerSnapshotCreator.cs +++ b/tracer/src/Datadog.Trace/Debugger/Snapshots/DebuggerSnapshotCreator.cs @@ -40,6 +40,7 @@ internal class DebuggerSnapshotCreator : IDebuggerSnapshotCreator, IDisposable private string _message; private List _errors; private string _snapshotId; + private ObjectPool _scopeMembersPool; public DebuggerSnapshotCreator(bool isFullSnapshot, ProbeLocation location, bool hasCondition, string[] tags, CaptureLimitInfo limitInfo) { @@ -55,6 +56,7 @@ public DebuggerSnapshotCreator(bool isFullSnapshot, ProbeLocation location, bool Tags = tags; _limitInfo = limitInfo; _accumulatedDuration = new TimeSpan(0, 0, 0, 0, 0); + _scopeMembersPool = new ObjectPool(); Initialize(); } @@ -182,11 +184,14 @@ internal void CreateMethodScopeMembers(ref CaptureInfo info) { if (info.IsAsyncCapture()) { - MethodScopeMembers = new MethodScopeMembers(info.AsyncCaptureInfo.HoistedLocals.Length + (info.LocalsCount ?? 0), info.AsyncCaptureInfo.HoistedArguments.Length + (info.ArgumentsCount ?? 0)); + MethodScopeMembers = _scopeMembersPool.Get( + new MethodScopeMembersParameters( + info.AsyncCaptureInfo.HoistedLocals.Length + (info.LocalsCount ?? 0), + info.AsyncCaptureInfo.HoistedArguments.Length + (info.ArgumentsCount ?? 0))); } else { - MethodScopeMembers = new MethodScopeMembers(info.LocalsCount.Value, info.ArgumentsCount.Value); + MethodScopeMembers = _scopeMembersPool.Get(new MethodScopeMembersParameters(info.LocalsCount.Value, info.ArgumentsCount.Value)); } } @@ -901,8 +906,7 @@ public void Dispose() try { Stop(); - MethodScopeMembers?.Dispose(); - MethodScopeMembers = null; + _scopeMembersPool.Return(MethodScopeMembers); _jsonWriter?.Close(); } catch diff --git a/tracer/test/Datadog.Trace.Tests/Debugger/DebuggerExpressionLanguageTests.cs b/tracer/test/Datadog.Trace.Tests/Debugger/DebuggerExpressionLanguageTests.cs index d735a7b9570f..b682dd0c8719 100644 --- a/tracer/test/Datadog.Trace.Tests/Debugger/DebuggerExpressionLanguageTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Debugger/DebuggerExpressionLanguageTests.cs @@ -192,8 +192,8 @@ private string GetJsonPart(string json) private MethodScopeMembers CreateScopeMembers() { - var scope = new MethodScopeMembers(10, 5); - + var scope = new MethodScopeMembers(); + scope.Set(new MethodScopeMembersParameters(10, 5)); // Add locals scope.AddMember(new ScopeMember("IntLocal", TestObject.IntNumber.GetType(), TestObject.IntNumber, ScopeMemberKind.Local)); scope.AddMember(new ScopeMember("DoubleLocal", TestObject.DoubleNumber.GetType(), TestObject.DoubleNumber, ScopeMemberKind.Local)); diff --git a/tracer/test/Datadog.Trace.Tests/Debugger/ObjectPoolTests.cs b/tracer/test/Datadog.Trace.Tests/Debugger/ObjectPoolTests.cs new file mode 100644 index 000000000000..133809ccb956 --- /dev/null +++ b/tracer/test/Datadog.Trace.Tests/Debugger/ObjectPoolTests.cs @@ -0,0 +1,203 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +using System; +using System.Threading.Tasks; +using Datadog.Trace.Debugger.Helpers; +using FluentAssertions; +using Xunit; + +#nullable enable + +namespace Datadog.Trace.Tests.Debugger +{ + public class ObjectPoolTests + { + [Fact] + public void Constructor_WithDefaultParameters_CreatesEmptyPool() + { + // Arrange & Act + var pool = new ObjectPool(); + + // Assert + Assert.Equal(0, pool.Count); + } + + [Fact] + public void Constructor_WithNegativeMaxSize_ThrowsArgumentException() + { + // Arrange, Act & Assert + Assert.Throws(() => new ObjectPool(maxSize: -1)); + } + + [Fact] + public void Get_WhenPoolEmpty_CreatesNewInstance() + { + // Arrange + var pool = new ObjectPool(); + + // Act + var item = pool.Get(); + + // Assert + item.Should().NotBeNull(); + pool.Count.Should().Be(0); + } + + [Fact] + public void Get_WithParameters_SetsParametersCorrectly() + { + // Arrange + var pool = new ObjectPool(); + const string testValue = "test"; + + // Act + var item = pool.Get(testValue); + + // Assert + item.Should().NotBeNull(); + item!.CurrentValue.Should().Be(testValue); + } + + [Fact] + public void Return_WhenBelowMaxSize_AddsToPool() + { + // Arrange + var pool = new ObjectPool(); + var item = pool.Get(); + + // Act + pool.Return(item); + + // Assert + pool.Count.Should().Be(1); + } + + [Fact] + public void Return_WhenAtMaxSize_DoesNotAddToPool() + { + // Arrange + var pool = new ObjectPool(maxSize: 1); + var item1 = pool.Get(); + var item2 = pool.Get(); + + // Act + pool.Return(item1); + pool.Return(item2); + + // Assert + pool.Count.Should().Be(1); + } + + [Fact] + public void Return_ResetsItem() + { + // Arrange + var pool = new ObjectPool(); + var item = pool.Get("test"); + + // Act + pool.Return(item); + + // Assert + item.Should().NotBeNull(); + item!.WasReset.Should().BeTrue(); + item.CurrentValue.Should().BeNull(); + } + + [Fact] + public void Get_ReturnsPooledInstance_WhenAvailable() + { + // Arrange + var pool = new ObjectPool(); + var item = pool.Get(); + pool.Return(item); + + // Act + var retrievedItem = pool.Get(); + + // Assert + retrievedItem.Should().BeSameAs(retrievedItem); + pool.Count.Should().Be(0); + } + + [Fact] + public void Get_WithCustomFactory_UsesFactoryToCreateInstances() + { + // Arrange + var factoryCallCount = 0; + var factory = new Func( + () => + { + factoryCallCount++; + return new TestPoolable(); + }); + + var pool = new ObjectPool(factory); + + // Act + var item = pool.Get(); + + // Assert + item.Should().NotBeNull(); + factoryCallCount.Should().Be(1); + } + + [Fact] + public void Return_WithNullItem_DoesNotThrowException() + { + // Arrange + var pool = new ObjectPool(); + + // Act & Assert + var exception = Record.Exception(() => pool.Return(null)); + exception.Should().BeNull(); + } + + [Fact] + public async Task ConcurrentAccess_IsThreadSafe() + { + // Arrange + var pool = new ObjectPool(); + const int operationCount = 1000; + var tasks = new Task[operationCount]; + + // Act + for (var i = 0; i < operationCount; i++) + { + tasks[i] = Task.Run( + () => + { + var item = pool.Get(); + pool.Return(item); + }); + } + + // Assert + await Task.WhenAll(tasks); + pool.Count.Should().BeLessThanOrEqualTo(operationCount); + pool.Count.Should().BeLessThanOrEqualTo(100); // Default max size + } + + private class TestPoolable : IPoolable + { + public string? CurrentValue { get; private set; } + + public bool WasReset { get; private set; } + + public void Set(string parameters) + { + CurrentValue = parameters; + WasReset = false; + } + + public void Reset() + { + CurrentValue = null; + WasReset = true; + } + } + } +}