Fix scroll jitter: filter display-safety once at ingestion#31
Open
castanley wants to merge 1 commit into
Open
Conversation
localHealthMetricRowIsDisplaySafe deep-scans every string in every metric row for forbidden platform-source markers, and the dailyRecoveryMetrics()/dailyActivityMetrics()/hourlyActivityMetrics() getters plus the static preferred*/trendRows(from:) helpers re-ran it on every call — i.e. on every SwiftUI body pass of Home and Health, 2-3x per row. An on-device Time Profiler trace (iPhone 17 Pro, release build, during a strap sync) attributed 22 s of a 42 s main-thread total to this scan; it is the dominant cause of the scroll jitter the README mentions, and it is worst while syncing. Metric rows are Swift value-type dictionaries with no stable object identity, so caches keyed on row/container identity never hit. Instead the scan runs exactly once, when packetInputReports is stored (didSet), into a per-family stored array; getters return it and the redundant re-filters in the from: helpers are removed (every caller passes getter-derived, already-safe rows). Extraction runs rarely, so display-safety filtering is off the render path entirely. Verified on hardware before/after: scrolling is smooth, including during sync.
bb08b5e to
8fb3abe
Compare
Author
|
Heads-up unrelated to this PR: |
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.
The forbidden-source-marker scan (localHealthMetricRowIsDisplaySafe) deep-reads every string in every metric row, and the dailyRecoveryMetrics()/dailyActivityMetrics()/hourlyActivityMetrics() getters plus the static preferred*/trendRows(from:) helpers re-run it on every call — every SwiftUI body pass of Home and Health, 2-3x per row.
Profiled on hardware (iPhone 17 Pro, release build, during a strap sync): 22 s of a 42 s main-thread CPU total was this scan. It is the dominant source of the scroll jitter in the README's performance note, and it is worst while syncing.
Metric rows are Swift value-type dictionaries with no stable object identity, so caches keyed on row/container identity never hit. This PR runs the scan exactly once — when packetInputReports is stored (didSet) — into a per-family stored array; getters return it and the redundant re-filters in the from: helpers are removed (every caller passes getter-derived, already-safe rows). Extraction runs rarely, so display-safety filtering is off the render path entirely.
Filtered output is identical; no behavior change. Verified on hardware before/after on a downstream fork: scrolling is smooth, including during sync. (Updated from an earlier per-render-cache version that didn't actually hit, for the value-type-identity reason above.)