feat: auto-hibernate idle background services, keeping chat apps live - #12
Conversation
|
Thanks for this, and no need to frame it as optional. The reasoning is the strongest part: exempting the Messaging category and keeping the Keep Loaded escape hatch is the right way to hibernate hard without dropping a real-time chat alert, and reusing hibernate() so the badge poller keeps running is exactly right. It compiles on current main and the suite is green here too (112). One real bug, and it is the interaction you flagged yourself.
The fix is the guard the eviction loop already uses: after the await, re-check Two smaller notes, neither blocking:
I am holding the merge on the active-service re-check plus a real-app run, the same bar as #10. Add the guard and I will take it through. |
The pool keeps up to 15 services fully resident (`maxLoaded`), and switching away only soft-hibernates — it pauses media but leaves the WebContent process running. So on a typical setup every service you've opened stays live indefinitely, each running its web app in the background. With ~10 services that is ten web apps' worth of CPU and RAM at all times, which drains the battery. This adds an opt-in idle sweep: a service backgrounded longer than the threshold is fully hibernated, freeing its process, and wakes from its snapshot when you return. Off by default (new `Optional` prefs, so SwiftData lightweight migration is a no-op on existing stores); a Settings > General > Performance toggle turns it on, with an interval picker (5 min to 1 hour, default 10). The reason it can be on without missing messages: a hibernated web app can't fire a real-time notification — it can only refresh its unread badge on the existing 60s poll. So the sweep never touches a service in the catalog's **Messaging** category (Slack, Teams, WhatsApp, Discord, …). Those stay fully live and notify instantly. It also skips any service marked "Keep Loaded" (covering custom, non-catalog chat apps) and any service in an active call, reusing the guards the cap-based eviction already applies. Everything else — email, docs, music, AI — hibernates when idle and updates its badge within a minute. Hibernation reuses the existing `hibernate()` path, so a hibernated service is registered with `HibernatedBadgePoller` exactly as a manually hibernated one is; no new badge plumbing. Verified by running the app with a 1-minute interval: the seven non-messaging services hibernated on the sweep (WebContent processes dropped from 22 to 13, main-process CPU fell to ~0%), while Slack, Teams and WhatsApp stayed live. Four tests cover the preference resolution, the interval clamp, and the Messaging-category classification the exemption depends on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
3ebecf3 to
8e98f95
Compare
|
Good catch, and thank you — that is the exact race I waved at and did not pin down. You are right that Fixed by pulling the re-validation into the pool, as you suggested, so both paths share one copy. New
Also took the smaller note: Verified in a real run with a 1-minute interval: the seven non-messaging services hibernated (WebContent 27 → 18), the active service stayed live, and no messaging app was touched. Suite still green (112). One clean commit, force-pushed. On the unit-test front — I could not add one for the race itself: |
|
Reviewed the new revision, and the fix is right.
It builds on current main here and the suite is green (112). On the missing unit test, your read is fair. That leaves the by-hand run, the same bar as #10. I'll take it through that and merge if it holds. Thanks for the quick, clean turnaround. |
|
Verified end to end, and merging. Two things blocked the obvious check. This app's SwiftUI controls aren't in the accessibility tree, so I couldn't drive the Settings toggle from automation, and the ad-hoc debug build gives me no log channel to read. A raw WebContent process count doesn't settle it either: WebKit reclaims idle renderers on its own, so with the feature off the count still fell from 27 to 22 over the same idle window. To get a clean read I added a short-lived trace to the sweep (reverted after) and watched two passes, active service Gmail, one-minute idle threshold:
So the sweep fires on its timer, tears down idle non-chat services, spares the Messaging category even when those are idle, and leaves the service in front untouched. The app stayed up across both passes. That matches the design, and with the race fix and the green suite (116 on the merge) I'm satisfied. Merging now. Thanks again. |
Findings from a four-dimension review after merging external contributions (#6-9,#11,#12) plus #13. Each verified against source. Security: - FaviconFetcher: re-validate every HTTP redirect hop with isFetchableIconURL via a per-task URLSession delegate. The guard previously checked only the first hop, so a public-host icon href could 302 to loopback/intranet (SSRF). - WebViewCoordinator.belongsToService: reduce hosts with the public-suffix-aware captureRegistrableDomain instead of the naive effectiveDomain, so shared multi-tenant hosting suffixes (*.vercel.app, *.github.io, *.pages.dev, *.workers.dev) no longer collapse to the bare suffix and let an attacker sibling load in a service's authenticated web view. Aligns link routing with the stricter capture-trust check. - WebViewCoordinator step 1: only hand a non-web scheme (mailto/tel/facetime/ imessage/...) to NSWorkspace.open on a .linkActivated click, so a page can't spam compose/call prompts via programmatic navigation. Correctness: - Extend the Messaging notification-critical exemption to the LRU cap-based eviction path, not just the idle-timer sweep. Chat apps now stay live even past maxLoaded, matching the Settings promise. Pool caches the classification via an AppState-provided isNotificationCritical closure. - ServiceSidebarView.moveService: guard the space's modelContext before reading .space.id, matching the file's other traversals, so a dangling link in the query snapshot can't crash Move-to-Space. - Correct the hibernated-badge cadence copy and comments: the transient sweep is every few minutes (180s) and raise-only, not 'about once a minute'. Tests: add belongsToService shared-hosting-tenant cases. Full suite 120 green.
First, thank you for the care you put into reviewing the earlier six. Merging five so quickly, and gating #10 on a real by-hand layout check, is exactly the kind of review I hoped for. It made the whole thing feel worthwhile.
Please treat this one as entirely optional. It scratches a personal itch more than it fixes a general problem: I work with AI tooling all day, my RAM is always under pressure, and I am the kind of person who watches Activity Monitor for sport. This grew out of that. If it doesn't fit your vision for Chorus, closing it costs me nothing — I mainly wanted to offer it in case it's useful to others like me.
What it does
The pool keeps up to
maxLoaded(15) services fully resident, and switching away only soft-hibernates: it suspends media and snapshots the view, but the WebContent process keeps running. So on a typical setup every service you have ever opened this session stays live in the background, each running its web app. With ~10 services that is ten web apps' worth of CPU and RAM held at all times. Full hibernation (freeing the process) only happens when you exceed 15 services, or when you hibernate one by hand.This adds an opt-in idle sweep: a service backgrounded longer than a threshold is fully hibernated, freeing its process, and wakes from its snapshot when you return to it.
Why it can be on without missing messages
This was the part I cared most about getting right. A fully hibernated web app cannot fire a real-time notification — it can only refresh its unread badge on the existing 60-second poll. For a chat app you rely on for work, that delay is unacceptable.
So the sweep never touches a service in the catalog's Messaging category (Slack, Teams, WhatsApp, Discord, …). Those stay fully live and notify instantly. It also skips:
neverHibernate), which covers custom, non-catalog chat apps you add yourself;hasActiveCallguard the cap-based eviction already applies), so a hibernation pass can't drop a live huddle.Everything else — email, docs, music, AI tabs — hibernates when idle and updates its badge within a minute. In my own use this took the resident-service count from ten down to the three chat apps plus whatever I'm actively looking at, which is a large, sustained RAM and CPU drop while losing nothing I need in real time.
How it's wired
hibernate(), which already firesonServiceHibernatedand registers the service withHibernatedBadgePoller. An auto-hibernated service keeps its unread badge exactly as a manually hibernated one does.WebViewPool.idleServiceIDs(idleFor:now:)answers only what the pool can — not active, notneverHibernate, not pinned, loaded, and idle past the threshold.AppStatethen applies the Messaging-category exemption (it has the catalog + model context) and the async call check before hibernating. The pool stays about web views; the policy lives where the data does.startQuietHoursTimer(a 60sTaskloop), running only while the feature is enabled.Preferences and migration
Two new
Optionalprefs —autoHibernateIdleEnabledandautoHibernateIdleMinutes— following the same pattern asautoDarkModeEnabledand the others, so SwiftData lightweight migration is a no-op on existing stores. Off by default (an upgrade changes nothing until asked). ASettings > General > Performancetoggle turns it on, with an interval picker (5 minutes to 1 hour, default 10). The minutes value is clamped to 1...120 on read, so a corrupt row can't set a zero or negative sweep.What I verified, and what I didn't
Ran the app with a 1-minute interval: the seven non-messaging services hibernated on the sweep — WebContent processes dropped from 22 to 13, main-process CPU fell to roughly zero — while Slack, Teams and WhatsApp stayed live and kept notifying. Four tests cover the preference resolution, the interval clamp, and the Messaging-category classification the exemption depends on. Full suite green (112).
I did not stress the interaction with the cap-based eviction under a rapidly changing service set, or test a service that is in a call and idle past the threshold beyond the single guard. Both look correct by reading, but I'd rather flag it than claim more than I checked.