native macOS codings agent orchestrator
docs(shelf): update jank investigation findings
+208
-188
+208
-188
doc-onevcat/shelf-jank-investigation.md
+208
-188
doc-onevcat/shelf-jank-investigation.md
···
1
1
# Shelf Book-Switch Jank Investigation
2
2
3
-
Last updated: 2026-04-28
4
-
Status: Closed for now — landed P0/P1 wins, no further structural change planned. Long-term signposts retained for future regressions.
3
+
Last updated: 2026-04-29
4
+
Status: Closed for now. The current branch keeps the fixes that improved trace data or UX without visible regressions, and drops the later experiments that introduced artifacts or reduced interaction quality.
5
5
6
6
## Summary
7
7
8
-
When switching books quickly on the Shelf, the app showed visible jank: dropped frames, occasional multi-second main-thread blocks, and a sluggish feel. After ~9 traces and four code commits we landed two real fixes that eliminated the worst behaviour (Severe Hangs caused by an observability storm) and confirmed that the residual ~250 ms-per-click cost is rooted in SwiftUI animation/layout work that we cannot reduce without giving up the spine-flow animation entirely.
8
+
Switching books quickly in Shelf mode showed visible frame drops, especially when switching with keyboard shortcuts. The first investigation pass found a large SwiftUI invalidation storm in the sidebar. The second pass narrowed the keyboard-path cost and simplified the Shelf spine layout.
9
9
10
-
Final shipped state:
10
+
The retained changes are:
11
11
12
-
- `RepositorySectionView` no longer subscribes to `WorktreeTerminalManager.states` — body invocations dropped from ~7400/sec to ~850/sec under the same workload.
13
-
- `orderedShelfBooks()` no longer rebuilds a `Dictionary` and a stack of `Set`s on every `ShelfView.body` call.
14
-
- The redundant TCA-action animation transaction on Shelf-originated book switches was removed; the view-level `.animation(value: openBookID)` continues to drive the spine flow.
15
-
- A long-term `OSSignposter` toolkit on `SupaLogger` plus a small set of body / lifecycle / NSView-representable signposts make future regressions much easier to localize on the Points of Interest timeline.
12
+
- `RepositorySectionView` no longer reads `WorktreeTerminalManager.states` just to render the tab-count badge. The read moved into a leaf view so invalidation stops at the badge.
13
+
- `orderedShelfBooks()` avoids per-body `Dictionary`, `Set`, and full `WorktreeRowModel` construction.
14
+
- Shelf-originated book switches no longer send an extra TCA animation transaction on top of the view-level `openBookID` animation.
15
+
- `CommandKeyObserver` only enters shortcut-hint mode for bare Command or bare Control, not every shortcut chord containing Command/Control.
16
+
- The sidebar key-forwarding `.onKeyPress` modifier is not installed while Shelf is active. This removed the `EnvironmentWriter: KeyPressModifier` invalidation path from the Shelf switching trace.
17
+
- Shelf spines are rendered from a single `ForEach`, removing the cross-list `matchedGeometryEffect` between left/right spine stacks.
18
+
- The open-book opacity transition was removed. Trace improvement was small, but the user reported no visible UX regression, so it is kept as a low-risk cleanup.
19
+
- Long-term signposts remain in place for future regressions.
16
20
17
-
What we tried but rolled back:
21
+
Experiments that were tried and rejected:
18
22
19
-
- Removing `.id(worktree.id)` from `ShelfOpenBookView` (Phase 2). Behaved as designed (subtree no longer rebuilt) but produced no measurable hitch reduction; reverted to keep the codebase smaller and to preserve `.transition(.opacity)`.
20
-
- Disabling the spine-flow animation entirely. Felt smooth in hand but did not actually reduce per-click work — it only removed the visual artefact. Reverted because the UX trade-off is not worth a perceptual-only win.
23
+
- Removing `.id(worktree.id)` from `ShelfOpenBookView`. It was behaviorally safe but gave no measurable win.
24
+
- Fully disabling or isolating the Shelf animation. It either only improved perception or caused visible terminal animation loss.
25
+
- Rendering the terminal/open area in an overlay outside the spine layout animation. This created a black-edge artifact during spine movement and made several SwiftUI cause counters worse.
26
+
- Removing context menus from closed-spine tab slots. This hurt interaction and did not improve hitches.
27
+
28
+
## Final Trace Shape
29
+
30
+
The final useful comparison line is run13/run15, after the keyboard-path and single-`ForEach` fixes. Hangs were treated as unreliable in this phase because they appeared and disappeared across similar runs; hitches and SwiftUI cause edges were more useful.
31
+
32
+
| Run | Main change | Selects | All hitches/select | Select-window hitches/select | Key SwiftUI result |
33
+
| --- | --- | ---: | ---: | ---: | --- |
34
+
| run10 | Rejected terminal animation isolation experiment | 25 | 296.5 ms | not used | UX regression; terminal animation disappeared |
35
+
| run11 | Bare Command/Control hint mode | 25 | 235.7 ms | 186.7 ms | `@Observable CommandKeyObserver.(Bool)` disappeared; `KeyPressModifier` fell to 116,284 source edges |
36
+
| run12 | Disable sidebar key forwarding in Shelf | 29 | 196.1 ms | 151.7 ms | `EnvironmentWriter: KeyPressModifier` fell to 0; `WorktreeRowsView.body [skipped]` fell sharply |
37
+
| run13 | Single `ForEach`, no matched geometry | 29 | 176.4 ms | 144.4 ms | `Layout: MatchedFrame` fell to 0; `LayoutChildGeometries` source edges fell from 111,421 to 4,279 |
38
+
| run14 | Rejected terminal overlay | 29 | 167.1 ms | 136.3 ms | Small hitch win, but `AnimatableFrameAttribute`, responders, and opacity renderer all worsened; visible black edge |
39
+
| run15 | Remove open-book opacity transition | 28 | 250.6 ms | 140.3 ms | Small select-window win; outside-window hitches dominated all-hitch total |
40
+
| run16 | Rejected context-menu scoping | 27 | 180.6 ms | 145.1 ms | Interaction regression; `View Responders` worsened |
41
+
42
+
`select-window hitches/select` is the preferred number for book-switch work because it only counts hitches between the first and last `reducer.selectWorktree` signpost. The "all hitches/select" number can be dominated by startup, recording, or unrelated post-workload spikes, as run15 shows.
21
43
22
44
## Problem
23
45
24
-
Quickly clicking between books on the Shelf produced obvious frame drops. Initial Instruments capture (Animation Hitches template, Debug build) showed the situation was severe:
46
+
Quickly switching between Shelf books produced obvious frame drops. Initial Instruments captures showed:
25
47
26
-
- 1 Severe Hang (2.02 s) plus 11 Hangs of 600–1000 ms each
27
-
- Main thread blocked for ~10.5 s out of a 25 s recording (~41 %)
28
-
- 460 234 SwiftUI updates in 25 s — i.e. ~18 000 view updates per second under steady fast-clicking
48
+
- severe hangs in early traces, including one multi-second main-thread block
49
+
- hundreds of thousands of SwiftUI update/cause edges in short recordings
50
+
- visible animation breakage during the 0.2 s spine-flow animation
29
51
30
-
These numbers said the issue was not "one slow function" but a runaway invalidation pattern at the SwiftUI layer.
52
+
The problem was not one slow reducer or one slow `NSViewRepresentable`. The reducer and Ghostty bridge signposts consistently measured in sub-millisecond ranges. The heavy work was mostly SwiftUI invalidation, layout, responder, and display-list work between our signposts.
31
53
32
54
## Investigation Timeline
33
55
34
-
### 1. Static analysis (no instrumentation yet)
56
+
### 1. Static Suspicions
35
57
36
-
Before touching Instruments we inspected the Shelf view tree and the `RepositoriesFeature` reducer. Suspicions filed in priority order:
58
+
The first static pass identified several plausible sources:
37
59
38
-
1. `ShelfView.body` recomputes `orderedShelfBooks()` on every body call. The implementation builds a `Dictionary(uniqueKeysWithValues:)` from `repositories` and routes worktree ordering through `worktreeRowSections(in:)`, which constructs a full `WorktreeRowModel` and several intermediate `Set`s per repository.
39
-
2. `.id(worktree.id)` on `ShelfOpenBookView` forces a full subtree teardown + rebuild on every book switch — including the Ghostty `NSViewRepresentable` wrappers.
40
-
3. The animation is double-applied: `ShelfView` has `.animation(.easeInOut(duration: 0.2), value: openBookID)` AND book-click handlers do `store.send(..., animation: .easeInOut(duration: 0.2))`.
41
-
4. Many spines each subscribe to `terminalManager.stateIfExists(for: book.id)`, which reads the entire `@Observable WorktreeTerminalManager.states` dictionary. Any terminal activity at all could fan out to every spine.
42
-
5. The `matchedGeometryEffect` bridging the left and right `ForEach` branches forces SwiftUI to interpolate spine identity across two separate lists.
60
+
1. `ShelfView.body` recomputes `orderedShelfBooks()` on every body call.
61
+
2. `.id(worktree.id)` on `ShelfOpenBookView` might force expensive subtree teardown.
62
+
3. Shelf book switching was applying animation twice: once in the TCA action send and once via `.animation(value: openBookID)`.
63
+
4. Sidebar repository rows read `terminalManager.stateIfExists(...)` while rendering tab counts.
64
+
5. The Shelf layout split spines across left/right `ForEach` lists and bridged identity with `matchedGeometryEffect`.
65
+
6. Keyboard-based switching might be over-invalidating shortcut hint UI through `CommandKeyObserver` and `.onKeyPress`.
43
66
44
-
Most of these turned out to be real but only some moved the needle.
67
+
Most of these were real; only some moved user-visible performance.
45
68
46
-
### 2. First Instruments capture and the smoking gun (Run 1, Debug)
69
+
### 2. Sidebar Tab-Count Subscription Storm
47
70
48
-
Animation Hitches + SwiftUI instrument template, Debug build, 25 s recording with rapid book clicking.
71
+
Early `swiftui-causes` data showed `RepositorySectionView.body` and `@Observable WorktreeTerminalManager.(Dictionary<String, WorktreeTerminalState>)` as top offenders.
49
72
50
-
Top "cause edges" in `swiftui-causes`:
51
-
52
-
| Source | Cause edges |
53
-
|---|---|
54
-
| `Layout: AnimatableFrameAttribute` | 233 177 |
55
-
| `@Observable WorktreeTerminalManager.(Dictionary<String, WorktreeTerminalState>)` | 196 968 |
56
-
| `RepositorySectionView.body` | **184 681** |
57
-
| `EnvironmentWriter: KeyPressModifier` | 164 594 |
58
-
| `Layout: LayoutChildGeometries` | 124 230 |
59
-
| `@Observable CommandKeyObserver.(Bool)` | 65 891 |
60
-
61
-
`RepositorySectionView.body` running ~7400 times per second was the smoking gun. The cause was traced back to:
73
+
The cause was:
62
74
63
75
```swift
64
76
RepoHeaderRow(
···
68
80
)
69
81
```
70
82
71
-
`openTabCount` iterates the repository's worktrees and calls `terminalManager.stateIfExists(...)` for each — so the body of *every* sidebar repository section subscribed to changes in `WorktreeTerminalManager.states` (which churns on any terminal activity), and *every* `tabManager.tabs` array within that. Any keystroke into a single terminal could fan out to the entire sidebar.
83
+
`openTabCount` iterated worktrees and called `terminalManager.stateIfExists(...)` from the parent sidebar row. That subscribed every repository section body to the global terminal state dictionary, so unrelated terminal activity fanned out through the sidebar.
72
84
73
-
### 3. P0 fixes (commit `0fe682cb`)
85
+
Fixes landed in `0fe682cb`:
74
86
75
-
Four changes landed together:
87
+
- Move tab-count reads into `RepoHeaderTabCountBadge`.
88
+
- Rewrite `orderedShelfBooks()` to use direct repository/worktree ordering.
89
+
- Remove the redundant Shelf-originated action animation.
90
+
- Add `SupaLogger` signpost helpers and focused Shelf/Ghostty signposts.
76
91
77
-
- **P0-1**: Extract the tab-count badge into a dedicated leaf view `RepoHeaderTabCountBadge`. The leaf reads `terminalManager` itself; the parent `RepositorySectionView` no longer touches it. SwiftUI's invalidation graph stops at the badge subtree, leaving the rest of the sidebar untouched.
78
-
- **P0-2**: Rewrite `orderedShelfBooks()` to use direct `IdentifiedArray` lookup (`repositories[id:]`) and route through the lighter `orderedWorktrees(in:)` rather than `worktreeRowSections(in:)`. Avoids per-repo `WorktreeRowModel` and `Set` allocations.
79
-
- **P1-1**: Drop the redundant `animation:` parameter from Shelf-originated `store.send` calls. The view-level `.animation(value: openBookID)` already covers Shelf and left-nav-originated changes.
80
-
- Also added `OSSignposter` to `SupaLogger` and instrumented the obvious suspects (reducer cases, focus/sync calls, book-click events).
92
+
Result:
81
93
82
-
Result in the next Release-build trace:
94
+
| Metric | Before | After |
95
+
| --- | ---: | ---: |
96
+
| `RepositorySectionView.body` cause edges | 184,681 | 14,456 |
97
+
| `WorktreeTerminalManager.states` cause edges | 196,968 | 14,456 |
98
+
| Severe hangs from this storm | present | gone |
83
99
84
-
| Metric | Before | After P0 | Δ |
85
-
|---|---|---|---|
86
-
| `RepositorySectionView.body` cause edges | 184 681 | 14 456 | **−92 %** |
87
-
| `WorktreeTerminalManager.states` cause edges | 196 968 | 14 456 | **−93 %** |
88
-
| Severe Hangs | 1 (2.02 s) | 0 | gone |
100
+
This was the largest unambiguous win.
89
101
90
-
Body-storm pattern eliminated. The 2 + second main-thread death observed at baseline never reappeared in any subsequent trace. **This is the unambiguous win of the entire investigation.**
102
+
### 3. `.id(worktree.id)` Experiment
91
103
92
-
### 4. The "still feels janky" plateau
104
+
Hypothesis: the residual cost came from `ShelfOpenBookView` teardown/remount on every book switch.
93
105
94
-
Despite P0 the user still felt visible jank. Subsequent Release traces showed:
106
+
We tried migrating focus logic into `onChange(of: worktree.id, initial: true)` and removing the outer `.id(worktree.id)`. This behaved correctly, but trace data did not improve. The likely reason is that a deeper `.id(node.structuralIdentity)` in the terminal split tree already forces the relevant Ghostty surface wrapper lifecycle work.
95
107
96
-
- Hitch budget remained roughly proportional to user clicks (~250–300 ms per click)
97
-
- Hangs in the 600–1000 ms range still occurred under fast clicking
98
-
- The dominant `swiftui-causes` work shifted to layout / animation / preferences:
99
-
- `Layout: AnimatableFrameAttribute` ~256 000
100
-
- `EnvironmentWriter: KeyPressModifier` ~240 000
101
-
- `Layout: LayoutChildGeometries` ~152 000
102
-
- `View Creation / Reuse` ~138 000
108
+
Decision: reverted. The extra code was not worth keeping.
103
109
104
-
These are SwiftUI-internal categories — nothing user-code we could optimize directly. We added more granular signposts to localize cost: `OpenBook.onAppear` / `onDisappear`, `Ghostty.makeNSView` / `updateNSView` / `dismantleNSView`, `ShelfView.body` / `ShelfSpineView.body` event counters.
110
+
### 4. Animation Experiments
105
111
106
-
The data was unambiguous and surprising:
112
+
Disabling the root Shelf animation made the UI feel much smoother, but traces showed the work mostly remained. This clarified that animation was a perception amplifier: missed frames are much more visible when the spine is supposed to glide.
107
113
108
-
| Signpost | Per-click avg / max |
109
-
|---|---|
110
-
| `reducer.selectWorktree` | 0.18 / 0.21 ms |
111
-
| `OpenBook.onAppear` | 0.10 / 0.18 ms |
112
-
| `Ghostty.makeNSView` | 0.33 / 0.60 ms |
113
-
| `focusSelectedTab` | 0.05 / 0.10 ms |
114
-
| `syncFocus` | 0.03 / 0.09 ms |
115
-
| `applySurfaceActivity` | 0.03 / 0.10 ms |
114
+
A later attempt to isolate book-switch animation around the spine stacks and disable it around the terminal area also failed product-wise. The terminal animation disappeared and the trace did not justify the UX loss.
116
115
117
-
**Total instrumented user-code work over a 22 s recording with 32 clicks: ≈25 ms.** The remaining ~250 ms-per-click was happening *between* our signposts, entirely inside SwiftUI's layout / animation / display-list pipeline.
116
+
Decision: keep the normal spine-flow animation.
118
117
119
-
### 5. Phase 1 + Phase 2 — remove `.id(worktree.id)`
118
+
### 5. Keyboard-Path Fixes
120
119
121
-
Hypothesis: the residual cost is the SwiftUI subtree teardown / remount triggered by `.id`, even though our own `makeNSView` is fast. By switching to identity-stable view reuse and migrating the focus-sync logic into `.onChange(of: worktree.id, initial: true)`, the heavy SwiftUI bookkeeping (attribute graph, preference values, accessibility nodes) for the entire ShelfOpenBookView subtree should not run per click.
120
+
The user reproduced the problem primarily with shortcuts. run8/run10 showed high `@Observable CommandKeyObserver.(Bool)` and `EnvironmentWriter: KeyPressModifier` cost.
122
121
123
-
Implemented in two phases for safety:
122
+
Two fixes were retained:
124
123
125
-
- Phase 1: add the new `onChange` path with verification-only signposts, leave `.id` in place. Trace showed `OpenBook.onChange.worktreeID` firing 1:1 with `OpenBook.onAppear` — confirmed equivalence.
126
-
- Phase 2: migrate logic, remove `.id`.
124
+
- `CommandKeyObserver.shouldShowShortcuts(for:)` now returns true only for bare Command or bare Control, not shortcut chords such as Control+number or Command+Shift.
125
+
- `SidebarListView` does not install its sidebar-to-terminal `.onKeyPress` forwarding modifier while Shelf is active.
127
126
128
-
Pre-implementation we worried that `GhosttySurfaceScrollView` stores its `surfaceView` as `private let` — meaning if SwiftUI reused the wrapper across book switches via `updateNSView`, the wrapper would still display the previous book's surface. Inspection of `TerminalSplitTreeView.swift:27` showed there is already an inner `.id(node.structuralIdentity)` on `SubtreeView` that forces dismantle + make whenever the split tree changes. So the surface-binding bug was a non-issue in practice. The user confirmed visually.
127
+
Results:
129
128
130
-
Result: behaviourally clean, but **no measurable perf gain**:
129
+
- run11 removed `@Observable CommandKeyObserver.(Bool)` from the hot SwiftUI causes.
130
+
- run12 removed `EnvironmentWriter: KeyPressModifier` from the Shelf switching path entirely.
131
+
- `WorktreeRowsView.body [skipped]` dropped from 26,466 in run11 to 2,853 in run12.
132
+
- select-window hitches/select improved from 186.7 ms in run11 to 151.7 ms in run12.
131
133
132
-
| Metric | With `.id` (Run 6) | Without `.id` (Run 8) |
133
-
|---|---|---|
134
-
| Hitches per click | 262 ms | 316 ms |
135
-
| Hitch share of recording | 37 % | 39 % |
136
-
| Severe Hangs | 0 | 4 (system-pressure variance) |
134
+
The behavior trade-off is narrow: while Shelf is active and the sidebar has focus, ordinary typed characters are no longer forwarded into the selected terminal. Direct terminal input is unaffected.
137
135
138
-
Phase 2 was reverted in commit `c9b852eb`'s parent state. Lesson: the inner `.id(node.structuralIdentity)` was already forcing the heavy work, the outer one was redundant overhead but not the dominant cost.
136
+
### 6. Single-`ForEach` Shelf Layout
139
137
140
-
### 6. Animation off — the perceptual experiment
138
+
The original Shelf layout split spines into left and right stacks:
141
139
142
-
Single-line change: `.animation(.easeInOut(duration: 0.2), value: openBookID)` → `.animation(nil, value: openBookID)`.
140
+
```swift
141
+
left spines
142
+
open book area
143
+
right spines
144
+
```
143
145
144
-
User reported: "去掉动画确实完全不卡了". But the trace told a different story:
146
+
Because a book moved between the two `ForEach` subtrees when the open index changed, the code used `matchedGeometryEffect` to bridge identity.
145
147
146
-
| Metric | Animation on (Run 8) | Animation off (Run 9) |
147
-
|---|---|---|
148
-
| Hitches per click | 316 ms | 256 ms |
149
-
| Hangs | 5 | 20 |
150
-
| Hang share of recording | 83 % | 51 % |
148
+
The retained rewrite renders all spines from a single `ForEach(books)` and inserts the open book area after the open book. This lets normal SwiftUI diffing preserve spine identity without matched geometry.
151
149
152
-
Per-click hitch barely moved. Total main-thread blocked time even *increased* in absolute terms, with many Hangs covering windows where multiple clicks landed inside one block.
150
+
run13 results:
153
151
154
-
The insight: **the spine-flow animation is the perceptual jank amplifier**, not the cause. When animation is running, every missed frame produces a visible "broken animation" artefact. When animation is off, the same amount of main-thread work just becomes a small response latency that the user reads as "instant click registered, slight delay before redraw" rather than "jank".
152
+
| Metric | run12 | run13 |
153
+
| --- | ---: | ---: |
154
+
| select-window hitches/select | 151.7 ms | 144.4 ms |
155
+
| SwiftUI cause rows | 1,303,129 | 1,026,467 |
156
+
| `Layout: MatchedFrame` source | 18,030 | 0 |
157
+
| `Layout: LayoutChildGeometries` source | 111,421 | 4,279 |
158
+
| `View Responders` dest | 102,928 | 69,228 |
159
+
| opacity renderer dest | 72,073 | 57,078 |
155
160
156
-
Without further structural rewrites (e.g. lazy-rendered spines, single-`ForEach` layout to drop `matchedGeometryEffect`, or rendering spines with `Canvas` instead of one `View` each), the per-click work appears to be roughly constant.
161
+
Decision: kept. The trace win was moderate and the user reported a clear subjective improvement.
157
162
158
-
### 7. Final decision — Option C
163
+
### 7. Terminal Overlay Experiment
159
164
160
-
Restore everything that didn't have a measurable win:
165
+
Hypothesis: keep spines animating, but remove the real terminal subtree from the animated `HStack`. Use a lightweight placeholder in the layout and render the terminal in an overlay at its final frame.
166
+
167
+
run14 showed a small hitch win, but the approach had two problems:
168
+
169
+
- Visual artifact: during spine movement the terminal overlay was already at the new position, while the old layout area exposed the window background, producing a black edge.
170
+
- SwiftUI causes got worse:
171
+
- `AnimatableFrameAttribute` source increased from 135,581 to 164,827.
172
+
- `View Responders` dest increased from 69,228 to 75,033.
173
+
- opacity renderer dest increased from 57,078 to 70,036.
174
+
175
+
Decision: reverted. The small hitch improvement did not justify the artifact and added complexity.
176
+
177
+
### 8. Open-Book Opacity Transition Removal
178
+
179
+
Removing the explicit `.transition(.opacity)` from `openBookArea` produced only a small win:
180
+
181
+
| Metric | run13 | run15 |
182
+
| --- | ---: | ---: |
183
+
| select-window hitches/select | 144.4 ms | 140.3 ms |
184
+
| select-window max hitch | 125.0 ms | 116.7 ms |
185
+
| `AnimatableFrameAttribute` source/select | 4,675 | 4,588 |
186
+
187
+
Several opacity-related SwiftUI causes did not improve after normalization, so this is not a major root-cause fix. However, the user reported no visible behavior regression, so it was kept as a simple low-risk improvement.
188
+
189
+
### 9. Closed-Spine Tab Context Menu Experiment
190
+
191
+
Hypothesis: closed spines did not need a full tab context menu, and removing it might reduce `View Responders` and the `TerminalTabContextMenu` view-list cost.
192
+
193
+
Implementation: only the open spine retained full tab context menus.
194
+
195
+
Result: rejected.
196
+
197
+
- select-window hitches/select regressed from 140.3 ms to 145.1 ms.
198
+
- `View Responders` dest/select worsened from 2,296 to 3,133.
199
+
- The old `ModifiedContent<ShelfSpineTabSlot, TerminalTabContextMenu>` shape was replaced by `_ConditionalContent<ModifiedContent, ShelfSpineTabSlot>`, so SwiftUI still had view-list complexity.
200
+
- Closed-spine right-click behavior got worse.
201
+
202
+
Decision: reset away. Do not optimize context menus by conditionally changing the child view type.
203
+
204
+
## What Worked vs What Did Not
205
+
206
+
| Change | Outcome |
207
+
| --- | --- |
208
+
| Sidebar tab-count leaf view | Large win. Removed the biggest invalidation storm. Kept. |
209
+
| Faster `orderedShelfBooks()` | Small/free hot-path cleanup. Kept. |
210
+
| Remove redundant action animation | Small/free cleanup. Kept. |
211
+
| Signpost toolkit and focused signposts | Made every later trace tractable. Kept. |
212
+
| Bare Command/Control shortcut-hint mode | Clear win for keyboard switching. Kept. |
213
+
| Disable sidebar `.onKeyPress` while Shelf is active | Clear win; removed `KeyPressModifier` from Shelf switching trace. Kept. |
214
+
| Single `ForEach` Shelf layout | Moderate trace win and good subjective win. Kept. |
215
+
| Remove open-book opacity transition | Small trace win, no observed UX cost. Kept. |
216
+
| Remove outer `.id(worktree.id)` | No measurable win. Reverted. |
217
+
| Disable/isolate animation | Either perceptual-only or visual regression. Reverted. |
218
+
| Terminal overlay outside layout animation | Small hitch win but black-edge artifact and worse causes. Reverted. |
219
+
| Closed-spine context-menu scoping | Worse trace and worse UX. Reset away. |
161
220
162
-
- `.id(worktree.id)` back, focus logic back in `.onAppear`, Phase-2-only `onChange` path removed
163
-
- Animation back to `.easeInOut(duration: 0.2)`
221
+
## Current Conclusions
164
222
165
-
Keep what is unambiguously good:
223
+
1. **The worst problem was not the terminal.** Reducer work, Ghostty `makeNSView`, focus, and sync signposts are all tiny relative to the hitch windows.
166
224
167
-
- All four P0/P1 fixes
168
-
- The `OSSignposter` toolkit on `SupaLogger` (`PointsOfInterest` category for stock-instrument visibility)
169
-
- Body counters and lifecycle signposts as long-term observability — they cost nothing when no Instruments session is attached
225
+
2. **The first major class was invalidation fan-out.** Sidebar tab counts, shortcut hints, and `.onKeyPress` environment machinery were multiplying work across unrelated views.
170
226
171
-
Net commits on `perf/shelf-jank-fixes`:
227
+
3. **The second major class is SwiftUI layout animation.** After invalidation storms were removed, the dominant costs became `AnimatableFrameAttribute`, `External: Time`, `View Responders`, and display-list renderer effects. These are consequences of animating a dense interactive SwiftUI tree.
172
228
173
-
- `0fe682cb` perf(shelf): cut sidebar tab-count subscription storm and add signposts
174
-
- `c9b852eb` chore(shelf): add long-term performance signposts
229
+
4. **Hangs were too unstable to use as the deciding metric in later runs.** The same user-visible workload could show very different hang counts. Hitches and normalized SwiftUI cause edges were more reliable.
175
230
176
-
## What Worked vs What Didn't
231
+
5. **Animation changes must be judged by both trace and eye.** Disabling animation can feel much better while barely moving work. Conversely, moving terminal rendering out of layout improved hitches slightly but created visible artifacts.
177
232
178
-
| Change | Cost | Outcome |
179
-
|---|---|---|
180
-
| **P0-1**: leaf view for sidebar tab count | ~30 lines | RepositorySectionView body invocations −92 %, killed the Severe Hang completely. **Kept.** |
181
-
| **P0-2**: rewrite `orderedShelfBooks()` | ~20 lines | Eliminated per-frame Dict/Set allocations on the ShelfView hot path. **Kept.** |
182
-
| **P1-1**: drop redundant action animation | 4 line touch | Removed double-animation transactions. Marginal but free. **Kept.** |
183
-
| `OSSignposter` toolkit + signposts | ~80 lines, multiple files | Made every subsequent investigation tractable. **Kept.** |
184
-
| Phase 2: remove `.id(worktree.id)` | ~30 lines | No measurable perf gain; lost `.transition(.opacity)`. **Reverted.** |
185
-
| Animation duration 0.2 s → 0.1 s | 1 line | Hitch per click 285 → 244 ms (−14 %). Not enough to justify UX change. **Reverted.** |
186
-
| Animation off entirely | 1 line | Felt smooth but trace showed work was unchanged. UX trade-off not worth perceptual-only win. **Reverted.** |
233
+
6. **Stable view shape matters.** The closed-spine context-menu experiment showed that replacing one expensive modifier with a conditional child shape can shift cost instead of reducing it.
187
234
188
235
## Methodology and Tooling Learned
189
236
190
237
### `xctrace` from the command line
191
238
192
-
`/usr/bin/xctrace` is a stub; the real tool ships inside Xcode at `/Applications/Xcode-26.4.1.app/Contents/Developer/usr/bin/xctrace`. Trying to inspect a trace recorded by Xcode 26.x with the system stub produces `Cannot load existing document because of an error: Missing features` — silent and confusing. Always use the Xcode-bundled binary:
239
+
`/usr/bin/xctrace` is a stub; use the Xcode-bundled tool. In this investigation the working path was:
193
240
194
241
```bash
195
242
XCT="/Applications/Xcode-26.4.1.app/Contents/Developer/usr/bin/xctrace"
196
243
197
-
# Discover what data is in the trace
198
244
"$XCT" export --input run.trace --toc --output toc.xml
199
245
200
-
# Pull a specific schema
246
+
"$XCT" export --input run.trace \
247
+
--xpath '/trace-toc/run[@number="1"]/data/table[@schema="hitches"]' \
248
+
--output hitches.xml
249
+
201
250
"$XCT" export --input run.trace \
202
251
--xpath '/trace-toc/run[@number="1"]/data/table[@schema="potential-hangs"]' \
203
252
--output hangs.xml
204
253
205
-
# Filter further by attribute (e.g. signposts under PointsOfInterest)
206
254
"$XCT" export --input run.trace \
207
255
--xpath '/trace-toc/run[@number="1"]/data/table[@schema="os-signpost"][@category="PointsOfInterest"]' \
208
256
--output poi.xml
209
-
```
210
257
211
-
Useful schemas that consistently held data:
212
-
213
-
- `potential-hangs` — main-thread block intervals with hang-type classification
214
-
- `hitches` — per-frame hitch events, often with a narrative description
215
-
- `os-signpost` — Begin/End/Event records, one table per attribute filter
216
-
- `swiftui-causes` — view dependency graph edges, the most useful single source for finding invalidation storms
217
-
- `swiftui-update-groups` — SwiftUI-internal update transactions with duration
258
+
"$XCT" export --input run.trace \
259
+
--xpath '/trace-toc/run[@number="1"]/data/table[@schema="swiftui-causes"]' \
260
+
--output swiftui-causes.xml
261
+
```
218
262
219
-
The XML export uses an aggressive `id`/`ref` interning pattern. To aggregate counts you must build a per-trace `id → label` table from the first definition and then resolve refs. A simple Python script walks the rows in a single pass; the bigger files (1 GB+ for `swiftui-updates` on a 25 s recording) needed buffered streaming.
263
+
The XML export interns values with `id`/`ref`. Aggregation scripts must resolve refs; a streaming parser is necessary for large `swiftui-causes` and `swiftui-updates` exports.
220
264
221
-
### Animation Hitches template's signpost filter is hard-coded
222
-
223
-
The stock Animation Hitches template includes an `os_signpost` instrument, but it's filtered to `subsystem == "com.apple.ConditionInducer.LowSeverity"` — i.e. only Apple's internal conditioning signposts. Our subsystem (`com.onevcat.prowl`) is not captured by default. Two ways to make app signposts visible in this template:
224
-
225
-
1. Add a generic `os_signpost` instrument to the trace document and configure it manually (subsystem filter or empty).
226
-
2. **Use the well-known `"PointsOfInterest"` category** for the signposter. The stock **Points of Interest** instrument is hard-coded to that category and shows app signposts without further configuration. This is what `SupaLogger` does now.
227
-
228
-
We learned (1) the hard way in Run 3 and (2) when Run 4's "Points of Interest" lane stayed empty even with our signposts emitted.
229
-
230
-
### Hangs vs Hitches
231
-
232
-
These are not the same thing and Animation Hitches reports them in two different lanes:
233
-
234
-
- A **Hang** is a contiguous main-thread block longer than the configured threshold (default 33 ms; surfaces as Microhang, Hang, or Severe Hang). Stack traces are sometimes attached.
235
-
- A **Hitch** is one or more consecutive frames missing vsync. Multiple frames missed in a row collapse into one hitch event with a longer duration. Hitches do not require the main thread to be blocked — GPU / render-server / commit-phase issues all surface as hitches.
236
-
237
-
A trace can have many Hitches and zero Hangs (frame production too slow to keep up with vsync but main thread always yielding within 33 ms — exactly what Run 4 looked like). It can also have many Hangs and very visible jank. The two metrics measure different aspects of "slow" and need to be read together.
265
+
### Signposts
238
266
239
-
### Signpost begin/end with `inout` state
267
+
`SupaLogger` emits signposts under subsystem `com.onevcat.prowl` and category `PointsOfInterest`. This category is visible in the stock Points of Interest instrument.
240
268
241
-
`SupaLogger.interval(_:_:)` takes a closure, but TCA reducer cases bind `state` as `inout` and Swift cannot capture `inout` parameters in a non-escaping closure. The workaround is a manual `beginInterval` / `endInterval` token API:
269
+
Reducer code that mutates `inout state` should use manual begin/end tokens:
242
270
243
271
```swift
244
272
case .selectWorktree(let id, let focusTerminal):
245
273
let token = repositoriesLogger.beginInterval("reducer.selectWorktree")
246
274
defer { repositoriesLogger.endInterval(token) }
247
-
// ... mutate state directly ...
275
+
// mutate state
248
276
```
249
277
250
-
The token wraps `OSSignpostIntervalState` so callers don't need to import `OSLog` themselves.
251
-
252
-
### Signpost inside SwiftUI `body`
253
-
254
-
`var body: some View` is implicitly `@ViewBuilder` and rejects `defer`. To count body invocations, an `Event` signpost via a discarded `let _ =` works:
278
+
SwiftUI bodies can use event signposts:
255
279
256
280
```swift
257
281
var body: some View {
258
282
let _ = shelfLogger.event("ShelfView.body")
259
-
// ... view tree ...
283
+
// view tree
260
284
}
261
285
```
262
286
263
-
Body intervals (Begin/End) are not directly representable this way; use the SwiftUI instrument's "View Body" track instead when you need duration.
287
+
### Hangs vs Hitches
264
288
265
-
### `OSSignposter.emitEvent` doesn't always show up in `xctrace export`
289
+
- A **Hang** is a main-thread block longer than the threshold.
290
+
- A **Hitch** is one or more missed frames.
266
291
267
-
In our traces, `Begin` and `End` rows from `interval(_:)` consistently appeared, but `emitEvent` (point signposts) sometimes didn't show in the exported `os-signpost` table even though they were visible in the Instruments UI. Worth knowing if a signpost count looks low — verify in the UI before assuming the call site didn't fire.
292
+
They measure different failure modes. For this investigation, hitches normalized by `reducer.selectWorktree` count were the most useful metric.
268
293
269
-
## Conclusions
294
+
## Future Work
270
295
271
-
1. **The single biggest win was finding the observability storm in `RepositorySectionView`**. Static analysis spotted the suspicious read; `swiftui-causes` confirmed it; the leaf-view fix dropped it ~92 % and eliminated the Severe Hang. This is what to look for first when SwiftUI feels uniformly slow under any state change.
296
+
The current result is probably good enough. If Shelf book switching becomes a product priority again, the remaining useful directions are more structural:
272
297
273
-
2. **`@Observable` types must be subscribed to with care.** Reading any property of an `@Observable` instance inside a parent `body` subscribes that body to *every change* on that property — and properties of type `Dictionary<…>` or `Set<…>` change on every insert / remove / mutate. For values that change frequently, push the read down into a leaf view that only renders that one value. The leaf still re-renders frequently, but the parent (and its other children) doesn't.
298
+
- **Custom spine layout instead of animated `HStack` frame interpolation.** Compute spine positions directly and animate transforms/offsets with stable child identity. This is more invasive but targets `AnimatableFrameAttribute` directly.
299
+
- **Reduce per-spine interactivity in a stable way.** Avoid conditional child types; consider stable lightweight wrappers if responder cost becomes a proven bottleneck.
300
+
- **Lazy or virtualized spines.** Useful only if real users commonly have many more visible/open books than the current test set.
301
+
- **Audit the inner terminal split-tree `.id(node.structuralIdentity)`.** This may be risky because `GhosttySurfaceScrollView` currently stores its surface view as immutable state; changing it needs a careful lifecycle audit.
302
+
- **Canvas-rendered spines.** Potentially large reduction in SwiftUI view-tree work, but high implementation and accessibility cost.
274
303
275
-
3. **Animation amplifies perceived jank far beyond its actual contribution to work.** The same ~250 ms of main-thread work feels broken when an animation is running through it and feels merely "slightly slow" when it isn't. Removing animation can be the right product call even when the trace numbers don't shift.
304
+
Do not re-try these unless a new trace suggests a different result:
276
305
277
-
4. **`.id(viewIdentity)` is not free.** Even when our own `makeNSView` body is fast, the SwiftUI bookkeeping around full subtree teardown (attribute graph, preferences, accessibility, transition orchestration) is real. But removing it may not yield measurable perf if there's another `.id` deeper in the tree forcing the same work — read the layout of `.id` modifiers across the whole subtree before assuming a removal will help.
278
-
279
-
5. **Signposts pay for themselves on the second investigation.** They cost nothing when no Instruments session is attached and let us collapse 30 minutes of guess-and-trace into a single targeted measurement. Worth keeping permanently around hot paths.
280
-
281
-
## Open Questions / Future Work
282
-
283
-
If we ever decide to attack the residual ~250 ms-per-click cost (probably only worthwhile if user feedback escalates):
284
-
285
-
- **Lazy-render spines**. We always render every spine even when many are off-screen. A `ScrollView` + `LazyHStack` could cap the work to the visible window.
286
-
- **Single-`ForEach` Shelf layout**. Drop the cross-`ForEach` `matchedGeometryEffect` by laying out spines in one list and overlaying / inline-expanding the open book area at the right index. SwiftUI's normal list-diff would handle spine flow without identity reconciliation across two lists.
287
-
- **`Canvas`-rendered spines**. Each spine is currently a `View` subtree with `Button`s, `ScrollView`, `.contextMenu`, `.help` etc. Cumulatively across ~10 spines this is a lot of view-tree work per click. A custom `Canvas`-based spine could cut SwiftUI's per-spine overhead by an order of magnitude — at the cost of having to reimplement hover, hit-testing, accessibility manually.
288
-
- **Eliminate the inner `.id(node.structuralIdentity)`**. We never tested this. It might let the surface wrappers be reused across compatible split-tree shapes, but `GhosttySurfaceScrollView.surfaceView` is `let` so we'd have to make it mutable and audit all attach/detach lifecycle paths first.
289
-
290
-
None of these are obviously worth doing today.
306
+
- removing only the outer open-book `.id`
307
+
- fully disabling Shelf animation
308
+
- moving terminal content to a final-position overlay
309
+
- conditionally removing closed-spine context menus
291
310
292
311
## Long-Term Observability Hooks
293
312
294
-
After this investigation the following signposts are permanently emitted to the `com.onevcat.prowl` subsystem under the `PointsOfInterest` category. They are visible in Instruments by adding the stock **Points of Interest** instrument to any trace:
313
+
Permanent signposts retained for future Shelf debugging:
295
314
296
-
- Reducer paths: `reducer.selectWorktree`, `reducer.selectRepository` (intervals)
297
-
- Terminal lifecycle: `Ghostty.makeNSView`, `Ghostty.updateNSView` (intervals); `Ghostty.dismantleNSView` (event)
298
-
- Shelf focus / sync: `OpenBook.onAppear`, `OpenBook.onChange.selectedTabId`, `focusSelectedTab`, `syncFocus`, `applySurfaceActivity` (intervals); `OpenBook.onDisappear` (event)
299
-
- View invalidation counters: `ShelfView.body`, `ShelfSpineView.body` (events)
300
-
- User-input markers: `BookClick.SwitchBook`, `BookClick.TabSwitchSameBook`, `BookClick.NewTabSpine` (events)
315
+
- Reducer paths: `reducer.selectWorktree`, `reducer.selectRepository`
316
+
- Terminal lifecycle: `Ghostty.makeNSView`, `Ghostty.updateNSView`, `Ghostty.dismantleNSView`
317
+
- Shelf focus/sync: `OpenBook.onAppear`, `OpenBook.onChange.selectedTabId`, `focusSelectedTab`, `syncFocus`, `applySurfaceActivity`, `OpenBook.onDisappear`
318
+
- View counters: `ShelfView.body`, `ShelfSpineView.body`
319
+
- User markers: `BookClick.SwitchBook`, `BookClick.TabSwitchSameBook`, `BookClick.NewTabSpine`
301
320
302
-
For future Shelf perf debugging the recommended starting workflow is:
321
+
Recommended future workflow:
303
322
304
-
1. Record with Animation Hitches template
305
-
2. Add the **Points of Interest** instrument to the document (one click)
306
-
3. Reproduce the workload
307
-
4. In the timeline, line up Hitches / Hangs against the Points of Interest lane to see which named work is on the critical path
323
+
1. Record with Animation Hitches template.
324
+
2. Add the Points of Interest instrument.
325
+
3. Reproduce the workload.
326
+
4. Export `hitches`, `os-signpost`, and `swiftui-causes`.
327
+
5. Normalize hitches by `reducer.selectWorktree` count and inspect SwiftUI source/destination pairs.