Skip to content

Attach jemalloc allocator stats to excessive-memory Sentry events - #15165

Draft
warp-agent-staging[bot] wants to merge 2 commits into
masterfrom
factory/core-3791-jemalloc-memory-diagnostics
Draft

Attach jemalloc allocator stats to excessive-memory Sentry events#15165
warp-agent-staging[bot] wants to merge 2 commits into
masterfrom
factory/core-3791-jemalloc-memory-diagnostics

Conversation

@warp-agent-staging

Copy link
Copy Markdown
Contributor

Description

Attaches jemalloc allocator stats and OS memory footprint samples to the "Excessive memory usage detected" Sentry event, so it's possible to tell apart memory that was freed before the heap dump was collected, memory retained (but not actively used) by the allocator, and genuinely non-heap memory usage.

Today the pprof heap profile attached to that event only accounts for sampled live jemalloc allocations at dump time, so it can look tiny (e.g. 0.39 GB) even when the OS footprint that tripped the 10 GB threshold is much larger, with no way to explain the gap.

This change calls epoch::advance() and then reads stats.allocated, stats.resident, stats.retained, and stats.mapped via tikv-jemalloc-ctl, plus a freshly re-read OS memory footprint (alongside the footprint that originally tripped the threshold), and attaches all of it as a new memory_diagnostics Sentry context next to the existing memory_breakdown context. Per the linked issue's explicit non-goal, the event is still always sent -- nothing about the heap content is used to suppress or drop it. A failure reading any individual jemalloc stat is logged and does not prevent the event from being sent.

Linked Issue

CORE-3791: https://linear.app/warpdotdev/issue/CORE-3791/excessive-memory-sentry-events-cant-distinguish-freed-before-dump-from

  • Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). N/A -- this is a backend observability change with no UI surface.

Testing

  • Added app/src/profiling_tests.rs, a unit test asserting the new memory_diagnostics_for_sentry helper attaches the triggering and freshly-read footprints and successfully reads jemalloc's stats.allocated/resident/retained/mapped.
  • cargo check -p warp --no-default-features --features heap_usage_tracking --lib -- passed
  • cargo check -p warp --lib (default features, jemalloc/heap tracking disabled) -- passed
  • cargo check -p warp --no-default-features --features jemalloc_auto_heap_profiling --lib (jemalloc enabled without heap_usage_tracking) -- passed
  • cargo test -p warp --no-default-features --features heap_usage_tracking --lib memory_diagnostics -- passed (1 test)
  • cargo clippy -p warp --no-default-features --features heap_usage_tracking --lib --tests -- -D warnings -- passed, no warnings
  • ./script/format --check -- passed
  • Not verified: actual compilation for macOS/Windows targets (this environment is Linux-only), since the feature gating (heap_usage_tracking -> jemalloc_pprof -> jemalloc_with_profiling -> jemalloc) and the new tikv-jemalloc-ctl dependency were added to the same non-platform-gated feature flags already used cross-platform by the existing jemalloc_pprof/crash_reporting features, and tikv-jemalloc-ctl is already a transitive dependency of the jemalloc_pprof crate that's compiled on all platforms today.

Screenshots / Videos

N/A -- backend-only change.

Agent Mode

  • Warp Agent Mode - This PR was created via Warp's AI Agent Mode

The heap profile attached to the 'Excessive memory usage detected'
Sentry event only accounts for sampled *live* jemalloc allocations at
dump time, so it can look tiny (e.g. 0.39 GB) even when the OS memory
footprint that tripped the alert is >=10 GB. There was no way to tell
apart memory that was freed between the threshold trip and the dump,
memory retained (but not actively used) by the allocator, and
genuinely non-heap memory.

Read jemalloc's stats.allocated/resident/retained/mapped via
tikv-jemalloc-ctl (advancing the epoch first so the reads aren't
stale), plus a freshly-read OS memory footprint, and attach them as a
'memory_diagnostics' Sentry context alongside the existing
memory_breakdown context. A failure to read any individual stat is
logged and does not prevent the event from being sent, and the event
is still sent unconditionally (no suppression based on heap content).

Fixes CORE-3791.

Co-Authored-By: Warp <agent@warp.dev>
@warp-agent-staging

Copy link
Copy Markdown
Contributor Author

@/tmp/pr_comment_xyNsYJ

…filing

tikv-jemallocator/stats (and jemalloc's --enable-stats) was previously
listed under jemalloc_with_profiling, which also gates
jemalloc_auto_heap_profiling -- an unrelated, pre-existing profiling
configuration that doesn't use the new diagnostic reads. --enable-stats
carries measurable allocation-path overhead, so this imposed an
unnecessary production cost on that build.

Move it to jemalloc_pprof, which is the only feature that actually
reads jemalloc allocator stats (via heap_usage_tracking).

Verified with 'cargo tree -e features -i tikv-jemalloc-sys' that
jemalloc_auto_heap_profiling no longer pulls in the stats feature,
while heap_usage_tracking still does.

Co-Authored-By: Warp <agent@warp.dev>
@warp-agent-staging

Copy link
Copy Markdown
Contributor Author

Cost of enabling jemalloc --enable-stats

Recording this here since it came up in review discussion and it is the main thing to weigh on this PR.

Which builds are affected. tikv-jemalloc-sys/stats arrives via jemalloc_pprof, which script/macos/bundle enables for every macOS channel including stable, and script/linux/bundle enables for Linux dev/preview only. Windows never enables jemalloc, and the TUI binary opts out through its own feature list. Within an affected channel it also reaches the cli/warpctrl binaries.

This is a real increment, not a no-op. Those configurations already shipped jemalloc_pprof/heap_usage_tracking before this PR, but without stats: jemalloc_with_profiling previously added only tikv-jemallocator/profiling, and the upstream jemalloc_pprof crate requests only tikv-jemalloc-ctl/use_std.

What the overhead actually is. Small/bin-classed allocations already maintain a per-thread byte counter regardless of the stats build flag (see jemalloc PR #1595, which deliberately moved thread_allocated/thread_deallocated out of config_stats). --enable-stats mainly adds one mutex-guarded increment on the large-allocation path (arena_large_malloc_stats_update / arena_large_dalloc_stats_update, gated by cassert(config_stats)). jemalloc maintainer interwq, in jemalloc/jemalloc#2077: "In our production setup, --enable-stats don't add too much overhead. It's certainly measurable in micro-benchmarks... We do enable stats in our production settings."

Per-event cost. epoch::advance() is the heavyweight part — it walks the bins across all arenas under per-arena mutexes — and the same maintainer suggests keeping repeated polling to roughly 10/sec. We call it exactly once per process, only after the 10 GB threshold has already fired. It runs inside ctx.spawn, which routes to the background executor rather than the foreground/UI executor, so it cannot stall rendering.

Everything else is noise. The new memory_diagnostics context serializes to roughly 246 bytes against a gzipped pprof attachment already measured in tens to hundreds of KB. jemalloc's per-arena stats structures are a few KB per arena, tens to low hundreds of KB total. Binary size was not measured — a full GUI release build was not practical in this sandbox — so treat that one as unquantified rather than zero.

The open question for the reviewer. jemalloc's stats are compile-time only, so there is no runtime gate. If stable macOS should carry no additional allocator overhead at all, the alternative is to enable stats only on dev/preview — at the cost of the fields being absent from exactly the alerts that stable users generate.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants