From 293e020e4a40b8d8699c8ca9d145eb50d4ed6354 Mon Sep 17 00:00:00 2001 From: xuhengyu Date: Fri, 19 Jun 2026 14:30:00 +0800 Subject: [PATCH 1/2] perf(datagrid): serve row count from cache to smooth scrolling --- CHANGELOG.md | 4 + .../Views/Results/DataGridCoordinator.swift | 2 +- ...bleViewCoordinatorRowCountCacheTests.swift | 128 ++++++++++++++++++ 3 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 TableProTests/Views/Results/TableViewCoordinatorRowCountCacheTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 0697c32b5..347a7ce52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Data grid now serves the row count from its existing cache instead of recomputing it on every layout pass, reducing CPU churn while scrolling large result sets. + ### Fixed - Oracle connections no longer crash the app when the server sends a backend message the driver cannot decode; the query fails with a clear error and the connection reconnects. (#483) diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index 75f9850e8..7b016ca4e 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -713,7 +713,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData // MARK: - NSTableViewDataSource func numberOfRows(in tableView: NSTableView) -> Int { - sortedIDs?.count ?? tableRowsProvider().count + sortedIDs?.count ?? cachedRowCount } } diff --git a/TableProTests/Views/Results/TableViewCoordinatorRowCountCacheTests.swift b/TableProTests/Views/Results/TableViewCoordinatorRowCountCacheTests.swift new file mode 100644 index 000000000..0438f69c4 --- /dev/null +++ b/TableProTests/Views/Results/TableViewCoordinatorRowCountCacheTests.swift @@ -0,0 +1,128 @@ +// +// TableViewCoordinatorRowCountCacheTests.swift +// TableProTests +// + +import SwiftUI +import TableProPluginKit +import Testing + +@testable import TablePro + +@Suite("TableViewCoordinator cachedRowCount sync") +@MainActor +struct TableViewCoordinatorRowCountCacheTests { + private func makeCoordinator(rows: ContiguousArray) -> TableViewCoordinator { + let coordinator = TableViewCoordinator( + changeManager: AnyChangeManager(DataChangeManager()), + isEditable: true, + selectedRowIndices: .constant([]), + delegate: nil, + layoutPersister: FakeRowCountPersister() + ) + var captured = TableRows(rows: rows, columns: ["c"]) + coordinator.tableRowsProvider = { captured } + coordinator.tableRowsMutator = { mutation in mutation(&captured) } + coordinator.updateCache() + return coordinator + } + + @Test("cachedRowCount tracks provider after initial load") + func cacheMatchesProviderOnLoad() { + let rows: ContiguousArray = [ + Row(id: .existing(0), values: [.text("a")]), + Row(id: .existing(1), values: [.text("b")]), + Row(id: .existing(2), values: [.text("c")]), + ] + let coordinator = makeCoordinator(rows: rows) + + #expect(coordinator.cachedRowCount == 3) + #expect(coordinator.cachedRowCount == coordinator.tableRowsProvider().count) + } + + @Test("updateCache picks up appended rows") + func updateCacheReflectsAppendedRows() { + let coordinator = makeCoordinator(rows: []) + + coordinator.tableRowsMutator { rows in + _ = rows.appendInsertedRow(values: [.text("a")]) + _ = rows.appendInsertedRow(values: [.text("b")]) + } + coordinator.updateCache() + + #expect(coordinator.cachedRowCount == 2) + #expect(coordinator.cachedRowCount == coordinator.tableRowsProvider().count) + } + + @Test("updateCache picks up removed rows") + func updateCacheReflectsRemovedRows() { + let rows: ContiguousArray = [ + Row(id: .existing(0), values: [.text("a")]), + Row(id: .existing(1), values: [.text("b")]), + Row(id: .existing(2), values: [.text("c")]), + ] + let coordinator = makeCoordinator(rows: rows) + + coordinator.tableRowsMutator { rows in + _ = rows.remove(rowIDs: [.existing(1)]) + } + coordinator.updateCache() + + #expect(coordinator.cachedRowCount == 2) + #expect(coordinator.cachedRowCount == coordinator.tableRowsProvider().count) + } + + @Test("updateCache reflects full replace") + func updateCacheReflectsFullReplace() { + let rows: ContiguousArray = [ + Row(id: .existing(0), values: [.text("a")]), + ] + let coordinator = makeCoordinator(rows: rows) + + coordinator.tableRowsMutator { rows in + _ = rows.replace(rows: [[.text("x")], [.text("y")], [.text("z")], [.text("w")]]) + } + coordinator.updateCache() + + #expect(coordinator.cachedRowCount == 4) + #expect(coordinator.cachedRowCount == coordinator.tableRowsProvider().count) + } + + @Test("sortedIDs count takes precedence over cachedRowCount fallback") + func sortedIDsCountPrecedesCache() { + let rows: ContiguousArray = [ + Row(id: .existing(0), values: [.text("a")]), + Row(id: .existing(1), values: [.text("b")]), + Row(id: .existing(2), values: [.text("c")]), + ] + let coordinator = makeCoordinator(rows: rows) + + coordinator.sortedIDs = [.existing(2), .existing(0)] + coordinator.updateCache() + + #expect(coordinator.cachedRowCount == 2) + } + + @Test("releaseData zeroes cachedRowCount") + func releaseDataZeroesCache() { + let rows: ContiguousArray = [ + Row(id: .existing(0), values: [.text("a")]), + Row(id: .existing(1), values: [.text("b")]), + ] + let coordinator = makeCoordinator(rows: rows) + #expect(coordinator.cachedRowCount == 2) + + coordinator.releaseData() + + #expect(coordinator.cachedRowCount == 0) + } +} + +@MainActor +private final class FakeRowCountPersister: ColumnLayoutPersisting { + func load(for tableName: String, connectionId: UUID) -> ColumnLayoutState? { nil } + + func save(_ layout: ColumnLayoutState, for tableName: String, connectionId: UUID) {} + + func clear(for tableName: String, connectionId: UUID) {} +} From 2b266cbf16081e58a83c2552fa1aeefd5cc47255 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 20 Jun 2026 11:50:35 +0700 Subject: [PATCH 2/2] refactor(datagrid): remove dead TableRowsController that bypassed row-count cache --- .../Views/Results/DataGridCoordinator.swift | 2 - TablePro/Views/Results/DataGridView.swift | 2 - .../Extensions/DataGridView+CellCommit.swift | 12 +- .../Views/Results/TableRowsController.swift | 54 ------ .../Results/TableRowsControllerTests.swift | 156 ------------------ ...bleViewCoordinatorRowCountCacheTests.swift | 29 ++++ 6 files changed, 33 insertions(+), 222 deletions(-) delete mode 100644 TablePro/Views/Results/TableRowsController.swift delete mode 100644 TableProTests/Views/Results/TableRowsControllerTests.swift diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index 7b016ca4e..e9381730a 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -109,7 +109,6 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData let cellFactory = DataGridCellFactory() let cellRegistry: DataGridCellRegistry let columnPool = DataGridColumnPool() - let tableRowsController = TableRowsController() let selectionController = GridSelectionController() var overlayEditor: CellOverlayEditor? var overlayViewer: CellOverlayViewer? @@ -234,7 +233,6 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData } tableView.reloadData() } - tableRowsController.detach() delegate = nil activeFKPreviewPopover?.close() clearFKPreviewState() diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index f268a06c8..22906b994 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -109,7 +109,6 @@ struct DataGridView: NSViewRepresentable { context.coordinator.tableView = tableView installSelectionOverlay(tableView: tableView, coordinator: context.coordinator) context.coordinator.attachScrollObservers(scrollView: scrollView) - context.coordinator.tableRowsController.attach(tableView) context.coordinator.tableRowsProvider = tableRowsProvider context.coordinator.tableRowsMutator = tableRowsMutator context.coordinator.paginationOffsetProvider = paginationOffsetProvider @@ -426,7 +425,6 @@ struct DataGridView: NSViewRepresentable { coordinator.persistColumnLayoutToStorage() coordinator.settingsCancellable = nil coordinator.themeCancellable = nil - coordinator.tableRowsController.detach() } func makeCoordinator() -> TableViewCoordinator { diff --git a/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift b/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift index 08a74372b..a24b5f5fd 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift @@ -26,14 +26,10 @@ extension TableViewCoordinator { in: tableView, schema: identitySchema ) else { return } - if case .cellChanged = delta { - tableRowsController.apply(.cellChanged(row: row, column: tableColumnIndex)) - } else { - tableView.reloadData( - forRowIndexes: IndexSet(integer: row), - columnIndexes: IndexSet(integer: tableColumnIndex) - ) - } + tableView.reloadData( + forRowIndexes: IndexSet(integer: row), + columnIndexes: IndexSet(integer: tableColumnIndex) + ) } @discardableResult diff --git a/TablePro/Views/Results/TableRowsController.swift b/TablePro/Views/Results/TableRowsController.swift deleted file mode 100644 index 60bc48d7a..000000000 --- a/TablePro/Views/Results/TableRowsController.swift +++ /dev/null @@ -1,54 +0,0 @@ -import AppKit -import Foundation - -@MainActor -final class TableRowsController { - weak var tableView: NSTableView? - - var insertAnimation: NSTableView.AnimationOptions = .slideDown - var removeAnimation: NSTableView.AnimationOptions = .slideUp - - init(tableView: NSTableView? = nil) { - self.tableView = tableView - } - - func attach(_ tableView: NSTableView) { - self.tableView = tableView - } - - func detach() { - tableView = nil - } - - func apply(_ delta: Delta) { - guard let tableView else { return } - switch delta { - case .cellChanged(let row, let column): - guard row >= 0, row < tableView.numberOfRows else { return } - guard column >= 0, column < tableView.numberOfColumns else { return } - tableView.reloadData(forRowIndexes: IndexSet(integer: row), columnIndexes: IndexSet(integer: column)) - case .cellsChanged(let positions): - guard !positions.isEmpty else { return } - var rowSet = IndexSet() - var colSet = IndexSet() - for position in positions { - if position.row >= 0, position.row < tableView.numberOfRows { - rowSet.insert(position.row) - } - if position.column >= 0, position.column < tableView.numberOfColumns { - colSet.insert(position.column) - } - } - guard !rowSet.isEmpty, !colSet.isEmpty else { return } - tableView.reloadData(forRowIndexes: rowSet, columnIndexes: colSet) - case .rowsInserted(let indices): - guard !indices.isEmpty else { return } - tableView.insertRows(at: indices, withAnimation: insertAnimation) - case .rowsRemoved(let indices): - guard !indices.isEmpty else { return } - tableView.removeRows(at: indices, withAnimation: removeAnimation) - case .columnsReplaced, .fullReplace: - tableView.reloadData() - } - } -} diff --git a/TableProTests/Views/Results/TableRowsControllerTests.swift b/TableProTests/Views/Results/TableRowsControllerTests.swift deleted file mode 100644 index 9c2479aa3..000000000 --- a/TableProTests/Views/Results/TableRowsControllerTests.swift +++ /dev/null @@ -1,156 +0,0 @@ -import AppKit -import Foundation -import TableProPluginKit -import Testing -@testable import TablePro - -@Suite("TableRowsController") -@MainActor -struct TableRowsControllerTests { - - final class RecordingTableView: NSTableView { - struct Reload { - let rows: IndexSet - let columns: IndexSet - } - - var insertCalls: [(IndexSet, NSTableView.AnimationOptions)] = [] - var removeCalls: [(IndexSet, NSTableView.AnimationOptions)] = [] - var rangeReloadCalls: [Reload] = [] - var fullReloadCount = 0 - var stubbedRowCount = 0 - - override var numberOfRows: Int { stubbedRowCount } - - override func insertRows(at indexes: IndexSet, withAnimation animationOptions: NSTableView.AnimationOptions = []) { - insertCalls.append((indexes, animationOptions)) - } - - override func removeRows(at indexes: IndexSet, withAnimation animationOptions: NSTableView.AnimationOptions = []) { - removeCalls.append((indexes, animationOptions)) - } - - override func reloadData(forRowIndexes rowIndexes: IndexSet, columnIndexes: IndexSet) { - rangeReloadCalls.append(Reload(rows: rowIndexes, columns: columnIndexes)) - } - - override func reloadData() { - fullReloadCount += 1 - } - } - - private func makeTableView(rows: Int, columns: Int) -> RecordingTableView { - let view = RecordingTableView(frame: .zero) - for index in 0.. = [ - CellPosition(row: 0, column: 0), - CellPosition(row: 0, column: 2), - CellPosition(row: 3, column: 1) - ] - controller.apply(.cellsChanged(positions)) - #expect(table.rangeReloadCalls.count == 1) - #expect(table.rangeReloadCalls.first?.rows == IndexSet([0, 3])) - #expect(table.rangeReloadCalls.first?.columns == IndexSet([0, 1, 2])) - } - - @Test("apply(.cellsChanged) with empty set is a no-op") - func cellsChangedEmptyNoOp() { - let table = makeTableView(rows: 5, columns: 3) - let controller = TableRowsController(tableView: table) - controller.apply(.cellsChanged([])) - #expect(table.rangeReloadCalls.isEmpty) - } - - @Test("apply(.rowsInserted) calls insertRows with the configured animation") - func rowsInsertedCallsInsert() { - let table = makeTableView(rows: 5, columns: 3) - let controller = TableRowsController(tableView: table) - controller.apply(.rowsInserted(IndexSet([5, 6]))) - #expect(table.insertCalls.count == 1) - #expect(table.insertCalls.first?.0 == IndexSet([5, 6])) - #expect(table.insertCalls.first?.1 == .slideDown) - } - - @Test("apply(.rowsInserted) with empty set is a no-op") - func rowsInsertedEmptyNoOp() { - let table = makeTableView(rows: 5, columns: 3) - let controller = TableRowsController(tableView: table) - controller.apply(.rowsInserted(IndexSet())) - #expect(table.insertCalls.isEmpty) - } - - @Test("apply(.rowsRemoved) calls removeRows") - func rowsRemovedCallsRemove() { - let table = makeTableView(rows: 5, columns: 3) - let controller = TableRowsController(tableView: table) - controller.apply(.rowsRemoved(IndexSet([1, 2]))) - #expect(table.removeCalls.count == 1) - #expect(table.removeCalls.first?.0 == IndexSet([1, 2])) - #expect(table.removeCalls.first?.1 == .slideUp) - } - - @Test("apply(.fullReplace) calls reloadData") - func fullReplaceReloadsAll() { - let table = makeTableView(rows: 5, columns: 3) - let controller = TableRowsController(tableView: table) - controller.apply(.fullReplace) - #expect(table.fullReloadCount == 1) - } - - @Test("apply(.columnsReplaced) calls reloadData") - func columnsReplacedReloadsAll() { - let table = makeTableView(rows: 5, columns: 3) - let controller = TableRowsController(tableView: table) - controller.apply(.columnsReplaced) - #expect(table.fullReloadCount == 1) - } - - @Test("apply with detached tableView is a no-op") - func detachedNoOp() { - let controller = TableRowsController() - controller.apply(.fullReplace) - } - - @Test("animation options are configurable") - func animationsConfigurable() { - let table = makeTableView(rows: 5, columns: 3) - let controller = TableRowsController(tableView: table) - controller.insertAnimation = .effectFade - controller.removeAnimation = .effectGap - - controller.apply(.rowsInserted(IndexSet(integer: 3))) - controller.apply(.rowsRemoved(IndexSet(integer: 1))) - - #expect(table.insertCalls.first?.1 == .effectFade) - #expect(table.removeCalls.first?.1 == .effectGap) - } -} diff --git a/TableProTests/Views/Results/TableViewCoordinatorRowCountCacheTests.swift b/TableProTests/Views/Results/TableViewCoordinatorRowCountCacheTests.swift index 0438f69c4..c34fad040 100644 --- a/TableProTests/Views/Results/TableViewCoordinatorRowCountCacheTests.swift +++ b/TableProTests/Views/Results/TableViewCoordinatorRowCountCacheTests.swift @@ -3,6 +3,7 @@ // TableProTests // +import AppKit import SwiftUI import TableProPluginKit import Testing @@ -88,6 +89,34 @@ struct TableViewCoordinatorRowCountCacheTests { #expect(coordinator.cachedRowCount == coordinator.tableRowsProvider().count) } + @Test("numberOfRows serves cachedRowCount when unsorted") + func numberOfRowsServesCacheWhenUnsorted() { + let rows: ContiguousArray = [ + Row(id: .existing(0), values: [.text("a")]), + Row(id: .existing(1), values: [.text("b")]), + ] + let coordinator = makeCoordinator(rows: rows) + let tableView = NSTableView() + + #expect(coordinator.numberOfRows(in: tableView) == 2) + #expect(coordinator.numberOfRows(in: tableView) == coordinator.tableRowsProvider().count) + } + + @Test("numberOfRows serves sortedIDs count when sorted") + func numberOfRowsServesSortedIDsCountWhenSorted() { + let rows: ContiguousArray = [ + Row(id: .existing(0), values: [.text("a")]), + Row(id: .existing(1), values: [.text("b")]), + Row(id: .existing(2), values: [.text("c")]), + ] + let coordinator = makeCoordinator(rows: rows) + let tableView = NSTableView() + coordinator.sortedIDs = [.existing(2), .existing(0)] + coordinator.updateCache() + + #expect(coordinator.numberOfRows(in: tableView) == 2) + } + @Test("sortedIDs count takes precedence over cachedRowCount fallback") func sortedIDsCountPrecedesCache() { let rows: ContiguousArray = [