[Performance] Reduce multicast renderer contention#82
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d5fa55c0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
NachoSoto
added a commit
that referenced
this pull request
Jul 12, 2026
## Summary This branch combines the independently reviewed changes from: - #77 — skip unchanged UIKit image sizes - #78 — streamline SwiftUI geometry observation - #79 — preserve `UIImage.scale` in the disk cache - #80 — remove the redundant image-scheduler hop - #81 — lazily initialize SwiftUI image state - #82 — reduce multicast contention while preserving subscription semantics ## Component benchmarks | Change | WatchChess workload | Result | |---|---|---:| | UIKit unchanged-size guard (#77) | 128,000 origin-only mutations | **-32.24% mean**, scheduler calls 128,000 → 0 | | SwiftUI geometry (#78) | 100 real cells × 80 relayouts | **-2.86% mean**, 95% CI for saving 9.053–45.532 ms | | Scheduler hop (#80) | 96 cold + 96 repeated requests | scheduler operations **-50%**; wall time neutral | | Lazy SwiftUI state (#81) | 20,000 body reconstructions | **-4.44% mean**, 12/12 paired wins | | Multicast completed hits (#82) | 50,000 real `PieceView` hits | **-89.3% / 9.33× faster** | | Multicast distinct keys (#82) | 24 concurrent keys | neutral: -1.4% mean, 95% CI spans zero | ## Combined WatchChess benchmark Release WatchChess, iPhone 17 Pro Max / iOS 26.5 simulators, 10 samples per variant and flow. Negative CPU/hitch changes are improvements. ### CPU | Flow | CPU time, baseline → combined | Change (paired-bootstrap 95% CI) | Mean one-core-equivalent CPU | |---|---:|---:|---:| | Tournament-list scrolling | 2.230 → 2.264 s | +1.54% (-2.79% to +6.09%): neutral | 12.01% → 12.07% | | Rounds scrolling | 1.059 → 1.024 s | **-3.25% (-5.55% to -1.27%)** | 13.03% → 12.79% | | Open game | 0.452 → 0.439 s | **-2.89% (-5.36% to -0.12%)** | **13.47% → 13.11%** (-2.63%; CI -4.37% to -0.87%) | ### FPS and hitches `FPS` below is a main-run-loop `CADisplayLink` callback proxy over the exact UI-action window, not compositor presentation FPS. | Flow | FPS, baseline → combined | FPS change (95% CI) | Hitches/min, baseline → combined | Hitch change (95% CI) | |---|---:|---:|---:|---:| | Tournament-list scrolling | 58.93 → 59.00 | +0.12% (-0.20% to +0.48%): neutral | 40.67 → 37.39 | -8.06% (-29.75% to +14.24%): neutral | | Rounds scrolling | 58.53 → 58.98 | **+0.77% (+0.13% to +1.39%)** | 63.24 → 51.68 | -18.28% (-35.36% to +5.33%): trend, CI spans zero | | Open game | 54.14 → 54.07 | -0.14% (-1.20% to +0.91%): neutral | 131.04 → 145.44 | +10.99% (-5.26% to +29.68%): neutral | Missed-refresh rates were also neutral for tournaments (-3.62%, CI -21.79% to +14.17%) and game (+2.99%, CI -5.93% to +13.06%). Rounds trended lower (-26.69%, CI -47.25% to +4.29%). ## Methodology and controls - two-pass, pass-outer crossover: pass 1 baseline on simulator A then candidate on B; pass 2 candidate on A then baseline on B - five measurements per pass, variant, and flow; 10 total per variant/flow - engine analysis forced off (`currentEngineStrength = 1`) and validated inside every app launch/report - exact 24-entry real API fixture in cache-only mode; any cache miss failed the run - identical 147-path warm AsyncImageView cache coverage per variant - exact AsyncImageView path/hash/mtime validation before sample 1 and after every five-sample block - API path/size/hash validation before and after every block - arithmetic-mean CPU headline; duration-weighted ratio-of-sums FPS/hitch rates - deterministic 100,000-resample, pass-stratified paired bootstrap confidence intervals - symmetric full cache hashing before each measured block; these are deliberately warm-cache measurements - hitches: clipped display-link interval at least 1.5× its target refresh period All earlier measurements with uncontrolled engine state or unequal cache warmness were quarantined and excluded. ## Rendering and cache regression checks - cold game and rounds screenshots were pixel-identical; cold tournament app content was pixel-identical outside dynamic system chrome - all four pass-1 seed/verify tournament screenshots were pixel-identical across the 1320×2590 app region; the only difference was 5,708 pixels in the translucent system search-field blur - six gated 8 FPS recordings covered tournament scrolling, rounds scrolling, and opening a game for both variants - manual contact-sheet review found no blank flashes, stale substitutions, missing portraits, or persistent placeholders; both game recordings showed the same normal transition placeholders and matching fully rendered end states - all six recordings passed pre/post AsyncImageView and API cache audits - baseline and candidate kept identical 147 relative cache paths; each variant's hashes and mtimes remained stable The different cache bytes between variants are intentional: #79 stores image scale in a versioned envelope. Legacy raw-PNG entries become one normal miss and regenerate in the corrected format. Existing remote images may therefore re-download once; offline users can temporarily miss legacy cached images until connectivity returns. ## #79 and #82 semantics #79 and the [#82 synchronous-subscriber review finding](#82 (comment)) are different fixes, and both are included: - #79 fixes PNG disk-cache scale metadata. - #82 attaches the first subscriber before starting synchronous upstream work, preserving first-request miss / repeat-request hit semantics. #82's `lifetime.observeEnded` intentionally holds the wrapper and in-flight entry only until producer completion or cancellation. Weak-reference tests verify release after both paths, while cancellation still disposes upstream work; no retain cycle or leak was observed. ## Validation - `swiftlint --config .swiftlint.yml --strict`: 0 violations - full iOS simulator suite: 69 XCTest/Quick + 13 Swift Testing tests = **82 passed** - focused multicast concurrency, synchronous, retry, memory-warning, cancellation, and weak-lifetime coverage - WatchChess Release cache preflights, 60 CPU samples, 60 frame reports, and six visual recordings passed their evidence checks - CI updated to macOS 26 / Xcode 26.5
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
1 participant
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
Performance
Isolated AsyncImageView benchmarks
WatchChess integration benchmark
Release-hosted WatchChess benchmark with real
PieceViewrender data and 10 strictly alternating samples per variant:Hot-hit paired mean saving was 236.892 ms (95% CI 234.031–239.753 ms). Updated distinct-key paired saving was 0.650 ms (95% CI -1.692 to 2.993 ms), so that workload has no measurable wall-time change after the safe subscriber-lifetime handling.
Rendering and cache parity
Every measured sample preserved output signatures and upstream counts. Final WatchChess correctness also verified:
Weak-reference tests prove the renderer is released after both active-producer completion and cancellation; ReactiveSwift drops the intentional lifetime hold when that lifetime ends.
A pre-existing PNG disk-reload scale-metadata loss remains identical here: the same 88×88 pixels reload as 88 pt @1× instead of 44 pt @2×. PR #79 fixes that separately.
Validation
swiftlint --config .swiftlint.yml --strict1416481