Skip to content

Commit 0fe682c

Browse files
onevcatonevcat
committed
perf(shelf): cut sidebar tab-count subscription storm and add signposts
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).
1 parent 47768c6 commit 0fe682c

8 files changed

Lines changed: 198 additions & 56 deletions

File tree

‎supacode/Features/Repositories/Reducer/RepositoriesFeature.swift‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ nonisolated let worktreeCreationProgressLineLimit = 200
1010
nonisolated let worktreeCreationProgressUpdateStride = 20
1111
nonisolated let archiveScriptProgressLineLimit = 200
1212
private let secondsPerDay: Double = 86_400
13+
private let repositoriesLogger = SupaLogger("RepositoriesFeature")
1314

1415
nonisolated struct WorktreeCreationProgressUpdateThrottle {
1516
private let stride: Int
@@ -792,6 +793,10 @@ struct RepositoriesFeature {
792793
return .none
793794

794795
case .selectRepository(let repositoryID):
796+
// `inout state` cannot be captured by a closure, so use the
797+
// begin/end token API rather than the `interval` helper.
798+
let selectRepoToken = repositoriesLogger.beginInterval("reducer.selectRepository")
799+
defer { repositoriesLogger.endInterval(selectRepoToken) }
795800
guard let repositoryID, state.repositories[id: repositoryID] != nil else { return .none }
796801
state.selection = .repository(repositoryID)
797802
state.sidebarSelectedWorktreeIDs = []
@@ -802,6 +807,8 @@ struct RepositoriesFeature {
802807
return .send(.delegate(.selectedWorktreeChanged(state.selectedTerminalWorktree)))
803808

804809
case .selectWorktree(let worktreeID, let focusTerminal):
810+
let selectWtToken = repositoriesLogger.beginInterval("reducer.selectWorktree")
811+
defer { repositoriesLogger.endInterval(selectWtToken) }
805812
setSingleWorktreeSelection(worktreeID, state: &state)
806813
if focusTerminal, let worktreeID {
807814
state.pendingTerminalFocusWorktreeIDs.insert(worktreeID)

‎supacode/Features/Repositories/Views/RepoHeaderRow.swift‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ struct RepoHeaderRow: View {
44
private static let debugHeaderLayers = false
55
let name: String
66
let isRemoving: Bool
7-
let tabCount: Int
87
/// User-pinned icon, when set. Renders before the repo name.
98
/// `nil` keeps the historical text-only layout intact.
109
let icon: RepositoryIconSource?
@@ -34,16 +33,6 @@ struct RepoHeaderRow: View {
3433
.font(.caption)
3534
.foregroundStyle(.tertiary)
3635
}
37-
if tabCount > 0 {
38-
Text("\(tabCount)")
39-
.font(.caption2)
40-
.monospacedDigit()
41-
.foregroundStyle(.secondary)
42-
.padding(.horizontal, 5)
43-
.padding(.vertical, 1)
44-
.background(.quaternary, in: .capsule)
45-
.help("\(tabCount) active \(tabCount == 1 ? "tab" : "tabs")")
46-
}
4736
}
4837
.background {
4938
if Self.debugHeaderLayers {
@@ -58,30 +47,58 @@ struct RepoHeaderRow: View {
5847
}
5948
}
6049

50+
/// Leaf view that renders the open-tab count badge for a repository.
51+
///
52+
/// Lives in its own `View` so the read of `terminalManager` (an
53+
/// `@Observable` whose `states` dictionary churns whenever terminal
54+
/// activity happens) is isolated to this subtree. Without this split,
55+
/// `RepositorySectionView.body` would subscribe to every change in
56+
/// `terminalManager.states` on every re-evaluation — which under heavy
57+
/// terminal activity caused tens of thousands of body invocations per
58+
/// second across the sidebar.
59+
struct RepoHeaderTabCountBadge: View {
60+
let repository: Repository
61+
let terminalManager: WorktreeTerminalManager
62+
63+
var body: some View {
64+
let count = RepositorySectionView.openTabCount(
65+
for: repository,
66+
terminalManager: terminalManager
67+
)
68+
if count > 0 {
69+
Text("\(count)")
70+
.font(.caption2)
71+
.monospacedDigit()
72+
.foregroundStyle(.secondary)
73+
.padding(.horizontal, 5)
74+
.padding(.vertical, 1)
75+
.background(.quaternary, in: .capsule)
76+
.help("\(count) active \(count == 1 ? "tab" : "tabs")")
77+
}
78+
}
79+
}
80+
6181
// MARK: - Previews
6282

6383
#Preview("RepoHeaderRow") {
6484
VStack(alignment: .leading, spacing: 12) {
6585
RepoHeaderRow(
6686
name: "supacode",
6787
isRemoving: false,
68-
tabCount: 3,
6988
icon: nil,
7089
iconTint: nil,
7190
repositoryRootURL: nil
7291
)
7392
RepoHeaderRow(
7493
name: "ghostty",
7594
isRemoving: false,
76-
tabCount: 0,
7795
icon: .sfSymbol("folder.fill"),
7896
iconTint: .blue,
7997
repositoryRootURL: URL(fileURLWithPath: "/tmp/ghostty")
8098
)
8199
RepoHeaderRow(
82100
name: "removing-repo",
83101
isRemoving: true,
84-
tabCount: 1,
85102
icon: .sfSymbol("hammer.fill"),
86103
iconTint: .orange,
87104
repositoryRootURL: URL(fileURLWithPath: "/tmp/removing")

‎supacode/Features/Repositories/Views/RepositorySectionView.swift‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,27 @@ struct RepositorySectionView: View {
3838

3939
let appearance = repositoryAppearances[repository.id] ?? .empty
4040
let header = HStack {
41-
RepoHeaderRow(
42-
name: repository.name,
43-
isRemoving: isRemovingRepository,
44-
tabCount: Self.openTabCount(
45-
for: repository,
41+
// Inner HStack groups the name row and the tab-count badge so they
42+
// share the leading-aligned region of the outer header. Crucially
43+
// the badge is its own leaf view (`RepoHeaderTabCountBadge`) — it
44+
// owns the `terminalManager` read so this view never subscribes
45+
// to the manager-wide states dictionary.
46+
HStack {
47+
RepoHeaderRow(
48+
name: repository.name,
49+
isRemoving: isRemovingRepository,
50+
icon: appearance.icon,
51+
iconTint: appearance.color?.color,
52+
repositoryRootURL: repository.rootURL,
53+
nameTooltip: repository.capabilities.supportsWorktrees
54+
? (isExpanded ? "Collapse" : "Expand")
55+
: "Open terminal in folder"
56+
)
57+
RepoHeaderTabCountBadge(
58+
repository: repository,
4659
terminalManager: terminalManager
47-
),
48-
icon: appearance.icon,
49-
iconTint: appearance.color?.color,
50-
repositoryRootURL: repository.rootURL,
51-
nameTooltip: repository.capabilities.supportsWorktrees
52-
? (isExpanded ? "Collapse" : "Expand")
53-
: "Open terminal in folder"
54-
)
60+
)
61+
}
5562
.frame(maxWidth: .infinity, alignment: .leading)
5663
.background {
5764
if Self.debugHeaderLayers {

‎supacode/Features/Shelf/Models/ShelfBook.swift‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import Foundation
2+
import IdentifiedCollections
23

34
/// A book on the Shelf — the unified abstraction over a Git worktree or
45
/// a plain folder repository.
@@ -38,10 +39,18 @@ extension RepositoriesFeature.State {
3839
/// while in Shelf mode adds its ID here, which causes its spine to
3940
/// materialize (with the standard spine-flow animation).
4041
func orderedShelfBooks() -> [ShelfBook] {
41-
let repositoriesByID = Dictionary(uniqueKeysWithValues: repositories.map { ($0.id, $0) })
42+
// `ShelfView.body` re-runs on every TCA state change, so this method
43+
// is on the per-frame hot path. The previous implementation built a
44+
// `Dictionary(uniqueKeysWithValues:)` per call and routed worktree
45+
// ordering through `worktreeRowSections(in:)` — which constructs a
46+
// full `WorktreeRowModel` per worktree (PR/info lookups, icon
47+
// resolution, etc.) plus several intermediate `Set` allocations per
48+
// repository. None of that detail is needed by the Shelf, which only
49+
// consumes id/name/branch. Use direct `IdentifiedArray` lookup and
50+
// `orderedWorktrees(in:)` for the lighter ordering path.
4251
var books: [ShelfBook] = []
4352
for repositoryID in orderedRepositoryIDs() {
44-
guard let repository = repositoriesByID[repositoryID] else { continue }
53+
guard let repository = repositories[id: repositoryID] else { continue }
4554
if repository.kind == .plain {
4655
guard openedWorktreeIDs.contains(repository.id) else { continue }
4756
books.append(
@@ -55,15 +64,34 @@ extension RepositoriesFeature.State {
5564
))
5665
continue
5766
}
58-
for row in worktreeRows(in: repository) {
59-
guard openedWorktreeIDs.contains(row.id) else { continue }
67+
for worktree in orderedWorktrees(in: repository)
68+
where openedWorktreeIDs.contains(worktree.id) {
6069
books.append(
6170
ShelfBook(
62-
id: row.id,
71+
id: worktree.id,
6372
repositoryID: repositoryID,
64-
displayName: row.name,
73+
displayName: worktree.name,
6574
projectName: repository.name,
66-
branchName: row.name,
75+
branchName: worktree.name,
76+
kind: .worktree
77+
))
78+
}
79+
// Preserve prior behavior of `worktreeRowSections` which also
80+
// surfaced any pending (in-creation) worktrees that had been
81+
// marked opened. The list is typically empty so the cost is
82+
// negligible — the win is avoiding `makePendingWorktreeRow` which
83+
// builds a full `WorktreeRowModel` per entry.
84+
for pending in pendingWorktrees
85+
where pending.repositoryID == repositoryID
86+
&& openedWorktreeIDs.contains(pending.id)
87+
{
88+
books.append(
89+
ShelfBook(
90+
id: pending.id,
91+
repositoryID: repositoryID,
92+
displayName: pending.progress.titleText,
93+
projectName: repository.name,
94+
branchName: pending.progress.titleText,
6795
kind: .worktree
6896
))
6997
}

‎supacode/Features/Shelf/Views/ShelfOpenBookView.swift‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import AppKit
22
import SwiftUI
33

4+
private let shelfLogger = SupaLogger("Shelf")
5+
46
/// Renders the terminal content for the currently open book.
57
///
68
/// Mirrors the terminal-content slice of `WorktreeTerminalTabsView` without
@@ -57,19 +59,23 @@ struct ShelfOpenBookView: View {
5759
}
5860
)
5961
.onAppear {
60-
state.ensureInitialTab(focusing: false)
61-
if shouldAutoFocusTerminal {
62-
state.focusSelectedTab()
62+
shelfLogger.interval("OpenBook.onAppear") {
63+
state.ensureInitialTab(focusing: false)
64+
if shouldAutoFocusTerminal {
65+
state.focusSelectedTab()
66+
}
67+
let activity = resolvedWindowActivity
68+
state.syncFocus(windowIsKey: activity.isKeyWindow, windowIsVisible: activity.isVisible)
6369
}
64-
let activity = resolvedWindowActivity
65-
state.syncFocus(windowIsKey: activity.isKeyWindow, windowIsVisible: activity.isVisible)
6670
}
6771
.onChange(of: state.tabManager.selectedTabId) { _, _ in
68-
if shouldAutoFocusTerminal {
69-
state.focusSelectedTab()
72+
shelfLogger.interval("OpenBook.onChange.selectedTabId") {
73+
if shouldAutoFocusTerminal {
74+
state.focusSelectedTab()
75+
}
76+
let activity = resolvedWindowActivity
77+
state.syncFocus(windowIsKey: activity.isKeyWindow, windowIsVisible: activity.isVisible)
7078
}
71-
let activity = resolvedWindowActivity
72-
state.syncFocus(windowIsKey: activity.isKeyWindow, windowIsVisible: activity.isVisible)
7379
}
7480
}
7581

‎supacode/Features/Shelf/Views/ShelfView.swift‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import ComposableArchitecture
22
import SwiftUI
33

4+
private let shelfLogger = SupaLogger("Shelf")
5+
46
/// Root view for Shelf presentation mode.
57
///
68
/// Phase 3 layout: three horizontal segments — a left stack of passed
@@ -27,6 +29,10 @@ struct ShelfView: View {
2729
@Environment(\.surfaceBackgroundOpacity) private var surfaceBackgroundOpacity
2830

2931
var body: some View {
32+
// Note: `body` is a `@ViewBuilder` getter so we can't add a
33+
// `defer`-based signpost interval directly here. Body-level cost is
34+
// already visible via the SwiftUI instrument's "View Body" track —
35+
// signposts in this file focus on user-input moments instead.
3036
let state = store.state
3137
let books = state.orderedShelfBooks()
3238
let openBookID = state.openShelfBookID
@@ -101,13 +107,21 @@ struct ShelfView: View {
101107

102108
/// Dispatch the open-book action only when `book` isn't already the open
103109
/// one — idempotent helper for taps that imply a book change.
110+
///
111+
/// No `animation:` is passed to `store.send` because the visible
112+
/// spine-flow animation is already driven by the view-level
113+
/// `.animation(.easeInOut(duration: 0.2), value: openBookID)` modifier
114+
/// on the root container — wrapping the dispatch in another animation
115+
/// transaction would double-run layout / transition machinery for the
116+
/// same change.
104117
private func switchToBookIfNeeded(_ book: ShelfBook) {
105118
guard !isOpen(book) else { return }
119+
shelfLogger.event("BookClick.NewTabSpine")
106120
switch book.kind {
107121
case .worktree:
108-
store.send(.selectWorktree(book.id, focusTerminal: true), animation: .easeInOut(duration: 0.2))
122+
store.send(.selectWorktree(book.id, focusTerminal: true))
109123
case .plainFolder:
110-
store.send(.selectRepository(book.repositoryID), animation: .easeInOut(duration: 0.2))
124+
store.send(.selectRepository(book.repositoryID))
111125
}
112126
}
113127

@@ -190,17 +204,20 @@ struct ShelfView: View {
190204
private func openBook(_ book: ShelfBook, selectingTab tabID: TerminalTabID?) {
191205
let isAlreadyOpen = store.state.openShelfBookID == book.id
192206
if let tabID, isAlreadyOpen, let state = terminalManager.stateIfExists(for: book.id) {
207+
shelfLogger.event("BookClick.TabSwitchSameBook")
193208
state.tabManager.selectTab(tabID)
194209
return
195210
}
196-
// Animate the spine flow and terminal crossfade. The duration and
197-
// curve mirror the Shelf design doc: ~200ms ease-in-out, snappy but
198-
// legible so the user can read each spine's movement.
211+
shelfLogger.event("BookClick.SwitchBook")
212+
// The spine flow / terminal crossfade animation is already driven
213+
// by the view-level `.animation(_:value: openBookID)` on the root
214+
// container (~200ms ease-in-out per the Shelf design doc), so the
215+
// dispatch itself does not pass an `animation:` argument here.
199216
switch book.kind {
200217
case .worktree:
201-
store.send(.selectWorktree(book.id, focusTerminal: true), animation: .easeInOut(duration: 0.2))
218+
store.send(.selectWorktree(book.id, focusTerminal: true))
202219
case .plainFolder:
203-
store.send(.selectRepository(book.repositoryID), animation: .easeInOut(duration: 0.2))
220+
store.send(.selectRepository(book.repositoryID))
204221
}
205222
if let tabID {
206223
// Apply tab selection eagerly; the target book's state already exists

‎supacode/Features/Terminal/Models/WorktreeTerminalState.swift‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -315,8 +315,10 @@ final class WorktreeTerminalState {
315315
}
316316

317317
func focusSelectedTab() {
318-
guard let tabId = tabManager.selectedTabId else { return }
319-
focusSurface(in: tabId)
318+
terminalStateLogger.interval("focusSelectedTab") {
319+
guard let tabId = tabManager.selectedTabId else { return }
320+
focusSurface(in: tabId)
321+
}
320322
}
321323

322324
@discardableResult
@@ -345,12 +347,20 @@ final class WorktreeTerminalState {
345347
}
346348

347349
func syncFocus(windowIsKey: Bool, windowIsVisible: Bool) {
348-
lastWindowIsKey = windowIsKey
349-
lastWindowIsVisible = windowIsVisible
350-
applySurfaceActivity()
350+
terminalStateLogger.interval("syncFocus") {
351+
lastWindowIsKey = windowIsKey
352+
lastWindowIsVisible = windowIsVisible
353+
applySurfaceActivity()
354+
}
351355
}
352356

353357
private func applySurfaceActivity() {
358+
terminalStateLogger.interval("applySurfaceActivity") {
359+
applySurfaceActivityImpl()
360+
}
361+
}
362+
363+
private func applySurfaceActivityImpl() {
354364
let selectedTabId = tabManager.selectedTabId
355365
var surfaceToFocus: GhosttySurfaceView?
356366
for (tabId, tree) in trees {

0 commit comments

Comments
 (0)