Repository navigation
perf(ios): draw a tail of the strap log, not all 5,000 lines - #2546
Merged
Merged
Conversation
The Live card rendered the whole log buffer in a non-lazy VStack over `Array(live.log.enumerated())`, so every appended line allocated a fresh 5,000-element array and built and diffed every row. The viewport is 200pt, about fifteen lines. A history drain plus a re-score burst emits hundreds of lines a minute, the CPU limit is per process rather than per thread, and iOS killed the app for 85% of a core over 57 seconds while it was not frontmost. Now the card draws the last 200 lines in a LazyVStack, and nothing at all while the scene is backgrounded. The buffer is untouched at 5,000 lines and Copy, Save and the export still read the archive from disk, so nothing is lost by drawing less. Two details that decide the shape: The tail is a SLICE, not a range. A LazyVStack realizes rows in a later main-actor turn than the body that produced them, so a range indexed back into the live buffer could read past the end after a trim. A slice is a copy-on-write snapshot and stays valid however the buffer moves. Its indices are ABSOLUTE, so between trims an append shifts only the window edges and every shared row keeps its identity. `scrollTo(log.indices.last)` therefore always addresses a row that was built, which a leading window would not. Gated on `.background` rather than `!= .active`: `.inactive` is when iOS takes the app-switcher snapshot, and blanking the log there would be visible for no gain. The kill needs sustained non-frontmost CPU. Reported by @pipiche38, who traced it to this card from a symbolicated microstackshot. Leaves the third suggestion in that report alone. `LiveState` carries 63 `@Published` properties, so a log line still invalidates every observer; splitting it is an architectural change, not this fix. What this changes is the cost of each invalidation.
Re-review of the previous commit. Two accuracy problems in my own work, no behaviour change. The comment claimed the backgrounded body is made "trivial". It is not. The header, the Divider and the NavigationLink are still built per appended line, and NavigationLink(destination:) constructs TestCentreView() eagerly, which evaluates its @State defaults and so does three UserDefaults reads. Measured against the five thousand Text views this removes that is microseconds, but someone profiling a surviving background cost needs to be pointed at it rather than told there is nothing there. The test file was named for a range. The helper returns a slice, which is the whole point of it, so the name pointed at the design that was rejected.
The macOS leg refused to compile the test bundle: `LiveState` is `@MainActor`, so the new static inherited that isolation and a plain XCTestCase could not call it. Nine call sites, one error each. `nonisolated` is the right answer rather than marking the test `@MainActor`, because the function IS actor-independent: it is pure over its arguments and touches nothing on the instance. It now matches the convention already in that file, where `redactPii`, `logSafeDeviceName`, `redactHexDump` and `scheduledExportText` are all nonisolated statics for the same reason. `swiftc -parse` cannot see this. It parses, it does not type-check or check isolation, so on a Linux host the macOS leg is the first thing that could have caught it.
ryanbr
added a commit
that referenced
this pull request
Sep 28, 2026
`LiveState` carries dozens of `@Published` properties on one `ObservableObject`, and an `ObservableObject` invalidates EVERY observer on ANY published change. So `@Published var log` woke every view in the app once per appended line, and a history drain plus a re-score burst emits hundreds a minute. That was the invalidation half of the background CPU kill in #2521; #2546 cut the cost of each invalidation, this cuts how many there are to at most four a second. The Android twin has throttled this for a while, with `boundedRevision(ble.logRevision, coalesceMs = 250)` in TestCentreScreen.kt. Same interval here, so the two platforms behave alike. Coalesces the PUBLISH, never the capture. `log` becomes a synchronous computed read of a non-published buffer, so code that appends and then inspects the log in the same turn still sees its own line, which many tests do. The disk archive and the #990 drained-rows tally stay per line and run before the publish. Leading edge plus a trailing flush, so the first line after an idle gap is immediate and the last line of a burst always lands rather than waiting for a line that may never come. The decision is a pure `nonisolated static func` returning an enum, so the policy is pinned without a clock or a run loop, and the async half is a thin adapter with no decisions of its own. Two isolation details, because a `static` member of a `@MainActor` type is actor-isolated: the interval constant is `nonisolated` so the nonisolated decision function can use it as a default argument, matching `legacyTailKey` in the same file. The card now observes `logRevision` rather than `log.count`. The revision is monotonic and ticks once per publish; the count plateaus while the ring trims, so it could stay equal across an append and skip a scroll.
5 tasks done
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.
Summary
The Live screen's strap-log card rendered the whole 5,000-line buffer in a non-lazy
VStackoverArray(live.log.enumerated()). So every appended line allocated a fresh 5,000-element array and built and diffed every row, while the viewport is.frame(height: 200), about fifteen lines. A history drain plus a re-score burst emits hundreds of lines a minute, the 80%-of-a-core limit is per process rather than per thread, and iOS killed the app for 85% of a core over 57 seconds while it was not frontmost.Addresses #2521, reported by @pipiche38 with a symbolicated microstackshot that traced the main thread into this card.
I verified the attached report independently. The figures match exactly (48s over 57s, limit 80% over 60s, 6 steps with 1 lost, iPhone18,1, iOS 27.0), and the main-thread shape is corroborated without needing symbols: the "Heaviest stack for the target process" is 4 samples rooted
dyld→ app → SwiftUI → UIKitCore → GraphicsServices → CoreFoundation, which is the main run loop. Every code claim in the report held.What changes
The card draws the last 200 lines in a
LazyVStack, and nothing at all while the scene is backgrounded.LiveState.maxLogLinesstays 5,000, and Copy, Save and the export still read the archive from disk, so nothing is lost by drawing less.Two details decide the shape, and both are the kind that pass review and then crash:
The tail is a slice, not a range. A
LazyVStackrealizes rows in a later main-actor turn than the body that produced them, on scroll, with no re-evaluation. A range indexed back into the live buffer would then read a bufferappend(log:)may have trimmed in between, out of bounds. AnArraySliceis a copy-on-write snapshot, so the rows it hands out stay valid however the buffer moves. I had written the range version first; it is the more obvious code and it is wrong.Its indices are absolute. Between trims an append shifts only the window edges, so every shared row keeps its identity and SwiftUI adds one row and drops one rather than re-identifying the list. It also means
scrollTo(log.indices.last)always addresses a row that was actually built, which a leading window would not. A trim does still renumber, becauseArray.removeFirstshifts every element down, but that is once pertrimSlack(256) lines and now touches 200 ids instead of rebuilding 5,000 rows.Gated on
.backgroundrather than!= .activedeliberately:.inactiveis also when iOS takes the app-switcher snapshot, and blanking the log there would be visible for no benefit. The kill needs sustained non-frontmost CPU.Being exact about what the backgrounded branch still costs, since a later reader profiling a residual will want pointing at it rather than told there is nothing there: the header, the
Dividerand theNavigationLinkare still built per appended line, andNavigationLink(destination:)constructsTestCentreView()eagerly, which evaluates its@Statedefaults and so performs threeUserDefaultsreads. Microseconds against the five thousandTextviews this removes, and left alone rather than converted to a deferred destination, which would change navigation construction semantics for a cost I measured as negligible.What this does not do
The report's third suggestion is untouched.
LiveStatecarries 63@Publishedproperties, so an appended log line still invalidates every view observing it; that is real, and splitting the object is an architectural change rather than part of this. What this changes is the cost of each invalidation, which is what the CPU limit measures.It also does not address the concurrency sample in the same report.
SleepStager.sessionHrvWindowsre-scans the whole R-R segment once per five-minute window, so an eight-hour night is about 96 passes over the night's beats, and it is called about five times per sleep session peranalyzeDay. That is a separate change to a hot path that produces shipped HRV values, so it deserves its own PR with an exact-output oracle.Parity
None needed, and I checked rather than assumed: Android exposes the strap log only through export, with no on-screen scrolling log card, so there is no twin of this view to fix.
Verification
swiftc -parseon all three files, andpython3 Tools/doc_comment_lint.py: clean.StrandTestscases onLiveState.renderedTail, including the two invariants above: that thescrollTotarget is always inside the rendered tail (a leading window passes every count assertion and fails this one), and that the snapshot survives the buffer being trimmed underneath it.