diff --git a/CHANGELOG.md b/CHANGELOG.md index d6b89d8e8..81038f6b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,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) - Quitting no longer loses unsaved SQL editor tabs. A window with no tabs loaded yet, such as one still waiting on its connection, could erase the saved tabs for that connection, so nothing came back on relaunch. Editors are also saved about a second after you stop typing instead of waiting up to 30 seconds, and a query over 500KB is kept in full rather than restored empty. (#1997) - Opening a large MongoDB collection no longer hangs. Row counts that back the pagination display now give up after 5 seconds and leave the estimate in place instead of holding the tab. - Stop now cancels a running MongoDB query on the server, rather than leaving it running until it finishes on its own. diff --git a/TablePro/Core/DataGrid/RowDisplayBox.swift b/TablePro/Core/DataGrid/RowDisplayBox.swift index 511b397d3..542262e21 100644 --- a/TablePro/Core/DataGrid/RowDisplayBox.swift +++ b/TablePro/Core/DataGrid/RowDisplayBox.swift @@ -6,16 +6,73 @@ 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 + } + + 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 + } + } - init(_ values: ContiguousArray) { - self.values = values + func removeAllValues() { + valuesByColumn.removeAll(keepingCapacity: true) + cost = 0 } } @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 +85,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() } @@ -54,11 +111,10 @@ final class RowDisplayCache { /// whose content changed in place keeps its id and would otherwise be served /// its pre-edit text. func clearValues(forID id: RowID) { - guard let box = storage[id] else { return } - totalCost -= rowCost(box.values) - for index in box.values.indices { - box.values[index] = nil - } + guard let entry = storage[id] else { return } + totalCost -= entry.cost + entry.box.removeAllValues() + storage[id] = Entry(box: entry.box, cost: entry.box.cost) } private func evictIfNeeded() { @@ -67,7 +123,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 { @@ -75,12 +131,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 1c3b68d92..d657c1a50 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,33 +499,59 @@ 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) { @@ -546,9 +562,9 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData 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 5400eb365..dcaaf711f 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("Clearing a row drops its formatted values and keeps the box") @@ -108,39 +100,52 @@ struct RowDisplayCacheTests { let cache = RowDisplayCache() let id = RowID.existing(1) let box = makeBox(["old", "VARCHAR(255)", "YES"]) - cache.setBox(box, forID: id, cost: cost(of: ["old", "VARCHAR(255)", "YES"])) + cache.setBox(box, forID: id) cache.clearValues(forID: id) let cleared = try #require(cache.box(forID: id)) #expect(cleared === box) - #expect(cleared.values.count == 3) - #expect(cleared.values.allSatisfy { $0 == nil }) + #expect(cleared.cachedColumnCount == 0) + #expect(!cleared.containsValue(at: 0)) } @Test("Clearing one row leaves the others formatted") func clearValuesLeavesOtherRows() throws { let cache = RowDisplayCache() - cache.setBox(makeBox(["a"]), forID: .existing(0), cost: 1) - cache.setBox(makeBox(["b"]), forID: .existing(1), cost: 1) + cache.setBox(makeBox(["a"]), forID: .existing(0)) + cache.setBox(makeBox(["b"]), forID: .existing(1)) cache.clearValues(forID: .existing(1)) let kept = try #require(cache.box(forID: .existing(0))) - #expect(kept.values[0] == "a") + #expect(kept.value(at: 0) == "a") let cleared = try #require(cache.box(forID: .existing(1))) - #expect(cleared.values[0] == nil) + #expect(!cleared.containsValue(at: 0)) + } + + @Test("Clearing a row releases its recorded cost") + func clearValuesReleasesCost() { + let cache = RowDisplayCache(countLimit: 10, costLimit: 6) + let clearedID = RowID.existing(0) + cache.setBox(makeBox(["12345"]), forID: clearedID) + + cache.clearValues(forID: clearedID) + cache.setBox(makeBox(["67890"]), forID: .existing(1)) + + #expect(cache.box(forID: clearedID) != nil) + #expect(cache.box(forID: .existing(1)) != nil) } @Test("Clearing an uncached row does nothing") func clearValuesForUnknownRow() { let cache = RowDisplayCache() - cache.setBox(makeBox(["a"]), forID: .existing(0), cost: 1) + cache.setBox(makeBox(["a"]), forID: .existing(0)) cache.clearValues(forID: .existing(9)) #expect(cache.box(forID: .existing(9)) == nil) - #expect(cache.box(forID: .existing(0))?.values.first == "a") + #expect(cache.box(forID: .existing(0))?.value(at: 0) == "a") } @Test("A cleared row accepts fresh values and keeps serving them") @@ -148,13 +153,13 @@ struct RowDisplayCacheTests { let cache = RowDisplayCache() let id = RowID.existing(2) let box = makeBox(["old"]) - cache.setBox(box, forID: id, cost: 3) + cache.setBox(box, forID: id) cache.clearValues(forID: id) - box.values[0] = "new" - cache.setBox(box, forID: id, cost: 3) + box.setValue("new", at: 0) + cache.setBox(box, forID: id) - #expect(cache.box(forID: id)?.values.first == "new") + #expect(cache.box(forID: id)?.value(at: 0) == "new") } @Test("Inserted row IDs of both kinds round-trip") @@ -162,9 +167,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..