Decode provider status feeds off the main actor#1406
Conversation
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Codex review: needs maintainer review before merge. Reviewed June 10, 2026, 10:13 PM ET / 02:13 UTC. Summary Reproducibility: yes. at source level: current main lacks an explicit concurrent-executor boundary for these UsageStore status helpers, and the linked Instruments report identifies this exact decode stack as a measured main-thread microhang. This read-only Linux review did not run the macOS app locally. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land the focused concurrent-executor and shared-formatter implementation after ordinary maintainer review and required checks, while continuing to track the separate merged-menu freeze mechanism in #1399. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main lacks an explicit concurrent-executor boundary for these UsageStore status helpers, and the linked Instruments report identifies this exact decode stack as a measured main-thread microhang. This read-only Linux review did not run the macOS app locally. Is this the best way to solve the issue? Yes. Explicitly moving the stateless asynchronous helpers onto the concurrent executor and reusing lock-guarded formatters is a narrow correction that preserves parsing and publication behavior without introducing a competing configuration or workflow. AGENTS.md: found and applied where relevant. Codex review notes: reasoning high; reviewed against 0e0102c30fe6. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
3ade9f4 to
4b8cf0a
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
UsageStore's status fetch/parse helpers are statics on a MainActor type, so the Google Workspace incidents feed (hundreds of kilobytes live) decoded on the main thread, stalling the UI 150-340ms per Google-status provider per refresh - refreshes that also fire during menu interaction (steipete#1399). The status helpers touch no store state, so they are now nonisolated and run on the concurrent executor, and the per-date-field ISO8601DateFormatter allocations are replaced with shared lock-guarded formatters (same pattern as CostUsageISO8601FormatterBox).
4b8cf0a to
7a96eb4
Compare
|
Landed as Proof:
Thanks @ProspectOre. |
Summary
Decode provider status feeds off the main actor and reuse shared ISO8601 formatters, removing a 150–340ms main-thread stall per Google-status provider per refresh.
Context
Credit to @kcharlan's instrumentation in #1399 (finding 4): with status checks on,
UsageStore.parseGoogleWorkspaceStatusdecodes the fullhttps://www.google.com/appsstatus/dashboard/incidents.jsonpayload (hundreds of kilobytes live) on the MainActor, allocating a freshISO8601DateFormatterper decoded date field. Their Instruments capture measured 150–340ms of main-thread stall per decode, once per Google-status provider (Gemini and Antigravity) per refresh — and refreshes fire during menu interaction. The single flagged microhang across their sessions was exactly this stack. The payload is live data, so the cost grows whenever Google has a busy incident week, on all app versions simultaneously.The same shape applies to the statuspage path (
fetchStatus), which is also MainActor-isolated with per-field formatter allocation; it is included for consistency.Change
fetchStatusandfetchWorkspaceStatusare explicitly@concurrent nonisolated, so feed fetching and decoding stay on the concurrent executor even if the package later adopts caller-inherited nonisolated async behavior. The pure parse helpers remainnonisolated; the caller (refreshStatus) already hops back to the main actor to publish results.ISO8601DateFormatter()allocations are replaced with shared lock-guarded formatters (StatusISO8601FormatterBox), mirroring the existingCostUsageISO8601FormatterBoxpattern in CodexBarCore.Validation
swift test --filter GoogleWorkspaceStatus— 4 tests pass, including a regression guard that records the executor immediately before JSON decoding begins.swift test— 3,532 tests in 402 suites pass.make check— 0 violations;git diff --checkclean.Runtime Proof
Live-feed artifact (terminal capture; decode of the real
https://www.google.com/appsstatus/dashboard/incidents.jsonpayload fetched June 10, 2026 — 420,521 bytes, matching the ~418 KB measured in #1399; 5 iterations per strategy, same process, this branch's packaged toolchain):~3.1× faster on the live payload (which happened to carry a real active Gemini incident that parsed correctly through the new path), and the entire cost now lands on a background executor instead of the main thread. The committed regression observes the exact pre-decode boundary rather than the actor-backed transport stub:
Synthetic stress check (699 KB fixture, 300 incidents x 4 dated updates, 10 iterations): 332.6–356.4 ms → 101.2–104.3 ms per decode, same ratio. The #1399 Instruments measurement (150–340 ms live) sits between these two payload sizes, consistent with both.
Honesty / scope notes
Timing harness (drop into Tests/CodexBarTests to reproduce)