Add foreground-task spawn-site census for memory attribution (APP-5393) - #15208
Open
warp-agent-staging[bot] wants to merge 2 commits into
Open
Add foreground-task spawn-site census for memory attribution (APP-5393)#15208warp-agent-staging[bot] wants to merge 2 commits into
warp-agent-staging[bot] wants to merge 2 commits into
Conversation
Adds a live census of in-flight main-thread (Foreground) tasks, keyed
by the #[track_caller] call site that spawned them, so that a heap
profile whose stack dead-ends in the executor's boxed Future::poll can
still name the code responsible.
- Foreground::spawn/spawn_abortable/spawn_boxed are now #[track_caller],
and the attribute is propagated up through AppContext::spawn_local,
ModelContext::{spawn,spawn_abortable,spawn_with_retry_on_error(_when)},
and the equivalent ViewContext methods, so that application-level
ctx.spawn(...) call sites are recorded instead of collapsing into the
shared plumbing that forwards to the executor.
- ForegroundTaskCensus tracks live/total counts per spawn site in a
RefCell (the executor is main-thread only), incrementing on spawn and
decrementing on completion or drop (covering abort/cancellation).
- Foreground::task_census_snapshot() exposes the top-N spawn sites by
live count plus the total live task count.
- Wires the snapshot into the existing excessive-memory Sentry report
(app/src/system/info.rs, app/src/profiling.rs) as a second
foreground_task_census context, alongside the existing
memory_breakdown context, gated the same way (heap_usage_tracking).
This is attribution instrumentation only; it does not fix a leak. See
the PR description for the investigation that concluded no
attributable leak exists in Sentry issue 7259255054.
Co-Authored-By: Warp <agent@warp.dev>
Contributor
Author
|
This PR was generated with Warp. |
The census added in the previous commit relies on an unbroken chain of `#[track_caller]` from the application's call site down to `Foreground::spawn_boxed`. `ModelContext`/`ViewContext::spawn_stream_local` and `spawner` were missing the attribute, so every stream task in the app collapsed onto a single line in the context wrapper -- the same bucketing failure the census exists to prevent, which would have hidden a real pile-up behind one entry. The added tests fail without the attributes (every site resolves to `context.rs`, so no site in the test file is recorded) and pass with them. Co-Authored-By: Warp <agent@warp.dev>
Contributor
Author
|
Four more occurrences of this signature since this PR was opened (Sentry All five affected releases ( Marked ready for review to run the full CI suite, which was skipped while this sat in draft. Responding as wilson: Open session · View factory task |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Description
This PR does not fix a memory leak. Investigation into Sentry issue 7259255054 (macOS Stable,
phys_footprintalert) found no attributable leak:phys_footprint, which includes compressed/swapped pages. Across the latest 100 events, ~53 had compressed memory at ≥70% of the reported footprint, so the headline 8-20 GB figures aren't straightforwardly live leaked heap.DispatchDelegate::run_on_main_thread→async_task::Runnable::run→ boxeddyn Future::poll) is shared by every foreground task. The stack can't name which task is responsible, and the leaf frames are unsymbolized AppKit.a9c0a1e(Coalesce superseded ProjectContextModel rule refresh tasks (APP-5401) #15147), which coalesced piled-upProjectContextModel::refresh_project_rules_for_repotasks. It merged 2026-08-15, after the affected release (v0.2026.07.29.09.05.stable_02).What this PR does build is the instrumentation the investigation called for (angle 3): a live census of in-flight
Foreground(main-thread) tasks, keyed by their spawn call site, attached to the existing excessive-memory Sentry report. The next time this alert fires, the event will name the call sites holding tasks open instead of dead-ending inFuture::poll.To be explicit about what this does and does not do: it will not reduce memory usage. It explains it.
How it works
Foreground::spawn/spawn_abortable/spawn_boxedare#[track_caller], and aForegroundTaskCensus(a plainRefCell-backed map, since the executor is main-thread only — no lock contention) increments a per-site counter on spawn and decrements it when the task completes or is dropped (covering abort/cancellation, via a guard).Foreground::task_census_snapshot(limit)returns the total live task count plus the top-N spawn sites by live count, sorted descending.app/src/system/info.rs→app/src/profiling.rs::dump_jemalloc_heap_profile, which now takes the snapshot and attaches it as a secondforeground_task_censusSentry context, alongside the existingmemory_breakdowncontext.The
#[track_caller]chain is the whole ballgame#[track_caller]on the threeForegroundmethods alone is not enough, and getting this wrong is the one way this change silently does nothing useful. Application code doesn't call the executor directly; it goes through context wrappers, and a wrapper that drops the attribute becomes the recorded location for every one of its callers. Two separate collapse points had to be fixed:ctx.spawn(...)routes throughAppContext::spawn_local— a single fixed line inapp.rs. Left unannotated, everyctx.spawncall in the app would collapse into that one line. The full chain (ctx.spawn→ModelContext::spawn_abortable→ModelContext::spawn_local→AppContext::spawn_local→Foreground::spawn_boxed) is now annotated, plus theViewContextequivalents and thespawn_with_retry_on_error(_when)helpers. This is the path therefresh_project_rules_for_reposibling used.ctx.spawn_stream_local(...),ctx.spawner()) route throughAppContext::spawn_stream_local. TheModelContext/ViewContextwrappers above it were still missing the attribute, so a whole class of potentially long-lived stream tasks collapsed onto two fixed lines in the context files. Caught in review and fixed inc48aa9e.Because this is silent when wrong, both collapse points now have regression tests that fail without the attributes.
Feature gating
The counting itself (in
warpui_core) is always compiled in and always on — it's cheap (a hash-map entry bump plus anRcclone per spawn, no locks, no extra heap allocation since it reuses the caller's existingLocalBoxFuturebox) and useful independent of Sentry reporting. Only the Sentry-facing wiring inapp/is gated behind the existingheap_usage_trackingfeature, matching howmemory_breakdownis already gated. I didn't add a new feature flag since this isn't a product-facing toggle — it reuses the existing profiling-feature convention.Scope
Foreground only, per the issue —
Background::spawn_boxedhas the same shape but runs across multiple real OS threads, so census tracking there would need a thread-safe structure (mutex/dashmap), which is a meaningfully different (and costlier) change. Not worth it for an executor that isn't implicated in this issue.Linked Issue
Linear: APP-5393
Testing
./script/formatand the clippy invocations from./script/presubmitpass, includingcargo clippy -p warpui_core --all-targets --all-features --tests -- -D warnings.cargo test -p warpui_core --lib: 321 passed, 0 failed, 7 ignored.crates/warpui_core/src/async/native/executor_tests.rscover:refresh_project_rules_for_repopile-up had it existed before APP-5401);crates/warpui_core/src/core/mod_tests.rscover the second collapse point: distinct callers ofModelContext/ViewContext::spawn_stream_localandspawner()must resolve to distinct sites. Verified these actually regress: with the#[track_caller]attributes reverted, both tests fail with every site attributed tocontext.rsand none to the calling file; with them, both pass.cargo check -p warp_tuistill builds (the TUI shares this executor).cargo check -p warp --features heap_usage_trackingcould not be completed in the authoring environment — rustc was SIGKILLed by a sandbox memory ceiling on thewarpcrate, including atCARGO_BUILD_JOBS=1. That is a resource limit, not an observed compile error, and the revision touches onlywarpui_core(adding attributes, which are not signature changes). Still, the feature-gatedapp/wiring deserves a CI confirmation before merge.Agent Mode