Merged
Conversation
Fast book-switch profiling on the Shelf showed `RepositorySectionView.body` running ~7400 times/sec under SwiftUI observation tracking — `openTabCount` inside the body subscribed every section to the entire `WorktreeTerminalManager.states` dictionary, which churns on any terminal activity. Combined with `ShelfView.body` rebuilding `worktreeRowSections` per call, a 25s fast-switching trace produced one Severe Hang (2.02s) plus 11 multi-hundred-ms Hangs. Changes: - RepositorySectionView: extract the tab-count badge into a dedicated leaf view (`RepoHeaderTabCountBadge`) so the parent body never reads `terminalManager`. Body invocations -92% in the rerun trace. - ShelfBook.orderedShelfBooks: replace `Dictionary(uniqueKeysWithValues:)` with `repositories[id:]` and route worktree ordering through `orderedWorktrees(in:)` to skip per-repo `WorktreeRowSections` model and Set construction. - ShelfView: drop the redundant TCA action animation (`store.send(..., animation:)`) on book open paths — the view-level `.animation(_:value: openBookID)` already drives the spine flow. - SupaLogger: add an `OSSignposter` plus `interval`, `beginInterval`/`endInterval` (token form for `inout`-bound paths), and `event` helpers — kept always-on since signposts are ~zero-cost when no Instruments session is attached. - Instrument hot paths: `WorktreeTerminalState.focusSelectedTab`, `syncFocus`, `applySurfaceActivity`; `ShelfOpenBookView.onAppear` / `onChange(selectedTabId)`; `RepositoriesFeature.selectWorktree` / `selectRepository` reducer cases; `ShelfView` book-click events. Verified via `make check`, `make build-app`, `make test` (928 passed).
After the perf push that landed in 0fe682c, several diagnostic signposts proved valuable enough to keep around — they cost nothing when no Instruments session is attached but make future regressions much easier to localize on the Points of Interest timeline. What this commit adds: - SupaLogger: signposter is now created with the well-known `"PointsOfInterest"` category so events surface in Apple's stock Points of Interest instrument without any custom subsystem filter (signpost names already carry origin granularity). - ShelfView.body / ShelfSpineView.body event markers — sanity-check body invocation cadence across animation transitions. - ShelfOpenBookView.onDisappear event — pairs with the existing onAppear interval so any future change to subtree mount/unmount cadence shows as a count delta. - GhosttyTerminalView.makeNSView / updateNSView intervals plus a dismantleNSView event override — counts and times the NSViewRepresentable lifecycle, the layer where book-switch teardown cost is most directly observable. Behavioural deltas: none — this is pure observability.
Closes the loop on the perf push that landed in 0fe682c / c9b852e. Captures the full investigation narrative — symptoms, hypotheses, the P0 wins (sidebar observability storm), the Phase 2 / animation experiments that did not pan out, methodology learnings around xctrace and Animation Hitches signpost filtering, and the long-term observability hooks that are now permanently in place. Intended as a starting reference if Shelf jank ever resurfaces.
This reverts commit eca655a.
This reverts commit 84f86c7.
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
This PR keeps the successful Shelf jank fixes from the investigation branch:
KeyPressModifierinvalidation path during book switchingForEach, eliminating matched-geometry churn between left/right spine stacksWhy
Book switching in Shelf mode was hitching heavily, especially when switching via keyboard shortcuts. Trace analysis showed a large SwiftUI invalidation storm from shortcut/key-press environment modifiers, followed by layout churn from the split spine rendering model.
The retained changes brought the most useful trace and UX improvements while avoiding the later experiments that introduced visible artifacts or degraded interaction.
Validation
make build-app 2>&1 | xcsift -f toonEnvironmentWriter: KeyPressModifierfrom the Shelf switching trace, eliminatedLayout: MatchedFrame, and reduced select-window hitch cost versus the earlier baseline.