From 7351aa0c8fd850e42dd18d57c0052f74862e17cf Mon Sep 17 00:00:00 2001 From: andiahmads Date: Sun, 26 Jul 2026 10:27:02 +0700 Subject: [PATCH] perf(datagrid): bound cache work to visible columns --- CHANGELOG.md | 1 + TablePro/Core/DataGrid/RowDisplayBox.swift | 80 ++++++++++--- .../Views/Results/DataGridCoordinator.swift | 106 ++++++++++-------- .../Core/DataGrid/RowDisplayCacheTests.swift | 67 ++++++----- ...ableViewCoordinatorDisplayCacheTests.swift | 87 +++++++++++++- 5 files changed, 250 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7a7637ea..579356113 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Data grids with hundreds of columns no longer format every offscreen cell while warming the scroll cache. Cache work now follows the columns in the viewport, which keeps wide ClickHouse results responsive while scrolling. (#1219) - The license activation sheet now opens when you click **Activate License**. It was built and then failed to appear, and once that happened further clicks did nothing at all. - File > Import from Other App..., Open Project Folder..., Import Connections... and Export Connections... now work when no welcome window is open. They used to do nothing. - The tooltip on the welcome screen's **+** button shows the shortcut you actually have bound for New Connection instead of always claiming ⌘N. diff --git a/TablePro/Core/DataGrid/RowDisplayBox.swift b/TablePro/Core/DataGrid/RowDisplayBox.swift index cf9f0107c..b8f1986cf 100644 --- a/TablePro/Core/DataGrid/RowDisplayBox.swift +++ b/TablePro/Core/DataGrid/RowDisplayBox.swift @@ -6,16 +6,68 @@ import Foundation final class RowDisplayBox { - var values: ContiguousArray + private enum CachedValue { + case text(String) + case empty + } + + private var valuesByColumn: [Int: CachedValue] = [:] + private(set) var cost: Int = 0 + + init(_ values: ContiguousArray = []) { + valuesByColumn.reserveCapacity(values.count) + for (column, value) in values.enumerated() { + setValue(value, at: column) + } + } + + var cachedColumnCount: Int { + valuesByColumn.count + } + + func containsValue(at column: Int) -> Bool { + valuesByColumn[column] != nil + } - init(_ values: ContiguousArray) { - self.values = values + func value(at column: Int) -> String? { + guard let value = valuesByColumn[column] else { return nil } + switch value { + case .text(let text): + return text + case .empty: + return nil + } + } + + func setValue(_ value: String?, at column: Int) { + guard column >= 0 else { return } + if case .text(let previous)? = valuesByColumn[column] { + cost -= previous.utf8.count + } + if let value { + valuesByColumn[column] = .text(value) + cost += value.utf8.count + } else { + valuesByColumn[column] = .empty + } + } + + func invalidateValue(at column: Int) { + guard let previous = valuesByColumn.removeValue(forKey: column) else { return } + if case .text(let previous) = previous { + cost -= previous.utf8.count + } } } @MainActor final class RowDisplayCache { - private var storage: [RowID: RowDisplayBox] = [:] + private struct Entry { + let box: RowDisplayBox + let cost: Int + } + + private var storage: [RowID: Entry] = [:] private var insertionOrder: [RowID] = [] private var insertionHead: Int = 0 private var totalCost: Int = 0 @@ -28,17 +80,17 @@ final class RowDisplayCache { } func box(forID id: RowID) -> RowDisplayBox? { - storage[id] + storage[id]?.box } - func setBox(_ box: RowDisplayBox, forID id: RowID, cost: Int) { + func setBox(_ box: RowDisplayBox, forID id: RowID) { if let existing = storage[id] { - totalCost -= rowCost(existing.values) + totalCost -= existing.cost } else { insertionOrder.append(id) } - storage[id] = box - totalCost += cost + storage[id] = Entry(box: box, cost: box.cost) + totalCost += box.cost evictIfNeeded() } @@ -55,7 +107,7 @@ final class RowDisplayCache { let oldest = insertionOrder[insertionHead] insertionHead += 1 if let removed = storage.removeValue(forKey: oldest) { - totalCost -= rowCost(removed.values) + totalCost -= removed.cost } } if insertionHead > 10_000 { @@ -63,12 +115,4 @@ final class RowDisplayCache { insertionHead = 0 } } - - private func rowCost(_ values: ContiguousArray) -> Int { - var total = 0 - for value in values { - if let s = value { total &+= s.utf8.count } - } - return total - } } diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index f13aa5cbb..d4ee72964 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -376,31 +376,15 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData func displayValue(forID id: RowID, column: Int, rawValue: PluginCellValue, columnType: ColumnType?) -> String? { if let box = displayCache.box(forID: id), - column >= 0, column < box.values.count, - let cached = box.values[column] { - return cached + box.containsValue(at: column) { + return box.value(at: column) } let format = column >= 0 && column < columnDisplayFormats.count ? columnDisplayFormats[column] : nil let formatted = CellDisplayFormatter.format(rawValue, columnType: columnType, displayFormat: format) ?? rawValue.asText - let neededCount = max(column + 1, columnDisplayFormats.count, cachedColumnCount) - let box: RowDisplayBox - if let existing = displayCache.box(forID: id) { - box = existing - if box.values.count < neededCount { - box.values.reserveCapacity(neededCount) - for _ in box.values.count..() - values.reserveCapacity(neededCount) - for _ in 0..= 0, column < box.values.count { - box.values[column] = formatted - } - displayCache.setBox(box, forID: id, cost: displayCacheCost(box.values)) + let box = displayCache.box(forID: id) ?? RowDisplayBox() + box.setValue(formatted, at: column) + displayCache.setBox(box, forID: id) return formatted } @@ -425,12 +409,15 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData } func preWarmDisplayCache(upTo rowCount: Int) { + guard let tableView else { return } + let columnIndices = visibleDataColumnIndices(in: tableView) + guard !columnIndices.isEmpty else { return } let tableRows = tableRowsProvider() let displayCount = displayIDs?.count ?? tableRows.count let count = min(rowCount, displayCount) guard count > 0 else { return } for displayIndex in 0..= deadline { break } } @@ -509,41 +499,67 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData } } - private func cacheDisplayRow(at displayIndex: Int, in tableRows: TableRows) { - guard let row = displayRow(at: displayIndex, in: tableRows) else { return } - guard displayCache.box(forID: row.id) == nil else { return } - - let columnCount = tableRows.columns.count - var values = ContiguousArray() - values.reserveCapacity(columnCount) - for _ in 0.. Int { + guard let row = displayRow(at: displayIndex, in: tableRows) else { return 0 } + let box = displayCache.box(forID: row.id) ?? RowDisplayBox() + let columnCount = min(row.values.count, tableRows.columns.count) + var cachedCount = 0 + for col in columnIndices where col >= 0 && col < columnCount && !box.containsValue(at: col) { let columnType = col < tableRows.columnTypes.count ? tableRows.columnTypes[col] : nil let format = col < columnDisplayFormats.count ? columnDisplayFormats[col] : nil - values[col] = CellDisplayFormatter.format( + let formatted = CellDisplayFormatter.format( row.values[col], columnType: columnType, displayFormat: format ) ?? row.values[col].asText + box.setValue(formatted, at: col) + cachedCount += 1 } - let box = RowDisplayBox(values) - displayCache.setBox(box, forID: row.id, cost: displayCacheCost(values)) - } - - private func displayCacheCost(_ values: ContiguousArray) -> Int { - var total = 0 - for value in values { - if let s = value { total &+= s.utf8.count } + guard cachedCount > 0 else { return 0 } + displayCache.setBox(box, forID: row.id) + return cachedCount + } + + func visibleDataColumnIndices(in tableView: NSTableView) -> IndexSet { + let visibleRect = tableView.visibleRect + let tableMaxY = tableView.bounds.maxY + guard visibleRect.width > 0, tableMaxY > 0, + let firstContentColumn = tableView.tableColumns.firstIndex(where: { !$0.isHidden }), + let lastContentColumn = tableView.tableColumns.lastIndex(where: { !$0.isHidden }) else { + return IndexSet() + } + let contentMinX = tableView.rect(ofColumn: firstContentColumn).minX + let contentMaxX = tableView.rect(ofColumn: lastContentColumn).maxX + guard contentMaxX > contentMinX, + visibleRect.maxX > contentMinX, + visibleRect.minX < contentMaxX else { return IndexSet() } + let probeY = min(max(visibleRect.midY, tableView.bounds.minY), tableMaxY - 1) + let leadingX = min(max(visibleRect.minX, contentMinX), contentMaxX - 1) + let trailingX = min(max(visibleRect.maxX - 1, leadingX), contentMaxX - 1) + let firstVisibleColumn = tableView.column(at: NSPoint(x: leadingX, y: probeY)) + let lastVisibleColumn = tableView.column(at: NSPoint(x: trailingX, y: probeY)) + guard firstVisibleColumn >= 0, lastVisibleColumn >= firstVisibleColumn else { return IndexSet() } + var indices = IndexSet() + for tableColumnIndex in firstVisibleColumn...lastVisibleColumn { + let tableColumn = tableView.tableColumns[tableColumnIndex] + guard !tableColumn.isHidden, + let dataColumnIndex = dataColumnIndex(from: tableColumn.identifier) else { continue } + indices.insert(dataColumnIndex) } - return total + return indices } private func invalidateDisplayCache(forDisplayRow displayIndex: Int, column: Int) { guard let row = displayRow(at: displayIndex) else { return } guard let box = displayCache.box(forID: row.id), - column >= 0, column < box.values.count else { return } - box.values[column] = nil - displayCache.setBox(box, forID: row.id, cost: displayCacheCost(box.values)) + box.containsValue(at: column) else { return } + box.invalidateValue(at: column) + displayCache.setBox(box, forID: row.id) } func applyDelta(_ delta: Delta) { diff --git a/TableProTests/Core/DataGrid/RowDisplayCacheTests.swift b/TableProTests/Core/DataGrid/RowDisplayCacheTests.swift index d62ca3b9c..74b3c3b5c 100644 --- a/TableProTests/Core/DataGrid/RowDisplayCacheTests.swift +++ b/TableProTests/Core/DataGrid/RowDisplayCacheTests.swift @@ -14,14 +14,6 @@ struct RowDisplayCacheTests { RowDisplayBox(ContiguousArray(values)) } - private func cost(of values: [String?]) -> Int { - var total = 0 - for v in values { - if let s = v { total &+= s.utf8.count } - } - return total - } - @Test("Empty cache returns nil for any lookup") func emptyLookup() { let cache = RowDisplayCache() @@ -35,7 +27,7 @@ struct RowDisplayCacheTests { let id = RowID.existing(42) let values = ["a", "b", "c"] let box = makeBox(values) - cache.setBox(box, forID: id, cost: cost(of: values)) + cache.setBox(box, forID: id) #expect(cache.box(forID: id) === box) } @@ -44,12 +36,12 @@ struct RowDisplayCacheTests { func countLimitEvictsFIFO() { let cache = RowDisplayCache(countLimit: 3, costLimit: 1_000_000) for index in 1...3 { - cache.setBox(makeBox(["row\(index)"]), forID: .existing(index), cost: 4) + cache.setBox(makeBox(["row\(index)"]), forID: .existing(index)) } #expect(cache.box(forID: .existing(1)) != nil) // Fourth insertion should evict the first. - cache.setBox(makeBox(["row4"]), forID: .existing(4), cost: 4) + cache.setBox(makeBox(["row4"]), forID: .existing(4)) #expect(cache.box(forID: .existing(1)) == nil) #expect(cache.box(forID: .existing(2)) != nil) #expect(cache.box(forID: .existing(3)) != nil) @@ -60,9 +52,9 @@ struct RowDisplayCacheTests { func costLimitEvicts() { let cache = RowDisplayCache(countLimit: 1_000, costLimit: 10) // First insert costs 6; under cap. - cache.setBox(makeBox(["abcdef"]), forID: .existing(1), cost: 6) + cache.setBox(makeBox(["abcdef"]), forID: .existing(1)) // Second insert costs 6 more; total 12 > 10, evicts first. - cache.setBox(makeBox(["123456"]), forID: .existing(2), cost: 6) + cache.setBox(makeBox(["123456"]), forID: .existing(2)) #expect(cache.box(forID: .existing(1)) == nil) #expect(cache.box(forID: .existing(2)) != nil) @@ -71,17 +63,17 @@ struct RowDisplayCacheTests { @Test("Replacing an existing key does not consume queue slot") func replaceExistingKey() { let cache = RowDisplayCache(countLimit: 2, costLimit: 1_000_000) - cache.setBox(makeBox(["v1"]), forID: .existing(1), cost: 2) - cache.setBox(makeBox(["v2"]), forID: .existing(2), cost: 2) + cache.setBox(makeBox(["v1"]), forID: .existing(1)) + cache.setBox(makeBox(["v2"]), forID: .existing(2)) // Replace id=1 without expanding the cache. - cache.setBox(makeBox(["v1-updated"]), forID: .existing(1), cost: 10) - #expect(cache.box(forID: .existing(1))?.values.first == "v1-updated") - #expect(cache.box(forID: .existing(2))?.values.first == "v2") + cache.setBox(makeBox(["v1-updated"]), forID: .existing(1)) + #expect(cache.box(forID: .existing(1))?.value(at: 0) == "v1-updated") + #expect(cache.box(forID: .existing(2))?.value(at: 0) == "v2") // Adding a new entry now evicts the oldest in insertion order (still id=1 // because replacing did not re-add it to the order). - cache.setBox(makeBox(["v3"]), forID: .existing(3), cost: 2) + cache.setBox(makeBox(["v3"]), forID: .existing(3)) #expect(cache.box(forID: .existing(1)) == nil) #expect(cache.box(forID: .existing(2)) != nil) #expect(cache.box(forID: .existing(3)) != nil) @@ -91,7 +83,7 @@ struct RowDisplayCacheTests { func removeAllResetsState() { let cache = RowDisplayCache() for index in 1...10 { - cache.setBox(makeBox(["x"]), forID: .existing(index), cost: 1) + cache.setBox(makeBox(["x"]), forID: .existing(index)) } cache.removeAll() for index in 1...10 { @@ -99,8 +91,8 @@ struct RowDisplayCacheTests { } // Cache continues to work after removeAll. - cache.setBox(makeBox(["fresh"]), forID: .existing(100), cost: 5) - #expect(cache.box(forID: .existing(100))?.values.first == "fresh") + cache.setBox(makeBox(["fresh"]), forID: .existing(100)) + #expect(cache.box(forID: .existing(100))?.value(at: 0) == "fresh") } @Test("Inserted row IDs of both kinds round-trip") @@ -108,9 +100,32 @@ struct RowDisplayCacheTests { let cache = RowDisplayCache() let existingID = RowID.existing(5) let insertedID = RowID.inserted(UUID()) - cache.setBox(makeBox(["existing"]), forID: existingID, cost: 8) - cache.setBox(makeBox(["inserted"]), forID: insertedID, cost: 8) - #expect(cache.box(forID: existingID)?.values.first == "existing") - #expect(cache.box(forID: insertedID)?.values.first == "inserted") + cache.setBox(makeBox(["existing"]), forID: existingID) + cache.setBox(makeBox(["inserted"]), forID: insertedID) + #expect(cache.box(forID: existingID)?.value(at: 0) == "existing") + #expect(cache.box(forID: insertedID)?.value(at: 0) == "inserted") + } + + @Test("Mutating an existing box updates its recorded cost") + func mutatedBoxCostUpdate() { + let cache = RowDisplayCache(countLimit: 10, costLimit: 5) + let box = makeBox(["a"]) + cache.setBox(box, forID: .existing(1)) + + box.setValue("123456", at: 0) + cache.setBox(box, forID: .existing(1)) + + #expect(cache.box(forID: .existing(1)) == nil) + } + + @Test("A value at a wide column uses one sparse cache slot") + func sparseWideColumn() { + let box = RowDisplayBox() + box.setValue("x", at: 499) + + #expect(box.cachedColumnCount == 1) + #expect(box.containsValue(at: 499)) + #expect(box.value(at: 499) == "x") + #expect(box.cost == 1) } } diff --git a/TableProTests/Views/Results/TableViewCoordinatorDisplayCacheTests.swift b/TableProTests/Views/Results/TableViewCoordinatorDisplayCacheTests.swift index 9733f69a4..0fda82ede 100644 --- a/TableProTests/Views/Results/TableViewCoordinatorDisplayCacheTests.swift +++ b/TableProTests/Views/Results/TableViewCoordinatorDisplayCacheTests.swift @@ -10,10 +10,18 @@ import Testing @testable import TablePro +private final class VisibleRectTableView: NSTableView { + var stubVisibleRect: NSRect = .zero + + override var visibleRect: NSRect { + stubVisibleRect + } +} + @Suite("TableViewCoordinator display cache invalidation") @MainActor struct TableViewCoordinatorDisplayCacheTests { - private func makeCoordinator() -> TableViewCoordinator { + private func makeCoordinator(tableRows: TableRows? = nil) -> TableViewCoordinator { let coordinator = TableViewCoordinator( changeManager: AnyChangeManager(DataChangeManager()), isEditable: true, @@ -21,7 +29,7 @@ struct TableViewCoordinatorDisplayCacheTests { delegate: nil, layoutPersister: FakeDisplayCachePersister() ) - var captured = TableRows( + var captured = tableRows ?? TableRows( rows: [Row(id: .existing(0), values: [.text("A")])], columns: ["name"], columnTypes: [.text(rawType: nil)] @@ -51,6 +59,81 @@ struct TableViewCoordinatorDisplayCacheTests { let fresh = coordinator.displayValue(forID: .existing(0), column: column, rawValue: value("B"), columnType: type) #expect(fresh == "B") } + + @Test("Wide-row prewarm formats only requested columns") + func wideRowPrewarmIsColumnBounded() { + let columnCount = 500 + let columns = (0..