Reclaim hidden terminal renderers on memory pressure - #7050
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR hooks the existing
Confidence Score: 5/5Safe to merge — the change is additive, non-destructive, and respects the hard invariant that visible surfaces are never touched. The pressure path is a clean O(n) filter that reuses the proven planner/controller machinery. Actor isolation is correct end-to-end: the callback is typed @mainactor, the controller is @mainactor, and no new cross-isolation calls are introduced. The retry logic is sound because rendererRealized is updated synchronously on mailbox success, so the post-release check fires only for genuinely dropped messages and the retry count is bounded at two extra passes. The new tests cover the key planner decisions for pressure mode. No structural or correctness issues were found. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant OS as macOS Kernel
participant PMG as PaneMemoryGuardrail
participant RRC as RendererRealizationController
participant RRP as RendererRealizationPlanner
participant TS as TerminalSurface
OS->>PMG: DispatchSourceMemoryPressure warning/critical
PMG->>RRC: reclaimForSystemMemoryPressure(now:)
RRC->>RRP: selectedSurfaceIds(trigger: .systemMemoryPressure)
RRP-->>RRC: hidden realized surface IDs
loop each selected surface
RRC->>TS: releaseRenderer()
TS-->>RRC: rendererRealized false on success, true on drop
end
opt mailbox drop detected
RRC->>RRC: "Task @MainActor evaluate retries-1"
end
PMG->>PMG: discardHiddenBrowserWebViews()
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant OS as macOS Kernel
participant PMG as PaneMemoryGuardrail
participant RRC as RendererRealizationController
participant RRP as RendererRealizationPlanner
participant TS as TerminalSurface
OS->>PMG: DispatchSourceMemoryPressure warning/critical
PMG->>RRC: reclaimForSystemMemoryPressure(now:)
RRC->>RRP: selectedSurfaceIds(trigger: .systemMemoryPressure)
RRP-->>RRC: hidden realized surface IDs
loop each selected surface
RRC->>TS: releaseRenderer()
TS-->>RRC: rendererRealized false on success, true on drop
end
opt mailbox drop detected
RRC->>RRC: "Task @MainActor evaluate retries-1"
end
PMG->>PMG: discardHiddenBrowserWebViews()
Reviews (2): Last reviewed commit: "Retry pressure renderer reclaim on dropp..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/App/RendererRealizationPlanner.swift`:
- Around line 42-43: The memory-pressure fast path in
RendererRealizationPlanner’s planning logic is doing unnecessary ranking work
before returning all hidden realized surfaces. Move the `trigger ==
.systemMemoryPressure` branch ahead of the sort/ranking path in the relevant
planning method so this case bypasses the full `sorted` pass. Build the returned
`Set` directly from `inputs`/the hidden-surface filter for that trigger, keeping
the path near-linear and avoiding repeated filter/map/sorted work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7f8c8597-5c99-453e-ad3d-90af821c7034
📒 Files selected for processing (4)
Sources/App/RendererRealizationController.swiftSources/App/RendererRealizationPlanner.swiftSources/AppDelegate.swiftcmuxTests/RendererRealizationPlannerTests.swift
Summary
Closes #7039.
References the master resource-accumulation tracker #5731.
This PR makes the existing non-destructive terminal renderer reclamation path react to macOS memory-pressure warnings. The normal timer still uses the configured idle/LRU policy. On a warning/critical
DispatchSourceMemoryPressureevent, cmux now asksRendererRealizationControllerto release every hidden realized terminal renderer immediately, while preserving the hard safety rule that visible terminals are never selected. The existing hidden-browser discard runs in the same callback afterward.This deliberately builds on the renderer realization machinery already on
main; it does not duplicate the broader shell-restart hibernation work from #5739. It also does not conflict with #6979/#6984, which tune/apply the same renderer-reclaim direction for canvas/default behavior.Measurement Baseline
Captured on the running stable app without building locally:
/Applications/cmux.app, PID26804, version0.64.17 (97).footprint -summary -pid 26804:2060 MBphysical footprint,2200 MBpeak.420 MBIOSurface,159 MBIOAccelerator,392 MBowned graphics footprint,22 MBCoreAnimation.heap -s 26804:118WorkspaceSwift objects present, matching the many-open-workspace shape from Master: unbounded per-workspace resource accumulation — ~30 GB memory, system-wide lag (immortal surfaces, mass agent auto-resume, unthrottled pollers) #5731/WindowServer watchdog killed the entire GUI session under OOM (35.2/36 GB) — stable cmux app is the #1 process at 2.6 GB (2026-06-27 spindump) #7039.cmux memory --all --groups 12:2.04 GBapp footprint plus4.55 GBchild RSS across150processes.The change targets the app-process graphics resident set. It does not claim to reduce child-process RSS from agents; that remains the #5731/#6313 child-process guardrail/workspace lifecycle problem.
Demo Video
Not attached. Per task instructions, the dev build must not be launched until after CI is green and the user explicitly approves the cloud build command.
Review Trigger
Memory/performance fix. Please review the renderer realization pressure path, visible-surface safety invariant, bounded retry behavior for dropped Ghostty mailbox enqueues, and Xcode project wiring for the new source file.
Validation
git diff --checkpython3 scripts/swift_file_length_budget.py --write-budgetpython3 scripts/swift_file_length_budget.py./scripts/lint-pbxproj-test-wiring.sh./scripts/check-pbxproj.shpython3 scripts/check-package-resolved-policy.pypython3 scripts/check-workspace-package-groups.py --checkPer task instructions, I did not run local tests,
reload.sh,reload-cloud.sh, or barexcodebuild.Checklist
Closes #7039and references Master: unbounded per-workspace resource accumulation — ~30 GB memory, system-wide lag (immortal surfaces, mass agent auto-resume, unthrottled pollers) #5731.cmux.xcodeproj.Localization Audit
No user-facing strings, settings rows, docs text, schema text, alerts, menus, or tooltips were added or changed. The changed Swift surfaces are internal planner/controller/callback code plus tests, so no localization catalog updates are required.