Skip to content

fix: bound profiler memory growth by trimming interned call stacks - #5503

Open
jamescrosswell wants to merge 8 commits into
deps/update-perfviewfrom
fix/5469-trim-live-session-state
Open

fix: bound profiler memory growth by trimming interned call stacks#5503
jamescrosswell wants to merge 8 commits into
deps/update-perfviewfrom
fix/5469-trim-live-session-state

Conversation

@jamescrosswell

@jamescrosswell jamescrosswell commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #5502 to keep the mechanical changes for the submodule bump independent of the fix itself. This PR targets deps/update-perfview and is just the fix.

Fixes #5469

What was leaking

TraceCallStacks.InternCallStackIndex interns every distinct call stack it observes and never
releases any of them. That is correct for a trace file, which is finite, but the profiler's
EventPipe session runs for the life of the process — so the tables grow forever. Two properties made
it worse than it sounds: growth tracks the diversity of stack shapes rather than sample volume,
and it happens whether or not a profile is running, which is why lowering ProfilesSampleRate
never helped and only setting it to 0 did.

The fix

TraceLog.TrimLiveSessionState() (microsoft/perfview#2452, contributed for this) discards those tables. This wires it up from an always-on ThreadSample handler on the factory.

A trim reissues CallStackIndex from zero, and SampleProfileBuilder caches against it, so a stale entry would resolve a sample to an unrelated stack — wrong frames, no exception.

So the cache must be invalidated. SampleProfilerSession carries a generation that increments on every trim; SampleProfileBuilder records what it saw and drops _stackIndexes when the generation increments. Affected stacks get walked again, at worst appearing twice in Profile.Stacks, which samples reference by the builder's own ids. Potentially we spend a bit more CPU then - but we avoid the memory leak.

Both run on the dispatch thread, so ordering can't bite: trim-then-AddSample sees the bump and
clears, AddSample-then-trim resolved against tables that were valid at that instant and sees the
bump next time.

The builder's other caches need no handling: _frameIndexesByCodeAddressIndex and
_frameIndexesByMethodIndex key off tables a trim doesn't touch, and _threadIndexes keys off
TraceLog.Threads, which is a different table from the per-thread roots inside TraceCallStacks.

Budget

MaxCallStackCount is 100,000 (~10 MB of tables). At the rate in the field report that is a trim
roughly every 7 minutes, and a trim measured at ~0.012 MiB of unreturned RSS. It is an internal
static rather than a SentryOption because it is a tuning constant for a bug fix, not a feature —
we could expose this if there was a compelling reason but I think best to leave internal initially.

Why not just restart the session

That was the obvious alternative and it was measured and rejected: it bounds managed memory but costs
3.1–3.3 MiB of RSS per recycle that is never returned, making RSS grow roughly 3x faster than the
leak. Trimming in place costs ~0.012 MiB, because no new EventPipe session is created.

Effect

Same workload, 600 s, real time session on .NET 9 in a Linux container:

without trim with trim
interned call stacks at 600 s 1,231,862, still climbing bounded, cycling 919 – 7,121
managed heap 3.4 → 119.2 MiB 3.5 → ~7 MiB, flat
RSS 65.2 → 200.7 MiB 65.6 → 81.4 MiB
samples processed 307,418 340,987

Throughput went slightly up, since the baseline spends real time growing and collecting those
tables. That workload is deliberately far more stack-diverse than a real service so the effect is
measurable in minutes — the shape matches the field report, the rate is exaggerated.

Test

Profiler_WhenNoProfileRunning_TrimsInternedCallStacks covers the idle path — no profile is ever
started, so the trim has to come from ordinary samples.

Profiler_WhileProfileRunning_StillTrimsInternedCallStacks covers the starvation case: it holds a
profile open for the whole test, asserts trimming still happens, then collects and validates the
profile to show a mid-profile trim doesn't corrupt its output. Both were mutation-tested — restoring
the old _inProgress guard makes the second fail with "No trim occurred while a profile was running".

Sentry.Profiling.Tests on net10.0: 19 passed, 0 failed, 3 skipped. net8.0/net9.0 hosts can't be
launched locally (no arm64 runtimes for those on this machine), so CI covers those.

Not included

SamplingTransactionProfilerFactory.Dispose() still disposes the Task rather than the session —
that is #5418, with its own PR (#5470), so it is left alone here to avoid a conflict. The handler is
consequently not unsubscribed on dispose, which is harmless while the session outlives the factory.

TraceLog interns every distinct call stack it observes and never releases them,
so the continuous EventPipe session behind transaction profiling grew without
bound - roughly 0.6 GB/day to OOM under production traffic on Linux.

Uses TraceLog.TrimLiveSessionState() (microsoft/perfview#2452) to discard those
tables once they pass a budget. The trim runs from an always-on ThreadSample
handler on the factory, which matters for two reasons: it is the session's event
processing thread, which is the only thread TrimLiveSessionState permits, and
the tables grow whether or not a profile is running, so trimming only at profile
boundaries would leave the growth unchecked whenever traffic is sparse.

Trimming is gated on no profile being in progress AND the previous profile
having stopped consuming samples. The second condition is not redundant:
_inProgress is cleared when a transaction finishes, but that profiler stays
subscribed and keeps resolving samples up to its end timestamp - roughly two
seconds later, because TraceLog dispatches on a delay. Trimming inside that
window would leave SampleProfileBuilder's CallStackIndex-keyed cache resolving
to unrelated stacks. Passing the end timestamp to OnFinish lets the handler tell
when that drain has finished.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 23.07692% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.73%. Comparing base (a9f09c4) to head (21be80c).

Files with missing lines Patch % Lines
...ry.Profiling/SamplingTransactionProfilerFactory.cs 40.00% 8 Missing and 1 partial ⚠️
src/Sentry.Profiling/SampleProfileBuilder.cs 0.00% 6 Missing and 1 partial ⚠️
src/Sentry.Profiling/SampleProfilerSession.cs 0.00% 3 Missing ⚠️
...rc/Sentry.Profiling/SamplingTransactionProfiler.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##           deps/update-perfview    #5503      +/-   ##
========================================================
- Coverage                 74.78%   74.73%   -0.06%     
========================================================
  Files                       513      513              
  Lines                     18759    18784      +25     
  Branches                   3669     3671       +2     
========================================================
+ Hits                      14029    14038       +9     
- Misses                     3856     3870      +14     
- Partials                    874      876       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/Sentry.Profiling/SamplingTransactionProfiler.cs Outdated
Comment thread src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs Outdated
Comment thread src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs Outdated
Comment thread src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs Outdated
Comment thread src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs Outdated
Comment thread src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs Outdated
Gating the trim on "no profile running, and the previous one has drained" could
starve indefinitely. Profiles run back to back under sustained load - a new one
starts as soon as the previous clears _inProgress - so the window where trimming
was permitted could simply never coincide with a dispatched sample. The tables
then grow unbounded, in exactly the workload where that matters most.

The guards were conservative because a trim reissues CallStackIndex from zero
and SampleProfileBuilder caches against it. Rather than avoid that, invalidate
it: SampleProfilerSession carries a generation that increments on every trim,
and the builder drops _stackIndexes when it sees the generation move. Its other
caches are unaffected, since a trim touches neither the code address and method
tables nor TraceLog.Threads.

That makes trimming safe at any moment, so both guards, _lastProfileEndTimeMs
and the timestamp on OnFinish all go away.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread src/Sentry.Profiling/SampleProfileBuilder.cs Outdated
Comment thread src/Sentry.Profiling/SampleProfilerSession.cs Outdated
Comment thread src/Sentry.Profiling/SampleProfilerSession.cs Outdated
Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
@jamescrosswell
jamescrosswell marked this pull request as ready for review August 25, 2026 05:12
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Aug 25, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 21be80c. Configure here.

// Never let this take down event processing for the whole session.
_options.LogWarning(e, "Failed to trim profiler session state.");
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trim invalidates in-flight stack indexes

Medium Severity

TrimSessionStateIfNeeded is subscribed to ThreadSample before the profiler's OnThreadSample, so when a trim fires it runs first and discards call-stack tables for the event currently being dispatched. AddSample then clears _stackIndexes, but that only drops the builder cache — it does not make the event's already-assigned CallStackIndex valid again in TraceLog. That sample can be walked as empty/wrong frames or skipped after an exception, and the same risk applies to any other in-flight samples that still carry pre-trim indexes (including the ~2s latent drain after Finish).

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 21be80c. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant