Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
25f23d8
deps: update perfview (removes the .il suffix from profile module names)
jamescrosswell Aug 23, 2026
fad203f
fix: bound profiler memory growth by trimming interned call stacks
jamescrosswell Aug 23, 2026
9d90d21
Tweak comments
jamescrosswell Aug 24, 2026
05a1957
Fail loudly if the TraceLog reflection target moves
jamescrosswell Aug 24, 2026
ef4fa83
Merge deps/update-perfview into fix/5469-trim-live-session-state
jamescrosswell Aug 24, 2026
ab58925
Merge remote comment tweaks
jamescrosswell Aug 24, 2026
e3d582a
Apply suggestion from @jamescrosswell
jamescrosswell Aug 24, 2026
b7db5ae
Dispose the EventPipeEventSource in the test helper
jamescrosswell Aug 24, 2026
a656b43
Merge remote comment removal
jamescrosswell Aug 24, 2026
50c51ef
Merge deps/update-perfview
jamescrosswell Aug 24, 2026
a9f09c4
Commit the regenerated sample.etlx instead of building it at test time
jamescrosswell Aug 24, 2026
129412c
Merge deps/update-perfview
jamescrosswell Aug 24, 2026
6638df7
Trim on a generation counter instead of waiting for an idle window
jamescrosswell Aug 25, 2026
21be80c
Concise comments
jamescrosswell Aug 25, 2026
cac6367
Note that a trim costs the in-flight sample
jamescrosswell Aug 25, 2026
955f72e
Merge concise comments
jamescrosswell Aug 25, 2026
137714e
Stop the trim tests racing the dispatch thread
jamescrosswell Aug 25, 2026
1456cea
Merge main
jamescrosswell Aug 26, 2026
8da501d
Latch off trimming after a failure instead of retrying every sample
jamescrosswell Aug 31, 2026
908f26f
Make the call stack budget per-factory rather than a static
jamescrosswell Aug 31, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/Sentry.Profiling/SampleProfileBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@
internal class SampleProfileBuilder
{
private readonly SentryOptions _options;
private readonly SampleProfilerSession? _session;
private readonly TraceLog _traceLog;

// The trim generation this builder's _stackIndexes cache was populated against.
private int _trimGeneration;

// Output profile being built.
public readonly SampleProfile Profile = new();

Expand All @@ -32,14 +36,30 @@
// TODO make downsampling conditional once this is available: https://github.com/dotnet/runtime/issues/82939
private readonly Downsampler _downsampler = new();

// For a TraceLog read from a file, where nothing trims it and no generation tracking is needed.
public SampleProfileBuilder(SentryOptions options, TraceLog traceLog)
{
_options = options;
_traceLog = traceLog;
}

public SampleProfileBuilder(SentryOptions options, SampleProfilerSession session)
: this(options, session.TraceLog)
{
_session = session;
_trimGeneration = session.TrimGeneration;
}

internal void AddSample(TraceEvent data, double timestampMs)
{
// The interning tables have been discarded since we last looked so we have to
// invalidate the _stackIndexes cache (means stacks get walked again)
if (_session is { } session && _trimGeneration != session.TrimGeneration)
{
_trimGeneration = session.TrimGeneration;
_stackIndexes.Clear();
}

Check failure on line 61 in src/Sentry.Profiling/SampleProfileBuilder.cs

View check run for this annotation

@sentry/warden / warden: find-bugs

AddSample keeps processing after trim instead of skipping the invalidated sample

When trim generation changes, clear `_stackIndexes` and return immediately; the current event's CallStackIndex was invalidated by TrimLiveSessionState, and continuing can cache a bad stack under a recycled index.
Comment thread
jamescrosswell marked this conversation as resolved.

var thread = data.Thread();
if (thread is null || thread.ThreadIndex == ThreadIndex.Invalid)
{
Expand Down
17 changes: 17 additions & 0 deletions src/Sentry.Profiling/SampleProfilerSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,29 @@ private SampleProfilerSession(SentryStopwatch stopwatch, EventPipeSession sessio

public TraceLog TraceLog => EventSource.TraceLog;

// Bumped every time the interning tables are discarded. Anything caching a
// CallStackIndex can invalidate it's cache whenever this changes
internal int TrimGeneration;

/// <summary>
/// Discards TraceLog's call stack interning tables, invalidating CallStackIndexes.
/// Must be called from the event processing thread.
/// </summary>
internal void TrimLiveSessionState()
{
OnTrimForTests?.Invoke();
TraceLog.TrimLiveSessionState();
TrimGeneration++;
}

internal bool IsStopped => _stopped;

internal static Action? BeforeStartupForTests;

internal static Action<SampleProfilerSession>? OnSessionCreatedForTests;

internal static Action? OnTrimForTests;

private static InterlockedBoolean _throwOnNextStartupForTests = false;

internal static bool ThrowOnNextStartupForTests
Expand Down
2 changes: 1 addition & 1 deletion src/Sentry.Profiling/SamplingTransactionProfiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public SamplingTransactionProfiler(SentryOptions options, SampleProfilerSession
_cancellationToken = cancellationToken;
_startTimeMs = session.Elapsed.TotalMilliseconds;
_endTimeMs = double.MaxValue;
_processor = new SampleProfileBuilder(options, session.TraceLog);
_processor = new SampleProfileBuilder(options, session);
session.SampleEventParser.ThreadSample += OnThreadSample;
cancellationToken.Register(() =>
{
Expand Down
36 changes: 36 additions & 0 deletions src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ internal class SamplingTransactionProfilerFactory : IDisposable, ITransactionPro

private const int SHUTDOWN_TIMEOUT_MS = 2_000;

// Once the interning tables exceed this many entries we discard them ASAP
internal int MaxCallStackCount = 100_000;

private readonly SentryOptions _options;

internal Task<SampleProfilerSession> _sessionTask;
Expand All @@ -31,6 +34,10 @@ internal bool IsDisposed
get { lock (_sessionLock) { return _disposed; } }
}

internal int TrimCount;

private bool _trimFailed;

private bool _errorLogged = false;

public SamplingTransactionProfilerFactory(SentryOptions options, TimeSpan startupTimeout)
Expand All @@ -54,6 +61,8 @@ public SamplingTransactionProfilerFactory(SentryOptions options, TimeSpan startu
// This can block indefinitely.
await session.WaitForFirstEventAsync(shutdownToken).ConfigureAwait(false);

session.SampleEventParser.ThreadSample += _ => TrimSessionStateIfNeeded(session);

return session;
});

Expand Down Expand Up @@ -137,6 +146,33 @@ private bool TryBeginShutdown(out SampleProfilerSession? sessionToStop)
return null;
}

/// <summary>
/// Must run on the event processing thread, which is the only thread allowed to trim - hence
/// hanging off ThreadSample rather than off profile completion.
/// </summary>
private void TrimSessionStateIfNeeded(SampleProfilerSession session)
{
if (_trimFailed || session.TraceLog.CallStacks.Count <= MaxCallStackCount)
{
return;
}

try
{
_options.LogDebug("Trimming profiler session state, {0} interned call stacks.", session.TraceLog.CallStacks.Count);
// Costs the in-flight sample: its stack mapping goes with the tables, so AddSample skips it.
session.TrimLiveSessionState();
TrimCount++;
}
catch (Exception e)
{
// Latch off rather than retrying on every subsequent sample, which would throw and log
// at sample rate on the event processing thread.
_trimFailed = true;
_options.LogError(e, "Failed to trim profiler session state. Profiling memory use is no longer bounded.");
}
}
Comment thread
cursor[bot] marked this conversation as resolved.

public void Dispose()
{
if (!TryBeginShutdown(out var session))
Expand Down
149 changes: 149 additions & 0 deletions test/Sentry.Profiling.Tests/SamplingTransactionProfilerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,155 @@ public void ProfilerIntegration_WithProfilingEnabled_SetsFactory()
Assert.NotNull(hub.Options.TransactionProfilerFactory);
}

[SkippableFact]
public void Profiler_WhenNoProfileRunning_TrimsInternedCallStacks()
{
Skip.If(TestEnvironment.IsGitHubActions, "Flaky in CI.");

// The interning tables grow whether or not a profile is running, so this deliberately never
// starts one - the trim has to happen off the back of ordinary samples. See #5469.
using var factory = new SamplingTransactionProfilerFactory(_testSentryOptions, TimeSpan.FromSeconds(30));
factory.MaxCallStackCount = 100;
SkipIfFailsInCI(() => factory._sessionTask.Wait(60_000));
var traceLog = factory._sessionTask.Result.TraceLog;

// Produce a variety of stack shapes so the sampler has something to intern. Run for the
// full duration rather than stopping at the first trim, so we observe the steady state.
var stopwatch = Stopwatch.StartNew();
var random = new Random(4242);
var highWaterMark = 0;
while (stopwatch.ElapsedMilliseconds < 3_000)
{
RecursiveWork(random.Next(8, 24), random);
highWaterMark = Math.Max(highWaterMark, traceLog.CallStacks.Count);
}

SkipIfFailsInCI(() =>
{
if (factory.TrimCount == 0)
{
throw new Exception($"No trim occurred; call stacks peaked at {highWaterMark}.");
}
});

Assert.True(factory.TrimCount > 0, $"Expected at least one trim, call stacks peaked at {highWaterMark}.");

// The table refills immediately after each trim, so its size at any instant is noise. What
// matters is that it stays bounded - untrimmed this workload reaches thousands in 3s.
Assert.True(highWaterMark < factory.MaxCallStackCount * 20,
$"Expected the interning table to stay bounded, but it peaked at {highWaterMark}.");
}

[SkippableFact]
public void Profiler_WhileProfileRunning_StillTrimsInternedCallStacks()
{
Skip.If(TestEnvironment.IsGitHubActions, "Flaky in CI.");

// A profile is held open for the whole test. Trimming must still happen: profiles can run
// back to back under load, so gating the trim on "no profile running" would starve it
// exactly when the tables grow fastest. The profile is then collected to show that trimming
// mid-profile does not corrupt its output.
using var factory = new SamplingTransactionProfilerFactory(_testSentryOptions, TimeSpan.FromSeconds(30));
factory.MaxCallStackCount = 100;
SkipIfFailsInCI(() => factory._sessionTask.Wait(60_000));

var clock = SentryStopwatch.StartNew();
var transactionTracer = new TransactionTracer(Substitute.For<IHub>(), "test", "");
var sut = factory.Start(transactionTracer, CancellationToken.None) as SamplingTransactionProfiler;
SkipIfFailsInCI(() => ArgumentNullException.ThrowIfNull(sut));
transactionTracer.TransactionProfiler = sut;

// Discard trims from before the profile started - the tiny budget means the session
// startup alone can trigger one. From here every trim happens while _inProgress is true.
factory.TrimCount = 0;

// Run for a fixed duration rather than stopping at the first trim: the profile needs to
// last long enough for samples to actually be dispatched to it, or there is nothing to
// validate at the end.
var stopwatch = Stopwatch.StartNew();
var random = new Random(4242);
while (stopwatch.ElapsedMilliseconds < 3_000)
{
RecursiveWork(random.Next(8, 24), random);
}

SkipIfFailsInCI(() =>
{
if (factory.TrimCount == 0)
{
throw new Exception("No trim occurred while a profile was running.");
}
});
Assert.True(factory.TrimCount > 0, "Expected trimming to happen even while a profile is running.");

sut!.Finish();
var elapsedNanoseconds = (ulong)((clock.CurrentDateTimeOffset - clock.StartDateTimeOffset).TotalMilliseconds * 1_000_000);
var collectTask = sut.CollectAsync(new SentryTransaction(transactionTracer));
collectTask.Wait();
ValidateProfile(collectTask.Result.Profile, elapsedNanoseconds);
}

[SkippableFact]
public void Profiler_WhenTrimFails_StopsRetrying()
{
Skip.If(TestEnvironment.IsGitHubActions, "Flaky in CI.");

var attempts = 0;
SampleProfilerSession.OnTrimForTests = () =>
{
attempts++;
throw new InvalidOperationException("Test exception");
};
try
{
using var factory = new SamplingTransactionProfilerFactory(_testSentryOptions, TimeSpan.FromSeconds(30));
factory.MaxCallStackCount = 100;
SkipIfFailsInCI(() => factory._sessionTask.Wait(60_000));

var stopwatch = Stopwatch.StartNew();
var random = new Random(4242);
while (stopwatch.ElapsedMilliseconds < 3_000)
{
RecursiveWork(random.Next(8, 24), random);
}

SkipIfFailsInCI(() =>
{
if (attempts == 0)
{
throw new Exception("The trim was never attempted, so the latch was not exercised.");
}
});

// Without the latch this would be attempted on every sample for the rest of the session,
// throwing and logging each time on the event processing thread.
Assert.Equal(1, attempts);
Assert.Equal(0, factory.TrimCount);
}
finally
{
SampleProfilerSession.OnTrimForTests = null;
}
}

private static long RecursiveWork(int depth, Random random)
{
if (depth <= 0)
{
double sink = 0;
for (var i = 1; i < 5_000; i++)
{
sink += Math.Sqrt(i);
}
return (long)sink;
}

// Branch so that different calls produce different stack shapes.
return (random.Next(2) == 0)
? RecursiveWork(depth - 1, random) + depth
: RecursiveWork(depth - 1, random) - depth;
}

[SkippableFact]
public void Downsampler_ShouldSample_Works()
{
Expand Down
Loading