Conversation
…and stop a suspended pass deferring every one after it A backgrounded offload defers its re-score to a processing task (ryanbr#1538) and then runs the Apple Health write-back straight away, before anything new is scored. The deferred pass published the widget snapshot when it finished but never Apple Health, so last night's sleep and vitals reached Health only on a later foreground, typically mid-day. The pass also timed itself with `Date()`, which counts the minutes a suspended process spends asleep. One overnight pass on a sleeping phone banked 19 003 s as its cost, and every background re-score after it deferred on that figure. It now times on the uptime clock, as the rest of the engine's probes already do, and the policy reads a measurement past 30 minutes as unknown, which also clears a value an install banked before this change. The processing task's expiry is now logged. A strap log that simply stopped at 00:01 had no way to say whether the pass was cut off there.
…of deferring it ryanbr#1538 read the killed background passes as work that could not finish inside a wake, and deferred any pass whose last completed run took over 20 s to a processing task. The on-device crash reports say otherwise: 26 `cpu_resource_fatal` kills on one iPhone in five nights ("48 seconds cpu time over 52 seconds ... exceeding limit of 80% cpu over 60 seconds"), each about 52 s into a pass. A cold pass is ~144 s of near-continuous CPU on that install (`re-score: done — scored 21 night(s) in 144311 ms`, `dayCache reused=0/21`), so every attempt in the background was killed, including the processing task it was deferred to. The deferral only moved the kill, and the night's scores reached the phone when the app was next opened. A backgrounded pass now rests after each night, in both loops, for as long as the night took (capped at 30 s), holding it near 50% CPU. Suspension between rests does not kill it; it resumes on the next wake, so a pass longer than any single wake completes. With that, how long a pass takes stops being a reason to defer an offload, and the measurement rule is gone: - a real update (an offload) runs in the background, paced; - a pass owed from a KILLED attempt still defers, as before; - a pass running in this process is not a killed one: its own started-mark reads as owed, and deferring on it recorded a newer debt the running pass could not settle (ryanbr#1681), so every later offload deferred. The trigger now reaches the engine, which re-arms one follow-up pass; - the backstop tick no longer runs in the background at all, since a paced pass costs minutes and every real update runs its own; - a processing task that finds a pass already running leaves it to settle its own debt. The measured duration is still banked and logged.
|
Reviewed the top commit only, as asked. The diagnosis is well evidenced and the pacing is carefully built. Notes below are questions rather than objections, and the draft gate is in the right place. What convinces26 The 30 s cap earns its comment.
The load-bearing assumption
This is the claim the whole design rests on, and it is the one I cannot check from the code. Suspension is indeed survivable, but a pass running under an expiring
Both leave "no cpu_resource_fatal" true, and only the first means a backgrounded pass reliably finishes. Worth capturing whether the expiry handler fired at all on the good night. Wall clockAt 1:1 pacing a 144 s pass becomes roughly 288 s of wall clock before suspensions, and each of 21 nights can rest up to 30 s. Fine for a processing task; much less so for a short refresh wake. It would help to say which wake type the paced pass is expected to complete under, since "it resumes next wake" is doing a lot of work if the answer is usually "it never finishes in one". The part I would watch hardestRemoving duration as a reason to defer takes away the safety net at the same time as adding the thing that replaces it. If pacing turns out not to hold the limit on some device (a slower phone, a larger history, a pass that is more CPU-dense per night than this one), there is no longer a deferral path to catch it, and the symptom returns as kills rather than as delays. A line in the log when a pass exceeds some multiple of its expected wall clock would make that visible without reintroducing the rule. Draft gateHolding until an overnight run shows no new crash reports, Also worth noting for whoever sequences this: it is stacked on #2279, so that one needs to go first. |
|
Good news first: the parent has landed. #2279 is on main as Could you rebase onto current main before this goes any further? Two reasons, and the second is the one that matters. The parent went in as a SQUASH. The commit this branch carries ( The branch point is also well behind main now. Diffing To be clear, this PR is not proposing any of that. It is what a stale branch point looks like from main's side, and it is exactly the shape that lands as a silent revert if someone merges on a green tick without reading the file list. A rebase makes the diff say what the change actually is: one pacing commit. Worth knowing too: the 4/4 green on Nothing about the change itself is in question here. The review notes on the pacing still stand, as does the draft gate you set for the overnight run, and I would still like the expiry-handler question answered from that night: whether the pass genuinely resumed after suspension, or ran on detached past its grant. "No cpu_resource_fatal reports" reads the same either way, and only the first means a backgrounded pass reliably finishes. |
|
Landed as To explain the detour rather than leave it looking like your PR was bypassed: this branch was stacked on #2279, which merged as a SQUASH, so its parent commit was not an ancestor of main. Combined with a branch point well behind main, the diff from main's side read as roughly 2,110 deletions, and Your authorship is intact on the merged commit, I was only the committer, and I changed nothing in it. Verified before merging that the rebased hunks were byte-identical to yours and that the auto-merge had not disturbed anything in the files main had moved under you. Closing this one as superseded rather than stale. Two things carried into the squash message so they do not disappear with this PR:
Excellent diagnosis on this one. Reading 26 crash reports against a 144 s pass and concluding that the earlier remedy had only moved where the kill happened is the part that was hard to get right. |
|
Thanks for landing it via #2296, and for checking the hunks were byte-identical. Here is the overnight result I owed you, with what it does and does not settle. What the night showed (paced build installed 2026-09-16 evening, one iPhone 16):
Why those passes were hours long at all turned out to be a separate problem, now #2293:
Replayed against a copy of that phone's database: a pass with nothing new went from 96 s to 2.9 s, and one with a minute of new heart rate from 16 s to 7.9 s. So pacing was holding the CPU limit, but the work being paced was mostly redundant. On the safety net: agreed that duration-based deferral is gone. Two things now make its absence visible:
One more data point: that phone was running a Debug ( |
2 participants
What this PR does
#1538 read the killed background passes as work that could not finish inside one wake, so it deferred any pass slower than 20 s to a processing task. The device's own crash reports point to a different cause: iOS's background CPU limit.
Across five nights, one iPhone 16 (WHOOP 5.0) logged 26
NOOP Staging.cpu_resource_fatal-*.ipsreports, each one:On that install a cold pass is about 144 s of near-continuous CPU (
re-score: done — scored 21 night(s) in 144311 ms,dayCache reused=0/21). Every background attempt was killed about 52 s in, including the processing-task attempts it had been deferred to, which fired every ~30 min between 00:28 and 04:00. Deferring only moved where the kill happened. The night's scores appeared when the app was next opened.The fix: in the background, a pass now rests after each night, in both the pass-1 and score2 loops, for as long as that night's work took (capped at 30 s), which keeps it near 50% CPU. A suspension between rests doesn't kill the pass; it picks up on the next wake, so a pass longer than any single wake still completes.
Once a pass can't be killed for running long, its duration is no longer a reason to defer, so the measurement rule is removed:
analyzeRecent's #1538 background-rescore "owed" flag can be cleared by an unrelated pass — a night's sleep score can silently freeze on stale data #1681), so every later offload deferred as well. The trigger now reaches the engine, which re-arms one follow-up pass.Type of change
How it was tested
RescoreBackgroundPolicyTests(rewritten around the new rules and pacing),RescoreBackgroundSchedulerTests(updated, plus a running-pass case),IntelligenceForcedRescoreRearmTests: 36 tests pass.NOOPiOSbuilds for device and is installed on the reporting iPhone.cpu_resource_fatalreports,re-score: donewhile backgrounded, and the night in Apple Health by morning without opening the app. I'll post the strap-log lines and crash-report listing here.Checklist
docs/CONTRIBUTING.md(doc_comment_lint.pyclean; parity-governance suite and ratchet clean)Strand.xcodeproj/) or any secrets/keystoresRelated issues
Refs #1538, #1681, #2238