perf: size hosted menu charts without a throwaway hosting controller - #1352
Conversation
Each hosted chart/submenu item built TWO SwiftUI hierarchies on every (re)build: the MenuHostingView that is actually displayed, plus a separate NSHostingController created solely to measure height via sizeThatFits. That controller's result was always immediately overwritten by the subsequent refreshHostedSubviewHeights() fittingSize pass (run from both menuWillOpen and refreshHostedSubviewMenu), so the second hierarchy was pure overhead on a popup-menu hot path that scales with provider/account count. Measure the live displayed view via fittingSize instead (the same mechanism refreshHostedSubviewHeights already uses), via a shared hostedSubviewFittingHeight helper. Final heights are unchanged; only the redundant SwiftUI tree is removed. Adds a test asserting the append-path height matches the authoritative re-measure across chart types/providers. refs steipete#1321 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Codex review: needs maintainer review before merge. Reviewed June 7, 2026, 12:25 PM ET / 16:25 UTC. Summary Reproducibility: Source-reproducible: current main clearly builds a second hosting controller for hosted chart measurement, and #1321 has multiple user reports of 0.32.x menu lag. I did not reproduce the full multi-account lag 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:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge after normal maintainer review and passing checks as a narrow hot-path reduction that complements, but does not by itself close, the broader menu-lag work. Do we have a high-confidence way to reproduce the issue? Source-reproducible: current main clearly builds a second hosting controller for hosted chart measurement, and #1321 has multiple user reports of 0.32.x menu lag. I did not reproduce the full multi-account lag locally. Is this the best way to solve the issue? Likely yes: reusing the displayed view's existing AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against db184430bc4b. 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
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Added runtime proof to the PR body: a direct measurement on the real |
|
🦞👀 Command router queued. I will update this comment with the next step. |
clawsweeper
Bot
added
proof: sufficient
2 participants
Summary
Follow-up to the popup-menu lag reported in #1321, focused on the hosted chart submenus (cost/usage/credits history, storage breakdown, z.ai hourly, usage breakdown).
Every hosted chart item built two SwiftUI hierarchies on each (re)build:
MenuHostingViewthat is actually displayed, andNSHostingController(rootView:)created solely to measure height viasizeThatFits.The controller-measured height was always immediately overwritten by the subsequent
refreshHostedSubviewHeights()fittingSizepass — which runs from both call paths (menuWillOpenafter hydrate, andrefreshHostedSubviewMenu). So the second SwiftUI tree was pure overhead on a popup-menu hot path that scales with provider/account count and re-runs whenever provider data changes while a chart submenu is open.This change measures the live displayed view via
fittingSizeinstead (the exact mechanismrefreshHostedSubviewHeightsalready uses), extracted into a sharedhostedSubviewFittingHeight(for:width:)helper. Final heights are identical; only the redundant hosting-controller hierarchy is removed.Why it's safe:
refreshHostedSubviewHeightsre-measures it on every open/refresh today).Proof
Runtime measurement of the removed work, on the real
CostHistoryChartMenuView(30 daily entries, menu-card width 310, 400 builds, warm), comparing the two paths directly:That ~0.5 ms is per chart-submenu build, on the main thread, and recurs on every open/refresh while a chart submenu is visible — multiplied across enabled providers/accounts, which is exactly the multi-provider setup the #1321/#1325 reporters describe.
Correctness: the new regression test asserts the height the append path assigns equals the authoritative
refreshHostedSubviewHeightsre-measure (and is non-trivial) across cost-history / usage-history / storage-breakdown — i.e. dropping the throwaway controller did not change sizing behavior.(What I can't provide locally: an end-to-end popup recording on the reporters' exact multi-account machine. The change is a strict reduction of main-thread SwiftUI work on the documented hot path, with identical output.)
Test plan
swift buildmake check(SwiftFormat + SwiftLint, 0 violations)hosted chart items size to the displayed view without a throwaway controller— asserts the append-path height matches the authoritativerefreshHostedSubviewHeightsre-measure (and is non-trivial) across cost-history / usage-history / storage-breakdown for a seeded providerStatusMenuHostedSubmenuRefreshTests,StatusMenuOpenRefreshTests,StatusMenuHeightCacheTests(41 tests) passNotes
Complements #1351 (menu readiness signature cost). Touches the hosted-submenu sizing path that overlaps @hhh2210's recent menu-card height-cache work, so review from that area owner is welcome.
Made with Cursor