perf: stop recomputing derived state on every SwiftUI render
An audit for v0.1.0 turned up the same pattern in four places: a computed property doing real work (filtering/sorting/scoring a collection), read multiple times per render including from unrelated state changes (selection, hover, scroll), so the work reran far more often than the underlying data actually changed. Converted each to a @State cache recomputed only via onChange of its real inputs: - DocumentSearchSheet: matchingLineIndices re-scanned the whole document per access, read once per visible row plus twice more in the header/step logic - O(n^2) case-insensitive scan per frame on a large document. Also split into an ordered array (for currentMatchIndex/stepping) plus a parallel Set for the per-row highlight check, which was an O(k) linear .contains before. - CollectionDocumentsOutline: tree rebuilt the whole dictionary- grouped, recursively-sorted document tree on every body evaluation, not just when documents/sortOption actually changed. - CommandPaletteView: results re-scored and re-sorted the entire index (up to the whole local workspace cache in Full Workspace mode) on every render, including ones from selectedIndex moving as arrow keys are pressed. - CollectionOverviewView: sortedDocuments re-sorted on every render; same pattern, smaller blast radius (capped at 100 docs). Also: - HomeViewModel.fetchPinnedThrowing fetched each pinned document serially in a for loop (one round trip at a time) - switched to a TaskGroup so latency doesn't scale with pin count, results reordered back to pins.list's own order since task completion order isn't submission order. - AvatarCropperView.renderFinalImage ran ImageRenderer + JPEG compression synchronously on the main actor from the "Use Photo" button tap. ImageRenderer itself has to stay on the main actor (it captures live SwiftUI state), but JPEG compression on the already- rendered bitmap has no SwiftUI dependency left - hopped that part to a detached Task via tiffRepresentation (plain Data, unlike NSImage itself isn't Sendable) so it doesn't hitch the UI. No crash risks or retain cycles found in the same audit (no try!/ as!, force-unwraps essentially absent outside a hardcoded URL literal, weak self already used where it matters) - this is purely the perf half of the findings. Not compiler-verified - Outpost app target has no CLI build path.
This commit is contained in:
@@ -74,8 +74,10 @@ struct AvatarCropperView: View {
|
|||||||
Button("Cancel", role: .cancel, action: onCancel)
|
Button("Cancel", role: .cancel, action: onCancel)
|
||||||
Spacer()
|
Spacer()
|
||||||
Button("Use Photo") {
|
Button("Use Photo") {
|
||||||
if let data = renderFinalImage() {
|
Task {
|
||||||
onConfirm(data)
|
if let data = await renderFinalImage() {
|
||||||
|
onConfirm(data)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
.buttonStyle(.borderedProminent)
|
.buttonStyle(.borderedProminent)
|
||||||
@@ -100,15 +102,26 @@ struct AvatarCropperView: View {
|
|||||||
.clipped()
|
.clipped()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `ImageRenderer` itself has to run on the main actor (it captures live
|
||||||
|
/// SwiftUI view state), but JPEG compression on the bitmap it produces
|
||||||
|
/// is pure CPU work with no SwiftUI dependency left — hopping off for
|
||||||
|
/// just that part avoids a visible hitch on tapping "Use Photo".
|
||||||
|
/// `tiffRepresentation` (plain `Data`, unlike `NSImage` itself) is what
|
||||||
|
/// actually crosses the actor boundary; mirrors `NSImage.jpegData(
|
||||||
|
/// compressionQuality:)`'s own logic rather than calling it directly, so
|
||||||
|
/// crossing doesn't require handing a non-Sendable `NSImage` to a
|
||||||
|
/// detached task.
|
||||||
@MainActor
|
@MainActor
|
||||||
private func renderFinalImage() -> Data? {
|
private func renderFinalImage() async -> Data? {
|
||||||
let content = avatarContent
|
let content = avatarContent
|
||||||
.clipShape(Circle())
|
.clipShape(Circle())
|
||||||
.frame(width: diameter, height: diameter)
|
.frame(width: diameter, height: diameter)
|
||||||
let renderer = ImageRenderer(content: content)
|
let renderer = ImageRenderer(content: content)
|
||||||
renderer.scale = 2 // @2x so it isn't a blurry 320px avatar on Retina displays
|
renderer.scale = 2 // @2x so it isn't a blurry 320px avatar on Retina displays
|
||||||
guard let nsImage = renderer.nsImage else { return nil }
|
guard let tiffData = renderer.nsImage?.tiffRepresentation else { return nil }
|
||||||
return nsImage.jpegData(compressionQuality: 0.9)
|
return await Task.detached(priority: .userInitiated) {
|
||||||
|
NSBitmapImageRep(data: tiffData)?.representation(using: .jpeg, properties: [.compressionFactor: 0.9])
|
||||||
|
}.value
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|||||||
@@ -30,8 +30,15 @@ struct CollectionDocumentsOutline: View {
|
|||||||
/// every document in the tree.
|
/// every document in the tree.
|
||||||
@State private var pinsByDocumentID: [String: OutlinePin] = [:]
|
@State private var pinsByDocumentID: [String: OutlinePin] = [:]
|
||||||
|
|
||||||
private var tree: [DocumentNode] {
|
/// Recomputed only when `viewModel.documents`/`sortOption` actually change
|
||||||
buildDocumentTree(from: viewModel.documents, sortedBy: sortOption)
|
/// (below) instead of being a computed property — this rebuilt the whole
|
||||||
|
/// dictionary-grouped, recursively-sorted tree on every `body` evaluation,
|
||||||
|
/// including renders triggered by unrelated state (selection, hover,
|
||||||
|
/// pins) that don't change the tree's shape at all.
|
||||||
|
@State private var tree: [DocumentNode] = []
|
||||||
|
|
||||||
|
private func rebuildTree() {
|
||||||
|
tree = buildDocumentTree(from: viewModel.documents, sortedBy: sortOption)
|
||||||
}
|
}
|
||||||
|
|
||||||
init(
|
init(
|
||||||
@@ -86,7 +93,10 @@ struct CollectionDocumentsOutline: View {
|
|||||||
.task(id: "\(refreshToken)-\(externalRefreshToken)") {
|
.task(id: "\(refreshToken)-\(externalRefreshToken)") {
|
||||||
await viewModel.load()
|
await viewModel.load()
|
||||||
await loadPins()
|
await loadPins()
|
||||||
|
rebuildTree()
|
||||||
}
|
}
|
||||||
|
.onChange(of: viewModel.documents) { rebuildTree() }
|
||||||
|
.onChange(of: sortOption) { rebuildTree() }
|
||||||
}
|
}
|
||||||
|
|
||||||
private func loadPins() async {
|
private func loadPins() async {
|
||||||
|
|||||||
@@ -34,8 +34,15 @@ struct CollectionOverviewView: View {
|
|||||||
self.onOpenDocument = onOpenDocument
|
self.onOpenDocument = onOpenDocument
|
||||||
}
|
}
|
||||||
|
|
||||||
private var sortedDocuments: [OutlineDocument] {
|
/// Recomputed only when `viewModel.documents`/`selectedTab` actually
|
||||||
selectedTab.sorted(viewModel.documents)
|
/// change (below) instead of being a computed property re-sorted on
|
||||||
|
/// every render — capped at 100 documents per collection page, so lower
|
||||||
|
/// blast radius than the sidebar/command-palette versions of this same
|
||||||
|
/// pattern, but the same fix.
|
||||||
|
@State private var sortedDocuments: [OutlineDocument] = []
|
||||||
|
|
||||||
|
private func resortDocuments() {
|
||||||
|
sortedDocuments = selectedTab.sorted(viewModel.documents)
|
||||||
}
|
}
|
||||||
|
|
||||||
var body: some View {
|
var body: some View {
|
||||||
@@ -94,6 +101,8 @@ struct CollectionOverviewView: View {
|
|||||||
await viewModel.checkForRemoteChanges()
|
await viewModel.checkForRemoteChanges()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
.onChange(of: viewModel.documents) { resortDocuments() }
|
||||||
|
.onChange(of: selectedTab) { resortDocuments() }
|
||||||
}
|
}
|
||||||
|
|
||||||
// Spans the full window width, centered, directly under the toolbar —
|
// Spans the full window width, centered, directly under the toolbar —
|
||||||
|
|||||||
@@ -41,22 +41,31 @@ struct CommandPaletteView: View {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private var results: [Result] {
|
/// Recomputed only when `query`/`collections`/`documents` actually change
|
||||||
|
/// (below) instead of being a computed property — Full Workspace mode's
|
||||||
|
/// index can be large (every document in the local cache, sub-documents
|
||||||
|
/// included), and this was re-scanning + re-sorting the entire thing on
|
||||||
|
/// every render, including ones triggered by unrelated state like
|
||||||
|
/// `selectedIndex` changing as arrow keys move the selection.
|
||||||
|
@State private var results: [Result] = []
|
||||||
|
|
||||||
|
private func recomputeResults() {
|
||||||
let trimmed = query.trimmingCharacters(in: .whitespacesAndNewlines)
|
let trimmed = query.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||||
guard !trimmed.isEmpty else {
|
guard !trimmed.isEmpty else {
|
||||||
// No query yet: surface collections first, then the most
|
// No query yet: surface collections first, then the most
|
||||||
// recent/full-workspace documents as-is, capped so the panel
|
// recent/full-workspace documents as-is, capped so the panel
|
||||||
// doesn't dump the entire workspace with nothing typed.
|
// doesn't dump the entire workspace with nothing typed.
|
||||||
return (collections.map(Result.collection) + documents.map(Result.document))
|
results = (collections.map(Result.collection) + documents.map(Result.document))
|
||||||
.prefix(20)
|
.prefix(20)
|
||||||
.map { $0 }
|
.map { $0 }
|
||||||
|
return
|
||||||
}
|
}
|
||||||
let scored: [(Result, Int)] = collections.compactMap { collection in
|
let scored: [(Result, Int)] = collections.compactMap { collection in
|
||||||
matchScore(collection.name, query: trimmed).map { (Result.collection(collection), $0) }
|
matchScore(collection.name, query: trimmed).map { (Result.collection(collection), $0) }
|
||||||
} + documents.compactMap { document in
|
} + documents.compactMap { document in
|
||||||
matchScore(document.title, query: trimmed).map { (Result.document(document), $0) }
|
matchScore(document.title, query: trimmed).map { (Result.document(document), $0) }
|
||||||
}
|
}
|
||||||
return scored.sorted { $0.1 < $1.1 }.prefix(30).map(\.0)
|
results = scored.sorted { $0.1 < $1.1 }.prefix(30).map(\.0)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Lower is better — exact match, then prefix match, then earliest
|
/// Lower is better — exact match, then prefix match, then earliest
|
||||||
@@ -100,7 +109,10 @@ struct CommandPaletteView: View {
|
|||||||
.textFieldStyle(.plain)
|
.textFieldStyle(.plain)
|
||||||
.font(.title3)
|
.font(.title3)
|
||||||
.focused($isSearchFieldFocused)
|
.focused($isSearchFieldFocused)
|
||||||
.onChange(of: query) { selectedIndex = 0 }
|
.onChange(of: query) {
|
||||||
|
selectedIndex = 0
|
||||||
|
recomputeResults()
|
||||||
|
}
|
||||||
.onSubmit { selectCurrent() }
|
.onSubmit { selectCurrent() }
|
||||||
// Attached directly on the field itself, not an
|
// Attached directly on the field itself, not an
|
||||||
// ancestor — confirmed live that .onKeyPress on the
|
// ancestor — confirmed live that .onKeyPress on the
|
||||||
@@ -169,6 +181,8 @@ struct CommandPaletteView: View {
|
|||||||
isSearchFieldFocused = true
|
isSearchFieldFocused = true
|
||||||
await loadResults()
|
await loadResults()
|
||||||
}
|
}
|
||||||
|
.onChange(of: collections) { recomputeResults() }
|
||||||
|
.onChange(of: documents) { recomputeResults() }
|
||||||
}
|
}
|
||||||
|
|
||||||
private func resultRow(_ result: Result, isSelected: Bool) -> some View {
|
private func resultRow(_ result: Result, isSelected: Bool) -> some View {
|
||||||
|
|||||||
@@ -22,9 +22,26 @@ struct DocumentSearchSheet: View {
|
|||||||
@State private var currentMatchIndex = 0
|
@State private var currentMatchIndex = 0
|
||||||
@FocusState private var isSearchFieldFocused: Bool
|
@FocusState private var isSearchFieldFocused: Bool
|
||||||
|
|
||||||
private var matchingLineIndices: [Int] {
|
/// Recomputed only when `query`/`lines` actually change (`recomputeMatches()`)
|
||||||
guard !query.isEmpty else { return [] }
|
/// instead of being a computed property — this used to re-scan the whole
|
||||||
return lines.indices.filter { lines[$0].localizedCaseInsensitiveContains(query) }
|
/// document on every access, and it's read multiple times per row
|
||||||
|
/// (`isCurrentMatch`, the highlight check) on every SwiftUI re-render, so a
|
||||||
|
/// large document turned into an O(n²) case-insensitive scan per frame.
|
||||||
|
@State private var matchingLineIndices: [Int] = []
|
||||||
|
/// O(1) membership for the per-row highlight check below — `matchingLineIndices`
|
||||||
|
/// stays an ordered array (needed for `currentMatchIndex`/stepping), this is
|
||||||
|
/// just a parallel lookup so a common search term with many matches doesn't
|
||||||
|
/// make every row's highlight check an O(k) linear scan.
|
||||||
|
@State private var matchingLineIndexSet: Set<Int> = []
|
||||||
|
|
||||||
|
private func recomputeMatches() {
|
||||||
|
guard !query.isEmpty else {
|
||||||
|
matchingLineIndices = []
|
||||||
|
matchingLineIndexSet = []
|
||||||
|
return
|
||||||
|
}
|
||||||
|
matchingLineIndices = lines.indices.filter { lines[$0].localizedCaseInsensitiveContains(query) }
|
||||||
|
matchingLineIndexSet = Set(matchingLineIndices)
|
||||||
}
|
}
|
||||||
|
|
||||||
var body: some View {
|
var body: some View {
|
||||||
@@ -35,7 +52,10 @@ struct DocumentSearchSheet: View {
|
|||||||
TextField("Search in \"\(document.title.isEmpty ? "Untitled" : document.title)\"", text: $query)
|
TextField("Search in \"\(document.title.isEmpty ? "Untitled" : document.title)\"", text: $query)
|
||||||
.textFieldStyle(.plain)
|
.textFieldStyle(.plain)
|
||||||
.focused($isSearchFieldFocused)
|
.focused($isSearchFieldFocused)
|
||||||
.onChange(of: query) { currentMatchIndex = 0 }
|
.onChange(of: query) {
|
||||||
|
currentMatchIndex = 0
|
||||||
|
recomputeMatches()
|
||||||
|
}
|
||||||
|
|
||||||
if !matchingLineIndices.isEmpty {
|
if !matchingLineIndices.isEmpty {
|
||||||
Text("\(currentMatchIndex + 1) of \(matchingLineIndices.count)")
|
Text("\(currentMatchIndex + 1) of \(matchingLineIndices.count)")
|
||||||
@@ -83,7 +103,7 @@ struct DocumentSearchSheet: View {
|
|||||||
.padding(.horizontal, 4)
|
.padding(.horizontal, 4)
|
||||||
.background(
|
.background(
|
||||||
isCurrentMatch(index) ? Color.yellow.opacity(0.4)
|
isCurrentMatch(index) ? Color.yellow.opacity(0.4)
|
||||||
: matchingLineIndices.contains(index) ? Color.yellow.opacity(0.15)
|
: matchingLineIndexSet.contains(index) ? Color.yellow.opacity(0.15)
|
||||||
: Color.clear
|
: Color.clear
|
||||||
)
|
)
|
||||||
.id(index)
|
.id(index)
|
||||||
@@ -131,6 +151,7 @@ struct DocumentSearchSheet: View {
|
|||||||
do {
|
do {
|
||||||
let text = try await apiClient.documentInfo(id: document.id).text
|
let text = try await apiClient.documentInfo(id: document.id).text
|
||||||
lines = text.components(separatedBy: "\n")
|
lines = text.components(separatedBy: "\n")
|
||||||
|
recomputeMatches()
|
||||||
} catch {
|
} catch {
|
||||||
errorMessage = outlineErrorMessage(error, fallback: "Couldn't load this document.")
|
errorMessage = outlineErrorMessage(error, fallback: "Couldn't load this document.")
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -83,16 +83,25 @@ final class HomeViewModel {
|
|||||||
/// `pins.list` only returns pin records, not the documents themselves —
|
/// `pins.list` only returns pin records, not the documents themselves —
|
||||||
/// fetches each pinned document individually. Pins are a small curated
|
/// fetches each pinned document individually. Pins are a small curated
|
||||||
/// set (unlike a full collection tree), so the N+1 here is acceptable
|
/// set (unlike a full collection tree), so the N+1 here is acceptable
|
||||||
/// where it wouldn't be in the sidebar.
|
/// where it wouldn't be in the sidebar — but they're fetched concurrently
|
||||||
|
/// (a `TaskGroup`, not a serial loop) so latency doesn't scale with pin
|
||||||
|
/// count; `documentInfo` already goes through `CachingOutlineAPIClient`'s
|
||||||
|
/// own cached-read/retry path either way.
|
||||||
private func fetchPinnedThrowing() async throws -> [OutlineDocument] {
|
private func fetchPinnedThrowing() async throws -> [OutlineDocument] {
|
||||||
let pins = try await RetryPolicy.withRetry { try await apiClient.listPins(ListPinsRequest(collectionId: nil)) }
|
let pins = try await RetryPolicy.withRetry { try await apiClient.listPins(ListPinsRequest(collectionId: nil)) }
|
||||||
var documents: [OutlineDocument] = []
|
let client = apiClient
|
||||||
for pin in pins {
|
let documentsByID: [String: OutlineDocument] = await withTaskGroup(of: (String, OutlineDocument?).self) { group in
|
||||||
if let document = try? await apiClient.documentInfo(id: pin.documentId) {
|
for pin in pins {
|
||||||
documents.append(document)
|
group.addTask { (pin.documentId, try? await client.documentInfo(id: pin.documentId)) }
|
||||||
}
|
}
|
||||||
|
var result: [String: OutlineDocument] = [:]
|
||||||
|
for await (id, document) in group {
|
||||||
|
if let document { result[id] = document }
|
||||||
|
}
|
||||||
|
return result
|
||||||
}
|
}
|
||||||
return documents
|
// Preserve pins.list's own order rather than task-completion order.
|
||||||
|
return pins.compactMap { documentsByID[$0.documentId] }
|
||||||
}
|
}
|
||||||
|
|
||||||
private func fetch(tab: HomeTab) async throws -> [OutlineDocument] {
|
private func fetch(tab: HomeTab) async throws -> [OutlineDocument] {
|
||||||
|
|||||||
Reference in New Issue
Block a user