From 774ad929a015db19ee9b915aac7b50c879fa8e12 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 9 Mar 2026 17:25:26 +0700 Subject: [PATCH 1/5] perf: reduce memory usage by ~80-130 MB per connection - Eliminate dedicated ping driver, use main driver for health checks (~30-50 MB/connection) - Lazy-load plugin bundles on first use instead of at startup (~20-30 MB saved) - Evict inactive native window-tab row data after 5s, re-fetch on focus - Clear InMemoryRowProvider cache when RowBuffer is evicted - Remove duplicate sourceQuery string from RowBuffer - Add MEMORY-AUDIT.md with findings and fix tracking --- CHANGELOG.md | 4 + TablePro.xcodeproj/project.pbxproj | 12 +- TablePro/Core/Database/DatabaseDriver.swift | 3 + TablePro/Core/Database/DatabaseManager.swift | 56 +---- TablePro/Core/Plugins/PluginManager.swift | 60 +++++- TablePro/Models/Query/QueryTab.swift | 3 - TablePro/Views/Export/ExportDialog.swift | 1 + .../Main/Child/MainEditorContentView.swift | 4 + .../MainContentCoordinator+TabSwitch.swift | 1 - .../Views/Main/MainContentCoordinator.swift | 12 ++ TablePro/Views/Main/MainContentView.swift | 12 ++ .../Plugins/InstalledPluginsView.swift | 3 + .../Core/Plugins/PluginLazyLoadingTests.swift | 47 ++++ TableProTests/Models/RowBufferTests.swift | 111 ++++++++++ TableProTests/Views/Main/EvictionTests.swift | 112 ++++++++++ docs/development/MEMORY-AUDIT.md | 204 ++++++++++++++++++ 16 files changed, 575 insertions(+), 70 deletions(-) create mode 100644 TableProTests/Core/Plugins/PluginLazyLoadingTests.swift create mode 100644 TableProTests/Models/RowBufferTests.swift create mode 100644 TableProTests/Views/Main/EvictionTests.swift create mode 100644 docs/development/MEMORY-AUDIT.md diff --git a/CHANGELOG.md b/CHANGELOG.md index be060bb9..e5a418d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Reduce memory: eliminate dedicated ping driver (~30-50 MB per connection), use main driver for health checks +- Reduce memory: evict inactive native window-tab row data after 5s, re-fetch on focus +- Reduce memory: lazy-load plugin bundles on first use instead of at startup (~20-30 MB saved) +- Reduce memory: remove duplicate sourceQuery string from RowBuffer - Split DatabaseManager.sessionVersion into fine-grained connectionListVersion and connectionStatusVersion to reduce cascade re-renders - Extract AppState property reads into local lets in view bodies for explicit granular observation tracking - Reorganized project directory structure: Services, Utilities, Models split into domain-specific subdirectories diff --git a/TablePro.xcodeproj/project.pbxproj b/TablePro.xcodeproj/project.pbxproj index 62045dc2..e8182bf4 100644 --- a/TablePro.xcodeproj/project.pbxproj +++ b/TablePro.xcodeproj/project.pbxproj @@ -24,12 +24,6 @@ 5A867000D00000000 /* RedisDriver.tableplugin in Copy Plug-Ins */ = {isa = PBXBuildFile; fileRef = 5A867000100000000 /* RedisDriver.tableplugin */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; 5A868000A00000000 /* TableProPluginKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5A860000100000000 /* TableProPluginKit.framework */; }; 5A868000D00000000 /* PostgreSQLDriver.tableplugin in Copy Plug-Ins */ = {isa = PBXBuildFile; fileRef = 5A868000100000000 /* PostgreSQLDriver.tableplugin */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; - 5ACE00012F4F000000000004 /* CodeEditSourceEditor in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F000000000002 /* CodeEditSourceEditor */; }; - 5ACE00012F4F000000000005 /* CodeEditLanguages in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F000000000003 /* CodeEditLanguages */; }; - 5ACE00012F4F000000000006 /* CodeEditTextView in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F000000000007 /* CodeEditTextView */; }; - 5ACE00012F4F00000000000A /* Sparkle in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F000000000009 /* Sparkle */; }; - 5ACE00012F4F00000000000D /* MarkdownUI in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F00000000000C /* MarkdownUI */; }; - 5AEE5B362F5C9B7B00FA84D7 /* OracleNIO in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F00000000000F /* OracleNIO */; }; 5A86A000A00000000 /* TableProPluginKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5A860000100000000 /* TableProPluginKit.framework */; }; 5A86A000D00000000 /* CSVExport.tableplugin in Copy Plug-Ins */ = {isa = PBXBuildFile; fileRef = 5A86A000100000000 /* CSVExport.tableplugin */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; 5A86B000A00000000 /* TableProPluginKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5A860000100000000 /* TableProPluginKit.framework */; }; @@ -40,6 +34,12 @@ 5A86D000D00000000 /* XLSXExport.tableplugin in Copy Plug-Ins */ = {isa = PBXBuildFile; fileRef = 5A86D000100000000 /* XLSXExport.tableplugin */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; 5A86E000A00000000 /* TableProPluginKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5A860000100000000 /* TableProPluginKit.framework */; }; 5A86E000D00000000 /* MQLExport.tableplugin in Copy Plug-Ins */ = {isa = PBXBuildFile; fileRef = 5A86E000100000000 /* MQLExport.tableplugin */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; + 5ACE00012F4F000000000004 /* CodeEditSourceEditor in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F000000000002 /* CodeEditSourceEditor */; }; + 5ACE00012F4F000000000005 /* CodeEditLanguages in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F000000000003 /* CodeEditLanguages */; }; + 5ACE00012F4F000000000006 /* CodeEditTextView in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F000000000007 /* CodeEditTextView */; }; + 5ACE00012F4F00000000000A /* Sparkle in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F000000000009 /* Sparkle */; }; + 5ACE00012F4F00000000000D /* MarkdownUI in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F00000000000C /* MarkdownUI */; }; + 5AEE5B362F5C9B7B00FA84D7 /* OracleNIO in Frameworks */ = {isa = PBXBuildFile; productRef = 5ACE00012F4F00000000000F /* OracleNIO */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ diff --git a/TablePro/Core/Database/DatabaseDriver.swift b/TablePro/Core/Database/DatabaseDriver.swift index c38f2187..c06d52c3 100644 --- a/TablePro/Core/Database/DatabaseDriver.swift +++ b/TablePro/Core/Database/DatabaseDriver.swift @@ -297,6 +297,9 @@ extension DatabaseDriver { enum DatabaseDriverFactory { static func createDriver(for connection: DatabaseConnection) throws -> DatabaseDriver { let pluginId = connection.type.pluginTypeId + if PluginManager.shared.driverPlugins[pluginId] == nil { + PluginManager.shared.loadPendingPlugins() + } guard let plugin = PluginManager.shared.driverPlugins[pluginId] else { throw DatabaseError.connectionFailed( "\(pluginId) driver plugin not loaded. The plugin may be disabled or missing from the PlugIns directory." diff --git a/TablePro/Core/Database/DatabaseManager.swift b/TablePro/Core/Database/DatabaseManager.swift index 61ec395c..4b440ca3 100644 --- a/TablePro/Core/Database/DatabaseManager.swift +++ b/TablePro/Core/Database/DatabaseManager.swift @@ -40,10 +40,6 @@ final class DatabaseManager { /// Health monitors for active connections (MySQL/PostgreSQL only) private var healthMonitors: [UUID: ConnectionHealthMonitor] = [:] - /// Dedicated lightweight drivers used exclusively for health-check pings. - /// Separate from the main driver so pings never queue behind long-running user queries. - private var pingDrivers: [UUID: DatabaseDriver] = [:] - private var metadataCreationTasks: [UUID: Task] = [:] /// Current session (computed from currentSessionId) @@ -475,43 +471,15 @@ final class DatabaseManager { // Stop any existing monitor await stopHealthMonitor(for: connectionId) - // Create a dedicated lightweight driver for pings so they never - // queue behind long-running user queries on the main driver. - if let session = activeSessions[connectionId] { - let connectionForPing = session.effectiveConnection ?? session.connection - let dedicatedPingDriver: DatabaseDriver - do { - dedicatedPingDriver = try DatabaseDriverFactory.createDriver(for: connectionForPing) - } catch { - Self.logger.warning("Failed to create ping driver for \(connectionId): \(error.localizedDescription)") - return - } - do { - try await dedicatedPingDriver.connect() - pingDrivers[connectionId] = dedicatedPingDriver - } catch { - Self.logger.warning( - "Failed to create dedicated ping driver, will fall back to main driver") - } - } - let monitor = ConnectionHealthMonitor( connectionId: connectionId, pingHandler: { [weak self] in guard let self else { return false } - // Prefer the dedicated ping driver so pings are never blocked - // by long-running user queries on the main driver. - let pingDriver = await self.pingDrivers[connectionId] - let driver: DatabaseDriver - if let pingDriver { - driver = pingDriver - } else if let mainDriver = await self.activeSessions[connectionId]?.driver { - driver = mainDriver - } else { + guard let mainDriver = await self.activeSessions[connectionId]?.driver else { return false } do { - _ = try await driver.execute(query: "SELECT 1") + _ = try await mainDriver.execute(query: "SELECT 1") return true } catch { return false @@ -527,15 +495,6 @@ final class DatabaseManager { session.status = .connected } - // Also reconnect the dedicated ping driver so future pings - // don't fail immediately after a successful main reconnect. - let connectionForPing = session.effectiveConnection ?? session.connection - let newPingDriver = try await MainActor.run { - try DatabaseDriverFactory.createDriver(for: connectionForPing) - } - try await newPingDriver.connect() - await self.replacePingDriver(newPingDriver, for: connectionId) - return true } catch { return false @@ -610,22 +569,11 @@ final class DatabaseManager { return driver } - /// Replace the dedicated ping driver for a connection, disconnecting the old one. - private func replacePingDriver(_ newDriver: DatabaseDriver, for connectionId: UUID) { - pingDrivers[connectionId]?.disconnect() - pingDrivers[connectionId] = newDriver - } - /// Stop health monitoring for a connection private func stopHealthMonitor(for connectionId: UUID) async { if let monitor = healthMonitors.removeValue(forKey: connectionId) { await monitor.stopMonitoring() } - - // Disconnect and remove the dedicated ping driver - if let pingDriver = pingDrivers.removeValue(forKey: connectionId) { - pingDriver.disconnect() - } } /// Reconnect the current session (called from toolbar Reconnect button) diff --git a/TablePro/Core/Plugins/PluginManager.swift b/TablePro/Core/Plugins/PluginManager.swift index b54c66a3..34677106 100644 --- a/TablePro/Core/Plugins/PluginManager.swift +++ b/TablePro/Core/Plugins/PluginManager.swift @@ -37,6 +37,8 @@ final class PluginManager { private static let logger = Logger(subsystem: "com.TablePro", category: "PluginManager") + private var pendingPluginURLs: [(url: URL, source: PluginSource)] = [] + private init() {} // MARK: - Loading @@ -52,17 +54,34 @@ final class PluginManager { } if let builtInDir = builtInPluginsDir { - loadPlugins(from: builtInDir, source: .builtIn) + discoverPlugins(from: builtInDir, source: .builtIn) } - loadPlugins(from: userPluginsDir, source: .userInstalled) + discoverPlugins(from: userPluginsDir, source: .userInstalled) - validateDependencies() + Self.logger.info("Discovered \(self.pendingPluginURLs.count) plugin(s), will load on first use") + } + + /// Load all discovered but not-yet-loaded plugin bundles. + /// Called on first driver request or when the plugins settings screen opens. + func loadPendingPlugins() { + guard !pendingPluginURLs.isEmpty else { return } + let pending = pendingPluginURLs + pendingPluginURLs.removeAll() + + for entry in pending { + do { + try loadPlugin(at: entry.url, source: entry.source) + } catch { + Self.logger.error("Failed to load plugin at \(entry.url.lastPathComponent): \(error.localizedDescription)") + } + } + validateDependencies() Self.logger.info("Loaded \(self.plugins.count) plugin(s): \(self.driverPlugins.count) driver(s), \(self.exportPlugins.count) export format(s)") } - private func loadPlugins(from directory: URL, source: PluginSource) { + private func discoverPlugins(from directory: URL, source: PluginSource) { let fm = FileManager.default guard let contents = try? fm.contentsOfDirectory( at: directory, @@ -74,13 +93,42 @@ final class PluginManager { for itemURL in contents where itemURL.pathExtension == "tableplugin" { do { - _ = try loadPlugin(at: itemURL, source: source) + try discoverPlugin(at: itemURL, source: source) } catch { - Self.logger.error("Failed to load plugin at \(itemURL.lastPathComponent): \(error.localizedDescription)") + Self.logger.error("Failed to discover plugin at \(itemURL.lastPathComponent): \(error.localizedDescription)") } } } + private func discoverPlugin(at url: URL, source: PluginSource) throws { + guard let bundle = Bundle(url: url) else { + throw PluginError.invalidBundle("Cannot create bundle from \(url.lastPathComponent)") + } + + let infoPlist = bundle.infoDictionary ?? [:] + + let pluginKitVersion = infoPlist["TableProPluginKitVersion"] as? Int ?? 0 + if pluginKitVersion > Self.currentPluginKitVersion { + throw PluginError.incompatibleVersion( + required: pluginKitVersion, + current: Self.currentPluginKitVersion + ) + } + + if let minAppVersion = infoPlist["TableProMinAppVersion"] as? String { + let appVersion = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? "0.0.0" + if appVersion.compare(minAppVersion, options: .numeric) == .orderedAscending { + throw PluginError.appVersionTooOld(minimumRequired: minAppVersion, currentApp: appVersion) + } + } + + if source == .userInstalled { + try verifyCodeSignature(bundle: bundle) + } + + pendingPluginURLs.append((url: url, source: source)) + } + @discardableResult private func loadPlugin(at url: URL, source: PluginSource) throws -> PluginEntry { guard let bundle = Bundle(url: url) else { diff --git a/TablePro/Models/Query/QueryTab.swift b/TablePro/Models/Query/QueryTab.swift index 97f6561c..b5db3f37 100644 --- a/TablePro/Models/Query/QueryTab.swift +++ b/TablePro/Models/Query/QueryTab.swift @@ -254,9 +254,6 @@ final class RowBuffer { /// Whether this buffer's row data has been evicted to save memory private(set) var isEvicted: Bool = false - /// The query that produced this data (used to re-fetch after eviction) - var sourceQuery: String? - /// Evict row data to free memory. Column metadata is preserved. func evict() { guard !isEvicted else { return } diff --git a/TablePro/Views/Export/ExportDialog.swift b/TablePro/Views/Export/ExportDialog.swift index 49801e01..afec5710 100644 --- a/TablePro/Views/Export/ExportDialog.swift +++ b/TablePro/Views/Export/ExportDialog.swift @@ -63,6 +63,7 @@ struct ExportDialog: View { .frame(width: dialogWidth) .background(Color(nsColor: .windowBackgroundColor)) .onAppear { + PluginManager.shared.loadPendingPlugins() let available = availableFormats if !available.contains(where: { type(of: $0).formatId == config.formatId }) { if let first = available.first { diff --git a/TablePro/Views/Main/Child/MainEditorContentView.swift b/TablePro/Views/Main/Child/MainEditorContentView.swift index c6c27635..815c10f3 100644 --- a/TablePro/Views/Main/Child/MainEditorContentView.swift +++ b/TablePro/Views/Main/Child/MainEditorContentView.swift @@ -321,6 +321,10 @@ struct MainEditorContentView: View { } private func rowProvider(for tab: QueryTab) -> InMemoryRowProvider { + if tab.rowBuffer.isEvicted { + tabRowProviders.removeValue(forKey: tab.id) + return makeRowProvider(for: tab) + } if let cached = tabRowProviders[tab.id], tabProviderVersions[tab.id] == tab.resultVersion, tabProviderMetaVersions[tab.id] == tab.metadataVersion { diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+TabSwitch.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+TabSwitch.swift index e6aacf1d..71987fe6 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+TabSwitch.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+TabSwitch.swift @@ -124,7 +124,6 @@ extension MainContentCoordinator { let toEvict = sorted.dropLast(maxInactiveLoaded) for tab in toEvict { - tab.rowBuffer.sourceQuery = tab.query tab.rowBuffer.evict() } } diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index c1e528d6..99c499a8 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -127,6 +127,18 @@ final class MainContentCoordinator { } }() + /// Evict row data for all tabs in this coordinator to free memory. + /// Called when the coordinator's native window-tab becomes inactive. + /// Data is re-fetched automatically when the tab becomes active again. + func evictInactiveRowData() { + for tab in tabManager.tabs where !tab.rowBuffer.isEvicted + && !tab.resultRows.isEmpty + && !tab.pendingChanges.hasChanges + { + tab.rowBuffer.evict() + } + } + /// Remove sort cache entries for tabs that no longer exist func cleanupSortCache(openTabIds: Set) { if querySortCache.keys.contains(where: { !openTabIds.contains($0) }) { diff --git a/TablePro/Views/Main/MainContentView.swift b/TablePro/Views/Main/MainContentView.swift index 7e3b40da..a29b286a 100644 --- a/TablePro/Views/Main/MainContentView.swift +++ b/TablePro/Views/Main/MainContentView.swift @@ -45,6 +45,7 @@ struct MainContentView: View { @State private var queryResultsSummaryCache: (tabId: UUID, version: Int, summary: String?)? @State private var inspectorUpdateTask: Task? @State private var pendingTabSwitch: Task? + @State private var evictionTask: Task? /// Stable identifier for this window in WindowLifecycleMonitor @State private var windowId = UUID() @State private var hasInitialized = false @@ -303,6 +304,8 @@ struct MainContentView: View { guard let notificationWindow = notification.object as? NSWindow, notificationWindow === viewWindow else { return } isKeyWindow = true + evictionTask?.cancel() + evictionTask = nil DispatchQueue.main.async { syncSidebarToCurrentTab() } @@ -320,6 +323,15 @@ struct MainContentView: View { guard let notificationWindow = notification.object as? NSWindow, notificationWindow === viewWindow else { return } isKeyWindow = false + + // Schedule row data eviction for inactive native window-tabs. + // 5s delay avoids thrashing when quickly switching between tabs. + evictionTask?.cancel() + evictionTask = Task { @MainActor in + try? await Task.sleep(for: .seconds(5)) + guard !Task.isCancelled else { return } + coordinator.evictInactiveRowData() + } } .onChange(of: tables) { _, newTables in let syncAction = SidebarSyncAction.resolveOnTablesLoad( diff --git a/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift b/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift index 507dc199..93110d8a 100644 --- a/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift +++ b/TablePro/Views/Settings/Plugins/InstalledPluginsView.swift @@ -81,6 +81,9 @@ struct InstalledPluginsView: View { } return true } + .onAppear { + pluginManager.loadPendingPlugins() + } .alert(errorAlertTitle, isPresented: $showErrorAlert) { Button("OK") {} } message: { diff --git a/TableProTests/Core/Plugins/PluginLazyLoadingTests.swift b/TableProTests/Core/Plugins/PluginLazyLoadingTests.swift new file mode 100644 index 00000000..7d34ac3d --- /dev/null +++ b/TableProTests/Core/Plugins/PluginLazyLoadingTests.swift @@ -0,0 +1,47 @@ +// +// PluginLazyLoadingTests.swift +// TableProTests +// +// Tests for lazy plugin loading behavior +// + +import Foundation +import Testing +@testable import TablePro + +@Suite("Plugin Lazy Loading") +@MainActor +struct PluginLazyLoadingTests { + @Test("loadPendingPlugins is idempotent when called multiple times") + func loadPendingPluginsIdempotent() { + // loadPendingPlugins should not crash or duplicate when called multiple times + let manager = PluginManager.shared + manager.loadPendingPlugins() + let countAfterFirst = manager.plugins.count + manager.loadPendingPlugins() + let countAfterSecond = manager.plugins.count + #expect(countAfterFirst == countAfterSecond) + } + + @Test("loadPendingPlugins populates driverPlugins") + func loadPendingPopulatesDrivers() { + let manager = PluginManager.shared + manager.loadPendingPlugins() + // After loading, at least some driver plugins should be registered + // (the built-in plugins are always available in the test bundle) + #expect(manager.driverPlugins.isEmpty == false || manager.plugins.isEmpty) + } + + @Test("loadPendingPlugins with no pending is no-op") + func loadPendingNoPendingIsNoOp() { + let manager = PluginManager.shared + // Ensure all pending are loaded first + manager.loadPendingPlugins() + let driverCount = manager.driverPlugins.count + let pluginCount = manager.plugins.count + // Call again - should be no-op + manager.loadPendingPlugins() + #expect(manager.driverPlugins.count == driverCount) + #expect(manager.plugins.count == pluginCount) + } +} diff --git a/TableProTests/Models/RowBufferTests.swift b/TableProTests/Models/RowBufferTests.swift new file mode 100644 index 00000000..d14f4890 --- /dev/null +++ b/TableProTests/Models/RowBufferTests.swift @@ -0,0 +1,111 @@ +import Foundation +import Testing +@testable import TablePro + +@Suite("RowBuffer") +struct RowBufferTests { + // MARK: - Initialization + + @Test("Init with default values creates empty buffer") + func initDefaults() { + let buffer = RowBuffer() + #expect(buffer.rows.isEmpty) + #expect(buffer.columns.isEmpty) + #expect(buffer.columnTypes.isEmpty) + #expect(buffer.isEvicted == false) + } + + @Test("Init with data preserves all fields") + func initWithData() { + let rows = TestFixtures.makeQueryResultRows(count: 5) + let buffer = RowBuffer( + rows: rows, + columns: ["id", "name", "email"], + columnTypes: [.integer(rawType: "INT"), .text(rawType: "VARCHAR"), .text(rawType: "VARCHAR")] + ) + #expect(buffer.rows.count == 5) + #expect(buffer.columns == ["id", "name", "email"]) + #expect(buffer.columnTypes.count == 3) + #expect(buffer.isEvicted == false) + } + + // MARK: - Eviction + + @Test("evict() clears rows and sets isEvicted") + func evictClearsRows() { + let buffer = RowBuffer(rows: TestFixtures.makeQueryResultRows(count: 10), columns: ["a"]) + buffer.evict() + #expect(buffer.rows.isEmpty) + #expect(buffer.isEvicted == true) + } + + @Test("evict() preserves column metadata") + func evictPreservesMetadata() { + let fk = TestFixtures.makeForeignKeyInfo() + let buffer = RowBuffer( + rows: TestFixtures.makeQueryResultRows(count: 3), + columns: ["id", "user_id"], + columnTypes: [.integer(rawType: "INT"), .integer(rawType: "INT")], + columnDefaults: ["id": nil], + columnForeignKeys: ["user_id": fk], + columnEnumValues: ["status": ["a", "b"]], + columnNullable: ["id": false] + ) + buffer.evict() + #expect(buffer.columns == ["id", "user_id"]) + #expect(buffer.columnTypes.count == 2) + #expect(buffer.columnForeignKeys["user_id"]?.name == "fk_user") + #expect(buffer.columnEnumValues["status"] == ["a", "b"]) + #expect(buffer.columnNullable["id"] == false) + } + + @Test("Double evict is no-op") + func doubleEvictNoOp() { + let buffer = RowBuffer(rows: TestFixtures.makeQueryResultRows(count: 3), columns: ["a"]) + buffer.evict() + buffer.evict() + #expect(buffer.isEvicted == true) + #expect(buffer.rows.isEmpty) + } + + // MARK: - Restore + + @Test("restore() repopulates rows and clears isEvicted") + func restoreRepopulates() { + let buffer = RowBuffer(rows: TestFixtures.makeQueryResultRows(count: 3), columns: ["a"]) + buffer.evict() + #expect(buffer.isEvicted == true) + + let newRows = TestFixtures.makeQueryResultRows(count: 5) + buffer.restore(rows: newRows) + #expect(buffer.rows.count == 5) + #expect(buffer.isEvicted == false) + } + + @Test("restore() with empty rows clears eviction flag") + func restoreEmptyRows() { + let buffer = RowBuffer(rows: TestFixtures.makeQueryResultRows(count: 3), columns: ["a"]) + buffer.evict() + buffer.restore(rows: []) + #expect(buffer.isEvicted == false) + #expect(buffer.rows.isEmpty) + } + + // MARK: - Copy + + @Test("copy() creates independent buffer") + func copyCreatesIndependent() { + let original = RowBuffer(rows: TestFixtures.makeQueryResultRows(count: 3), columns: ["a", "b"]) + let copied = original.copy() + copied.rows.removeAll() + #expect(original.rows.count == 3) + #expect(copied.rows.isEmpty) + } + + @Test("copy() preserves eviction state as false") + func copyPreservesNonEvictedState() { + let original = RowBuffer(rows: TestFixtures.makeQueryResultRows(count: 3), columns: ["a"]) + let copied = original.copy() + #expect(copied.isEvicted == false) + } +} diff --git a/TableProTests/Views/Main/EvictionTests.swift b/TableProTests/Views/Main/EvictionTests.swift new file mode 100644 index 00000000..f98cffe9 --- /dev/null +++ b/TableProTests/Views/Main/EvictionTests.swift @@ -0,0 +1,112 @@ +// +// EvictionTests.swift +// TableProTests +// +// Tests for cross-window tab eviction +// + +import Foundation +import Testing +@testable import TablePro + +@Suite("Cross-Window Tab Eviction") +@MainActor +struct EvictionTests { + private func makeCoordinator() -> (MainContentCoordinator, QueryTabManager) { + let tabManager = QueryTabManager() + let changeManager = DataChangeManager() + let filterStateManager = FilterStateManager() + let toolbarState = ConnectionToolbarState() + let connection = TestFixtures.makeConnection() + let coordinator = MainContentCoordinator( + connection: connection, + tabManager: tabManager, + changeManager: changeManager, + filterStateManager: filterStateManager, + toolbarState: toolbarState + ) + return (coordinator, tabManager) + } + + private func addLoadedTab(to tabManager: QueryTabManager, tableName: String = "users") { + tabManager.addTableTab(tableName: tableName) + guard let index = tabManager.selectedTabIndex else { return } + let rows = TestFixtures.makeQueryResultRows(count: 10) + tabManager.tabs[index].rowBuffer.rows = rows + tabManager.tabs[index].rowBuffer.columns = ["id", "name", "email"] + tabManager.tabs[index].lastExecutedAt = Date() + } + + @Test("evictInactiveRowData evicts loaded tabs without pending changes") + func evictsLoadedTabs() { + let (coordinator, tabManager) = makeCoordinator() + addLoadedTab(to: tabManager, tableName: "users") + + #expect(tabManager.tabs[0].resultRows.count == 10) + #expect(tabManager.tabs[0].rowBuffer.isEvicted == false) + + coordinator.evictInactiveRowData() + + #expect(tabManager.tabs[0].rowBuffer.isEvicted == true) + #expect(tabManager.tabs[0].resultRows.isEmpty) + } + + @Test("evictInactiveRowData skips tabs with pending changes") + func skipsTabsWithPendingChanges() { + let (coordinator, tabManager) = makeCoordinator() + addLoadedTab(to: tabManager, tableName: "users") + + // Add a pending change + tabManager.tabs[0].pendingChanges.deletedRowIndices = [0] + + coordinator.evictInactiveRowData() + + // Should NOT be evicted because it has pending changes + #expect(tabManager.tabs[0].rowBuffer.isEvicted == false) + #expect(tabManager.tabs[0].resultRows.count == 10) + } + + @Test("evictInactiveRowData skips already evicted tabs") + func skipsAlreadyEvicted() { + let (coordinator, tabManager) = makeCoordinator() + addLoadedTab(to: tabManager, tableName: "users") + + // Pre-evict + tabManager.tabs[0].rowBuffer.evict() + #expect(tabManager.tabs[0].rowBuffer.isEvicted == true) + + // Should not crash or change state + coordinator.evictInactiveRowData() + #expect(tabManager.tabs[0].rowBuffer.isEvicted == true) + } + + @Test("evictInactiveRowData skips tabs with empty results") + func skipsEmptyResults() { + let (coordinator, tabManager) = makeCoordinator() + tabManager.addTableTab(tableName: "empty_table") + // Don't add any rows — resultRows is empty + + coordinator.evictInactiveRowData() + + // Should not evict (nothing to evict) + #expect(tabManager.tabs[0].rowBuffer.isEvicted == false) + } + + @Test("evictInactiveRowData preserves column metadata after eviction") + func preservesMetadataAfterEviction() { + let (coordinator, tabManager) = makeCoordinator() + addLoadedTab(to: tabManager, tableName: "users") + + coordinator.evictInactiveRowData() + + #expect(tabManager.tabs[0].rowBuffer.columns == ["id", "name", "email"]) + #expect(tabManager.tabs[0].rowBuffer.isEvicted == true) + } + + @Test("evictInactiveRowData with no tabs is no-op") + func noTabsIsNoOp() { + let (coordinator, _) = makeCoordinator() + // No tabs added — should not crash + coordinator.evictInactiveRowData() + } +} diff --git a/docs/development/MEMORY-AUDIT.md b/docs/development/MEMORY-AUDIT.md new file mode 100644 index 00000000..dff1062f --- /dev/null +++ b/docs/development/MEMORY-AUDIT.md @@ -0,0 +1,204 @@ +# Memory Audit — TablePro + +**Date:** 2026-03-09 +**Baseline measurements** (MySQL connection, default page size 1,000 rows): + +| State | Memory | Delta | +|-------|--------|-------| +| Welcome screen | ~40 MB | — | +| Connect to MySQL | ~150 MB | +110 MB | +| First table tab | ~160 MB | +10 MB | +| Second table tab | ~250 MB | +90 MB | +| Third table tab | ~380 MB | +130 MB | + +--- + +## Root Causes (ranked by impact) + +### 1. Three database driver instances per connection + +**Impact: ~90–150 MB fixed cost on connect** +**Files:** `Core/Database/DatabaseManager.swift` lines 159, 223, 484 + +Every connection creates three separate C-level driver instances: +- **Main driver** (line 159) — user queries +- **Metadata driver** (line 223) — background FK/column fetches +- **Ping driver** (line 484) — 30s health check + +Each carries its own C library state (libmariadb internal buffers, TLS context, TCP socket). For MySQL this is ~30–50 MB per connection × 3 = the bulk of the +110 MB jump on connect. + +**Fix options:** +- [ ] Multiplex metadata queries on the main driver when idle (eliminates 1 driver) +- [x] Use a lightweight ping mechanism (e.g., TCP keepalive or `mysql_ping` on main driver with a mutex) instead of a dedicated ping driver — **Done**: removed dedicated `pingDrivers` dict, health checks now use main driver +- [ ] Lazy-create the metadata driver only when needed, release after use + +--- + +### 2. Triple-copy of row data per tab + +**Impact: 3–10 MB duplicated per tab (more for wide/text-heavy tables)** +**Files:** +- `Models/Query/QueryTab.swift` — `RowBuffer.rows` (canonical store) +- `Models/Query/RowProvider.swift` lines 65–97 — `InMemoryRowProvider.sourceRows` (second copy) +- `Views/Main/Child/MainEditorContentView.swift` lines 332–342 — `makeRowProvider()` creates the provider + +**How it happens:** +1. `RowBuffer.rows: [QueryResultRow]` holds the query result (copy 1) +2. `makeRowProvider()` passes rows into `InMemoryRowProvider(rows:)`, which stores them as `sourceRows` (copy 2 — CoW breaks on first edit) +3. `InMemoryRowProvider.rowCache: [Int: TableRowData]` materializes up to 5,000 `TableRowData` objects, each wrapping another `[String?]` (copy 3) + +For sorted tabs, `sortedRows(for:)` at `MainEditorContentView.swift:406` calls `.map { tab.resultRows[$0] }`, materializing yet another full `[QueryResultRow]` array before handing it to the provider. + +**Fix options:** +- [ ] Make `InMemoryRowProvider` reference `RowBuffer` directly instead of copying rows +- [ ] Replace `rowCache` with index-based access into `sourceRows` (avoid `TableRowData` wrapper objects) +- [ ] For sorted tabs, store only the index permutation and let the provider apply it lazily + +--- + +### 3. Tab eviction does not work for native window-tabs + +**Impact: All inactive tabs retain full row data indefinitely** +**Files:** +- `Views/Main/Extensions/MainContentCoordinator+TabSwitch.swift` lines 109–130 + +`evictInactiveTabs(excluding:)` only fires when `tabManager.tabs.count > 2`. Since each native macOS tab is a separate `NSWindow` with its own `MainContentCoordinator` and `QueryTabManager` containing exactly 1 tab, the condition `tabs.count > 2` is never true for table tabs. Eviction never triggers. + +Even when eviction does trigger (multiple query sub-tabs within one window), `RowBuffer.evict()` clears `rows = []` but the `InMemoryRowProvider` in `MainEditorContentView.tabRowProviders` still holds its own `sourceRows` copy — it is not cleared. + +**Fix options:** +- [ ] Implement cross-window eviction via a global `TabMemoryManager` that tracks all open tabs across windows +- [x] When `RowBuffer.evict()` fires, also notify/invalidate the corresponding `InMemoryRowProvider` — **Done**: `rowProvider(for:)` checks `isEvicted` and drops cached provider +- [x] Add `NSWindow.didResignKeyNotification` observer to trigger proactive eviction — **Done**: `MainContentView` schedules 5s delayed eviction via `evictInactiveRowData()` on window resign + +--- + +### 4. All 8 plugin bundles loaded unconditionally at launch + +**Impact: ~20–40 MB at launch** +**File:** `Core/Plugins/PluginManager.swift` lines 44–63, 111 + +`loadAllPlugins()` calls `bundle.load()` on every `.tableplugin` bundle at startup, linking in static libraries for all 8 database engines (libmariadb, libpq, libfreetds, libmongoc, hiredis, etc.) regardless of whether the user connects to that database type. + +**Fix options:** +- [x] Lazy-load plugins: only call `bundle.load()` when a connection of that type is first established — **Done**: `discoverPlugins()` reads Info.plist at launch, `loadPendingPlugins()` defers `bundle.load()` until first driver request +- [x] Keep the plugin discovery (reading Info.plist) at launch but defer `bundle.load()` + principal class instantiation — **Done**: same as above + +--- + +### 5. Undo stack stores full row copies + +**Impact: Up to 100 × row-width per tab with unsaved changes** +**Files:** +- `Core/ChangeTracking/DataChangeUndoManager.swift` lines 14–19 +- `Core/ChangeTracking/DataChangeModels.swift` lines 82–84 + +`UndoAction.rowDeletion(rowIndex:originalRow:)` stores a complete `[String?]` for every deleted row. The undo stack is capped at 100 entries, but batch deletions (`.batchRowDeletion(rows:)`) can store hundreds of full row copies in a single entry. On a table with 50 columns and large text values, this can consume tens of MB. + +**Fix options:** +- [ ] Store only changed cells (column index + old value) instead of full rows for modifications +- [ ] For deletions, store row indices and reconstruct from `RowBuffer` on undo (if rows haven't been evicted) +- [ ] Reduce `maxUndoDepth` for batch operations or cap total undo memory + +--- + +### 6. Duplicate schema/table list + +**Impact: ~1–5 MB for large schemas (500+ tables)** +**Files:** +- `Models/Connection/ConnectionSession.swift` line 23 — `tables: [TableInfo]` +- `Core/Autocomplete/SQLSchemaProvider.swift` line 14 — `tables: [TableInfo]` + +The table list is stored in both `ConnectionSession.tables` (used by sidebar) and `SQLSchemaProvider.tables` (used by autocomplete). These are independent copies of the same data. + +**Fix options:** +- [ ] Make `SQLSchemaProvider` the single source of truth; have sidebar read from it +- [ ] Or store tables only in `ConnectionSession` and have `SQLSchemaProvider` reference it + +--- + +### 7. `TabPendingChanges` duplicated during tab switch + +**Impact: Proportional to unsaved edits, transient** +**Files:** +- `Models/Query/QueryTab.swift` line 338 — `pendingChanges: TabPendingChanges` +- `Core/ChangeTracking/DataChangeManager.swift` — live change state + +On tab switch, `DataChangeManager.saveState()` copies all changes into `QueryTab.pendingChanges`. During the transition, both `DataChangeManager` and `TabPendingChanges` hold the same data. The old `DataChangeManager` state is cleared after, but there's a window of double memory. + +**Fix options:** +- [ ] Use move semantics (`consuming` parameter) to transfer ownership instead of copying + +--- + +### 8. `NSWindow.didUpdateNotification` observer with no window filter + +**Impact: CPU overhead (not memory), scales with tab count** +**File:** `Views/Editor/SQLEditorCoordinator.swift` lines 273–283 + +Each `SQLEditorCoordinator` registers for `NSWindow.didUpdateNotification` with `object: nil`, meaning every editor instance's closure fires on every window update cycle across all windows. With N query tabs, N closures fire per update cycle. + +**Fix options:** +- [ ] Filter by `object: textView.window` after the editor's window is known +- [ ] Or use KVO on the specific window's `firstResponder` instead + +--- + +### 9. `RowBuffer.sourceQuery` duplicates `QueryTab.query` + +**Impact: Up to query size per tab (typically <500 KB)** +**Files:** +- `Models/Query/QueryTab.swift` line 258 — `RowBuffer.sourceQuery` +- `Views/Main/Extensions/MainContentCoordinator+TabSwitch.swift` line 127 + +Before eviction, `tab.query` is copied into `rowBuffer.sourceQuery` so the query can be re-executed on rehydration. Both `tab.query` and `rowBuffer.sourceQuery` then hold the same string. + +**Fix options:** +- [x] Remove `sourceQuery` from `RowBuffer`; read from `tab.query` on rehydration instead — **Done**: removed `sourceQuery` property and its assignment in eviction + +--- + +### 10. Per-window overhead from native tab architecture + +**Impact: ~5–15 MB per window (coordinator + services + NSWindow object tree)** + +Each macOS native tab creates a full `NSWindow` with its own: +- `MainContentCoordinator` (owns `QueryTabManager`, `DataChangeManager`, `FilterStateManager`, `ConnectionToolbarState`, `TabPersistenceCoordinator`, `RowOperationsManager`) +- `NSTableView` + `NSTableColumn[]` for the data grid +- `NSScrollView` with its clip view and scroller views +- SwiftUI hosting infrastructure + +This is inherent to the native-tab architecture and not easily reducible without moving to in-process tabs. + +**Fix options:** +- [ ] Consider lightweight in-process tab bar (shared NSWindow) for table tabs — major architectural change +- [ ] Or accept this as the cost of native tabs and focus on reducing per-tab data overhead (items 2, 3, 5 above) + +--- + +## Quick Wins (low effort, high impact) + +| # | Fix | Expected Savings | Effort | Status | +|---|-----|-------------------|--------|--------| +| 3 | Cross-window eviction + clear `InMemoryRowProvider` on evict | Reclaim ~90 MB per evicted tab | Medium | **Done** | +| 2 | `InMemoryRowProvider` references `RowBuffer` instead of copying | -3–10 MB per tab | Medium | Open | +| 9 | Remove `RowBuffer.sourceQuery`, use `tab.query` | -0–500 KB per tab | Low | **Done** | +| 4 | Lazy plugin loading | -20–30 MB at launch | Low-Medium | **Done** | +| 1 | Eliminate dedicated ping driver | -30–50 MB per connection | Medium | **Done** | + +--- + +## Measurement Plan + +To validate fixes, use Xcode Instruments with the **Allocations** and **Leaks** templates: + +1. Launch app → record baseline +2. Connect to MySQL → record delta +3. Open table tab → record delta +4. Open 2 more table tabs → record per-tab delta +5. Close middle tab → verify memory is reclaimed +6. Switch tabs → verify eviction triggers + +Track `VM Regions` and `All Heap Allocations` grouped by category. Filter by `TablePro` to exclude system framework overhead. + +Use `vmmap --summary ` for a quick per-region breakdown from Terminal. From 80969a15efb0bbad14bc34d3a0774f31b0c9e491 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 9 Mar 2026 18:06:12 +0700 Subject: [PATCH 2/5] perf: lazy metadata driver, zero-copy row provider, window-scoped notifications --- CHANGELOG.md | 3 + TablePro/Core/Database/DatabaseManager.swift | 119 ++++++++-------- TablePro/Models/Query/RowProvider.swift | 127 +++++++++++++++--- .../Views/Editor/SQLEditorCoordinator.swift | 5 +- .../Main/Child/MainEditorContentView.swift | 28 ++-- .../Views/Main/MainContentCoordinator.swift | 7 +- docs/development/MEMORY-AUDIT.md | 12 +- 7 files changed, 195 insertions(+), 106 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5a418d9..32e13826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Reduce memory: evict inactive native window-tab row data after 5s, re-fetch on focus - Reduce memory: lazy-load plugin bundles on first use instead of at startup (~20-30 MB saved) - Reduce memory: remove duplicate sourceQuery string from RowBuffer +- Reduce memory: InMemoryRowProvider references RowBuffer directly instead of copying rows (~3-10 MB per tab) +- Reduce memory: lazy-create metadata driver on first use instead of eagerly on connect (~30-50 MB per connection) +- Reduce CPU: filter NSWindow.didUpdateNotification by editor's own window (N editors → 1 handler per update cycle) - Split DatabaseManager.sessionVersion into fine-grained connectionListVersion and connectionStatusVersion to reduce cascade re-renders - Extract AppState property reads into local lets in view bodies for explicit granular observation tracking - Reorganized project directory structure: Services, Utilities, Models split into domain-specific subdirectories diff --git a/TablePro/Core/Database/DatabaseManager.swift b/TablePro/Core/Database/DatabaseManager.swift index 4b440ca3..b4fc38e8 100644 --- a/TablePro/Core/Database/DatabaseManager.swift +++ b/TablePro/Core/Database/DatabaseManager.swift @@ -69,6 +69,60 @@ final class DatabaseManager { activeSessions[connectionId]?.metadataDriver } + /// Lazily create the metadata driver for a connection if it doesn't exist yet. + /// Returns the metadata driver on success, or nil if creation fails (non-fatal). + /// Callers should fall back to the main driver when this returns nil. + @discardableResult + func ensureMetadataDriver(for connectionId: UUID) async -> DatabaseDriver? { + // Already created — return immediately + if let existing = activeSessions[connectionId]?.metadataDriver { + return existing + } + + // Already being created — wait for the in-flight task + if let existingTask = metadataCreationTasks[connectionId] { + await existingTask.value + return activeSessions[connectionId]?.metadataDriver + } + + guard let session = activeSessions[connectionId] else { return nil } + + let effectiveConnection = session.effectiveConnection ?? session.connection + let timeoutSeconds = AppSettingsManager.shared.general.queryTimeoutSeconds + let startupCmds = session.connection.startupCommands + let connName = session.connection.name + + let task = Task { [weak self] in + guard let self else { return } + defer { self.metadataCreationTasks.removeValue(forKey: connectionId) } + do { + let metaDriver = try DatabaseDriverFactory.createDriver(for: effectiveConnection) + try await metaDriver.connect() + if timeoutSeconds > 0 { + try? await metaDriver.applyQueryTimeout(timeoutSeconds) + } + await self.executeStartupCommands( + startupCmds, on: metaDriver, connectionName: connName + ) + if let savedSchema = self.activeSessions[connectionId]?.currentSchema, + let schemaMetaDriver = metaDriver as? SchemaSwitchable { + try? await schemaMetaDriver.switchSchema(to: savedSchema) + } + if let savedDatabase = self.activeSessions[connectionId]?.currentDatabase, + let adapter = metaDriver as? PluginDriverAdapter { + try? await adapter.switchDatabase(to: savedDatabase) + } + self.activeSessions[connectionId]?.metadataDriver = metaDriver + } catch { + Self.logger.warning("Metadata connection failed: \(error.localizedDescription)") + } + } + + metadataCreationTasks[connectionId] = task + await task.value + return activeSessions[connectionId]?.metadataDriver + } + /// Resolve a session by explicit connection ID func session(for connectionId: UUID) -> ConnectionSession? { activeSessions[connectionId] @@ -207,33 +261,9 @@ final class DatabaseManager { await startHealthMonitor(for: connection.id) } - // Create a dedicated metadata connection in the background so Phase 2 - // metadata queries (columns, FKs, count) run in parallel with main queries. - let metaConnection = effectiveConnection - let metaConnectionId = connection.id - let metaTimeout = AppSettingsManager.shared.general.queryTimeoutSeconds - metadataCreationTasks[metaConnectionId] = Task { [weak self] in - guard let self else { return } - defer { self.metadataCreationTasks.removeValue(forKey: metaConnectionId) } - do { - let metaDriver = try DatabaseDriverFactory.createDriver(for: metaConnection) - try await metaDriver.connect() - if metaTimeout > 0 { - try? await metaDriver.applyQueryTimeout(metaTimeout) - } - await self.executeStartupCommands( - connection.startupCommands, on: metaDriver, connectionName: connection.name - ) - if let savedSchema = self.activeSessions[metaConnectionId]?.currentSchema, - let schemaMetaDriver = metaDriver as? SchemaSwitchable { - try? await schemaMetaDriver.switchSchema(to: savedSchema) - } - activeSessions[metaConnectionId]?.metadataDriver = metaDriver - } catch { - // Non-fatal: Phase 2 falls back to main driver if metadata driver unavailable - Self.logger.warning("Metadata connection failed: \(error.localizedDescription)") - } - } + // Metadata driver is created lazily on first access via ensureMetadataDriver(for:) + // to avoid allocating ~30-50 MB for a second C-level driver when no metadata + // queries are needed (e.g. query-only tabs, short-lived connections). } catch { // Close tunnel if connection failed if connection.sshConfig.enabled { @@ -636,39 +666,8 @@ final class DatabaseManager { session.effectiveConnection = effectiveConnection } - // Recreate metadata connection in background - let metaConnection = effectiveConnection - let metaConnectionId = sessionId - let metaTimeout = AppSettingsManager.shared.general.queryTimeoutSeconds - let startupCmds = session.connection.startupCommands - let connName = session.connection.name - metadataCreationTasks[metaConnectionId] = Task { [weak self] in - guard let self else { return } - defer { self.metadataCreationTasks.removeValue(forKey: metaConnectionId) } - do { - let metaDriver = try DatabaseDriverFactory.createDriver(for: metaConnection) - try await metaDriver.connect() - if metaTimeout > 0 { - try? await metaDriver.applyQueryTimeout(metaTimeout) - } - await self.executeStartupCommands( - startupCmds, on: metaDriver, connectionName: connName - ) - if let savedSchema = self.activeSessions[metaConnectionId]?.currentSchema, - let schemaMetaDriver = metaDriver as? SchemaSwitchable { - try? await schemaMetaDriver.switchSchema(to: savedSchema) - } - // Restore database on metadata driver too for MSSQL - if let savedDatabase = self.activeSessions[metaConnectionId]?.currentDatabase, - let adapter = metaDriver as? PluginDriverAdapter { - try? await adapter.switchDatabase(to: savedDatabase) - } - activeSessions[metaConnectionId]?.metadataDriver = metaDriver - } catch { - Self.logger.warning( - "Metadata reconnection failed: \(error.localizedDescription)") - } - } + // Metadata driver is created lazily via ensureMetadataDriver(for:). + // Previous metadata driver was already disconnected above. // Restart health monitoring if session.connection.type != .sqlite { diff --git a/TablePro/Models/Query/RowProvider.swift b/TablePro/Models/Query/RowProvider.swift index c525f260..c004ecf3 100644 --- a/TablePro/Models/Query/RowProvider.swift +++ b/TablePro/Models/Query/RowProvider.swift @@ -62,10 +62,16 @@ final class TableRowData { /// Uses lazy TableRowData creation to avoid O(n) heap allocations on init. /// Cache is bounded to `maxCacheSize` entries; proximity-based eviction keeps /// the half closest to the current access point when the limit is exceeded. +/// +/// References `RowBuffer` directly to avoid duplicating row data. +/// An optional `sortIndices` array maps display indices to source-row indices, +/// so sorted views don't need a reordered copy of the rows. final class InMemoryRowProvider: RowProvider { private static let maxCacheSize = 5_000 - private var sourceRows: [QueryResultRow] + private let rowBuffer: RowBuffer + private var sortIndices: [Int]? + private var appendedRows: [QueryResultRow] = [] private var rowCache: [Int: TableRowData] = [:] private(set) var columns: [String] private(set) var columnDefaults: [String: String?] @@ -75,11 +81,17 @@ final class InMemoryRowProvider: RowProvider { private(set) var columnNullable: [String: Bool] var totalRowCount: Int { - sourceRows.count + bufferRowCount + appendedRows.count + } + + /// Number of rows coming from the buffer (respecting sort indices count when present) + private var bufferRowCount: Int { + sortIndices?.count ?? rowBuffer.rows.count } init( - rows: [QueryResultRow], + rowBuffer: RowBuffer, + sortIndices: [Int]? = nil, columns: [String], columnDefaults: [String: String?] = [:], columnTypes: [ColumnType]? = nil, @@ -87,17 +99,42 @@ final class InMemoryRowProvider: RowProvider { columnEnumValues: [String: [String]] = [:], columnNullable: [String: Bool] = [:] ) { + self.rowBuffer = rowBuffer + self.sortIndices = sortIndices self.columns = columns self.columnDefaults = columnDefaults self.columnTypes = columnTypes ?? Array(repeating: ColumnType.text(rawType: nil), count: columns.count) self.columnForeignKeys = columnForeignKeys self.columnEnumValues = columnEnumValues self.columnNullable = columnNullable - self.sourceRows = rows + } + + /// Convenience initializer that wraps rows in an internal RowBuffer. + /// Used by tests, previews, and callers that don't have a RowBuffer reference. + convenience init( + rows: [QueryResultRow], + columns: [String], + columnDefaults: [String: String?] = [:], + columnTypes: [ColumnType]? = nil, + columnForeignKeys: [String: ForeignKeyInfo] = [:], + columnEnumValues: [String: [String]] = [:], + columnNullable: [String: Bool] = [:] + ) { + let buffer = RowBuffer(rows: rows, columns: columns) + self.init( + rowBuffer: buffer, + columns: columns, + columnDefaults: columnDefaults, + columnTypes: columnTypes, + columnForeignKeys: columnForeignKeys, + columnEnumValues: columnEnumValues, + columnNullable: columnNullable + ) } func fetchRows(offset: Int, limit: Int) -> [TableRowData] { - let endIndex = min(offset + limit, sourceRows.count) + let total = totalRowCount + let endIndex = min(offset + limit, total) guard offset < endIndex else { return [] } var result: [TableRowData] = [] result.reserveCapacity(endIndex - offset) @@ -117,30 +154,37 @@ final class InMemoryRowProvider: RowProvider { /// Update a cell value func updateValue(_ value: String?, at rowIndex: Int, columnIndex: Int) { - guard rowIndex < sourceRows.count else { return } - // Update the source row - sourceRows[rowIndex].values[columnIndex] = value + guard rowIndex < totalRowCount else { return } + // Update the source row (buffer or appended) + let sourceIndex = resolveSourceIndex(rowIndex) + if let bufferIdx = sourceIndex.bufferIndex { + rowBuffer.rows[bufferIdx].values[columnIndex] = value + } else if let appendedIdx = sourceIndex.appendedIndex { + appendedRows[appendedIdx].values[columnIndex] = value + } // Update cached TableRowData if it exists rowCache[rowIndex]?.setValue(value, at: columnIndex) } /// Get row data at index func row(at index: Int) -> TableRowData? { - guard index >= 0 && index < sourceRows.count else { return nil } + guard index >= 0 && index < totalRowCount else { return nil } return materializeRow(at: index) } - /// Update rows from QueryResultRow array + /// Update rows by replacing the buffer contents and clearing appended rows func updateRows(_ newRows: [QueryResultRow]) { - self.sourceRows = newRows - self.rowCache.removeAll() + rowBuffer.rows = newRows + appendedRows.removeAll() + sortIndices = nil + rowCache.removeAll() } /// Append a new row with given values /// Returns the index of the new row func appendRow(values: [String?]) -> Int { - let newIndex = sourceRows.count - sourceRows.append(QueryResultRow(id: newIndex, values: values)) + let newIndex = totalRowCount + appendedRows.append(QueryResultRow(id: newIndex, values: values)) let rowData = TableRowData(index: newIndex, values: values) rowCache[newIndex] = rowData return newIndex @@ -148,8 +192,27 @@ final class InMemoryRowProvider: RowProvider { /// Remove row at index (used when discarding new rows) func removeRow(at index: Int) { - guard index >= 0 && index < sourceRows.count else { return } - sourceRows.remove(at: index) + guard index >= 0 && index < totalRowCount else { return } + let bCount = bufferRowCount + if index >= bCount { + // Removing from appended rows + let appendedIdx = index - bCount + guard appendedIdx < appendedRows.count else { return } + appendedRows.remove(at: appendedIdx) + } else { + // Removing from buffer rows + if let sorted = sortIndices { + let bufferIdx = sorted[index] + rowBuffer.rows.remove(at: bufferIdx) + // Rebuild sort indices: remove this entry and adjust indices above the removed one + var newIndices = sorted + newIndices.remove(at: index) + newIndices = newIndices.map { $0 > bufferIdx ? $0 - 1 : $0 } + sortIndices = newIndices + } else { + rowBuffer.rows.remove(at: index) + } + } // Clear entire cache since indices shift rowCache.removeAll() } @@ -158,15 +221,37 @@ final class InMemoryRowProvider: RowProvider { /// Indices should be sorted in descending order to maintain correct removal func removeRows(at indices: Set) { for index in indices.sorted(by: >) { - guard index >= 0 && index < sourceRows.count else { continue } - sourceRows.remove(at: index) + guard index >= 0 && index < totalRowCount else { continue } + removeRow(at: index) } - // Clear entire cache since indices shift - rowCache.removeAll() } // MARK: - Private + /// Resolve a display index to either a buffer index or an appended-row index. + private func resolveSourceIndex(_ displayIndex: Int) -> (bufferIndex: Int?, appendedIndex: Int?) { + let bCount = bufferRowCount + if displayIndex >= bCount { + return (nil, displayIndex - bCount) + } + if let sorted = sortIndices { + return (sorted[displayIndex], nil) + } + return (displayIndex, nil) + } + + /// Get the source QueryResultRow for a display index. + private func sourceRow(at displayIndex: Int) -> QueryResultRow { + let bCount = bufferRowCount + if displayIndex >= bCount { + return appendedRows[displayIndex - bCount] + } + if let sorted = sortIndices { + return rowBuffer.rows[sorted[displayIndex]] + } + return rowBuffer.rows[displayIndex] + } + private func materializeRow(at index: Int) -> TableRowData { if let cached = rowCache[index] { return cached @@ -176,7 +261,7 @@ final class InMemoryRowProvider: RowProvider { if rowCache.count >= Self.maxCacheSize { evictCacheIfNeeded(nearIndex: index) } - let rowData = TableRowData(index: index, values: sourceRows[index].values) + let rowData = TableRowData(index: index, values: sourceRow(at: index).values) rowCache[index] = rowData return rowData } diff --git a/TablePro/Views/Editor/SQLEditorCoordinator.swift b/TablePro/Views/Editor/SQLEditorCoordinator.swift index 03880cc0..9b188d1f 100644 --- a/TablePro/Views/Editor/SQLEditorCoordinator.swift +++ b/TablePro/Views/Editor/SQLEditorCoordinator.swift @@ -275,9 +275,10 @@ final class SQLEditorCoordinator: TextViewCoordinator { forName: NSWindow.didUpdateNotification, object: nil, queue: .main - ) { [weak self] _ in + ) { [weak self] notification in MainActor.assumeIsolated { - self?.checkFirstResponderChange() + guard let self, notification.object as? NSWindow == self.controller?.textView?.window else { return } + self.checkFirstResponderChange() } } } diff --git a/TablePro/Views/Main/Child/MainEditorContentView.swift b/TablePro/Views/Main/Child/MainEditorContentView.swift index 815c10f3..a2378955 100644 --- a/TablePro/Views/Main/Child/MainEditorContentView.swift +++ b/TablePro/Views/Main/Child/MainEditorContentView.swift @@ -335,7 +335,8 @@ struct MainEditorContentView: View { private func makeRowProvider(for tab: QueryTab) -> InMemoryRowProvider { InMemoryRowProvider( - rows: sortedRows(for: tab), + rowBuffer: tab.rowBuffer, + sortIndices: sortIndicesForTab(tab), columns: tab.resultColumns, columnDefaults: tab.columnDefaults, columnTypes: tab.columnTypes, @@ -345,31 +346,32 @@ struct MainEditorContentView: View { ) } - private func sortedRows(for tab: QueryTab) -> [QueryResultRow] { - guard !tab.rowBuffer.isEvicted else { return [] } + /// Returns sort index permutation for a tab, or nil if no sorting is needed. + /// For table tabs, sorting is handled server-side via SQL ORDER BY. + private func sortIndicesForTab(_ tab: QueryTab) -> [Int]? { + guard !tab.rowBuffer.isEvicted else { return nil } - // Table tabs: Don't apply client-side sorting (handled via SQL ORDER BY) + // Table tabs: no client-side sorting if tab.tabType == .table { - return tab.resultRows + return nil } - // Query tabs: Apply client-side sorting + // Query tabs: apply client-side sorting guard tab.sortState.isSorting else { - return tab.resultRows + return nil } // Check coordinator's async sort cache (for large datasets sorted on background thread) - // The cache stores index permutation to avoid duplicating all row data. if let cached = coordinator.querySortCache[tab.id], cached.columnIndex == (tab.sortState.columnIndex ?? -1), cached.direction == tab.sortState.direction, cached.resultVersion == tab.resultVersion { - return cached.sortedIndices.map { tab.resultRows[$0] } + return cached.sortedIndices } - // For large datasets sorted async, return unsorted until cache is ready + // For large datasets sorted async, return nil (unsorted) until cache is ready if tab.resultRows.count > 10_000 { - return tab.resultRows + return nil } // Small dataset: sort synchronously with view-level cache @@ -377,7 +379,7 @@ struct MainEditorContentView: View { cached.columnIndex == (tab.sortState.columnIndex ?? -1), cached.direction == tab.sortState.direction, cached.resultVersion == tab.resultVersion { - return cached.sortedIndices.map { tab.resultRows[$0] } + return cached.sortedIndices } let sortColumns = tab.sortState.columns @@ -407,7 +409,7 @@ struct MainEditorContentView: View { resultVersion: tab.resultVersion ) - return sortedIndices.map { tab.resultRows[$0] } + return sortedIndices } private func sortStateBinding(for tab: QueryTab) -> Binding { diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 99c499a8..1c55bca9 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -545,10 +545,9 @@ final class MainContentCoordinator { let cached = isMetadataCached(tabId: tabId, tableName: tableName) needsMetadataFetch = !cached - // If metadata is NOT cached and a dedicated metadata driver exists, - // start fetching columns+FKs on the separate connection so it runs - // in parallel with the main query. - if needsMetadataFetch, let metaDriver = DatabaseManager.shared.metadataDriver(for: connectionId) { + // If metadata is NOT cached, lazily create (or reuse) the dedicated + // metadata driver so Phase 2 queries run in parallel with the main query. + if needsMetadataFetch, let metaDriver = await DatabaseManager.shared.ensureMetadataDriver(for: connectionId) { parallelSchemaTask = Task { async let cols = metaDriver.fetchColumns(table: tableName) async let fks = metaDriver.fetchForeignKeys(table: tableName) diff --git a/docs/development/MEMORY-AUDIT.md b/docs/development/MEMORY-AUDIT.md index dff1062f..f1a3ab2b 100644 --- a/docs/development/MEMORY-AUDIT.md +++ b/docs/development/MEMORY-AUDIT.md @@ -30,7 +30,7 @@ Each carries its own C library state (libmariadb internal buffers, TLS context, **Fix options:** - [ ] Multiplex metadata queries on the main driver when idle (eliminates 1 driver) - [x] Use a lightweight ping mechanism (e.g., TCP keepalive or `mysql_ping` on main driver with a mutex) instead of a dedicated ping driver — **Done**: removed dedicated `pingDrivers` dict, health checks now use main driver -- [ ] Lazy-create the metadata driver only when needed, release after use +- [x] Lazy-create the metadata driver only when needed, release after use — **Done**: `ensureMetadataDriver(for:)` creates on first Phase 2 request; deduplicates in-flight creation tasks --- @@ -50,9 +50,9 @@ Each carries its own C library state (libmariadb internal buffers, TLS context, For sorted tabs, `sortedRows(for:)` at `MainEditorContentView.swift:406` calls `.map { tab.resultRows[$0] }`, materializing yet another full `[QueryResultRow]` array before handing it to the provider. **Fix options:** -- [ ] Make `InMemoryRowProvider` reference `RowBuffer` directly instead of copying rows +- [x] Make `InMemoryRowProvider` reference `RowBuffer` directly instead of copying rows — **Done**: primary init takes `rowBuffer: RowBuffer` + optional `sortIndices: [Int]?`; convenience init wraps rows for backward compat - [ ] Replace `rowCache` with index-based access into `sourceRows` (avoid `TableRowData` wrapper objects) -- [ ] For sorted tabs, store only the index permutation and let the provider apply it lazily +- [x] For sorted tabs, store only the index permutation and let the provider apply it lazily — **Done**: `sortIndicesForTab(_:)` returns `[Int]?` permutation; `InMemoryRowProvider` resolves display→source indices via `resolveSourceIndex()` --- @@ -139,7 +139,7 @@ On tab switch, `DataChangeManager.saveState()` copies all changes into `QueryTab Each `SQLEditorCoordinator` registers for `NSWindow.didUpdateNotification` with `object: nil`, meaning every editor instance's closure fires on every window update cycle across all windows. With N query tabs, N closures fire per update cycle. **Fix options:** -- [ ] Filter by `object: textView.window` after the editor's window is known +- [x] Filter by `object: textView.window` after the editor's window is known — **Done**: notification handler early-returns when `notification.object` doesn't match the editor's own window - [ ] Or use KVO on the specific window's `firstResponder` instead --- @@ -181,10 +181,10 @@ This is inherent to the native-tab architecture and not easily reducible without | # | Fix | Expected Savings | Effort | Status | |---|-----|-------------------|--------|--------| | 3 | Cross-window eviction + clear `InMemoryRowProvider` on evict | Reclaim ~90 MB per evicted tab | Medium | **Done** | -| 2 | `InMemoryRowProvider` references `RowBuffer` instead of copying | -3–10 MB per tab | Medium | Open | +| 2 | `InMemoryRowProvider` references `RowBuffer` instead of copying | -3–10 MB per tab | Medium | **Done** | | 9 | Remove `RowBuffer.sourceQuery`, use `tab.query` | -0–500 KB per tab | Low | **Done** | | 4 | Lazy plugin loading | -20–30 MB at launch | Low-Medium | **Done** | -| 1 | Eliminate dedicated ping driver | -30–50 MB per connection | Medium | **Done** | +| 1 | Eliminate dedicated ping driver + lazy metadata driver | -60–100 MB per connection | Medium | **Done** | --- From 2ab9bcf3c988c47ba294aca1ce93336f0cea7c9b Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 9 Mar 2026 18:17:01 +0700 Subject: [PATCH 3/5] perf: eliminate metadata driver, multiplex all queries on main driver --- CHANGELOG.md | 2 +- TablePro/Core/Database/DatabaseManager.swift | 82 ------------------- .../Models/Connection/ConnectionSession.swift | 3 +- .../MainContentCoordinator+Navigation.swift | 33 -------- .../Views/Main/MainContentCoordinator.swift | 19 +++-- .../Core/Database/DatabaseManagerTests.swift | 6 -- docs/development/MEMORY-AUDIT.md | 6 +- 7 files changed, 16 insertions(+), 135 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32e13826..9758e116 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Reduce memory: lazy-load plugin bundles on first use instead of at startup (~20-30 MB saved) - Reduce memory: remove duplicate sourceQuery string from RowBuffer - Reduce memory: InMemoryRowProvider references RowBuffer directly instead of copying rows (~3-10 MB per tab) -- Reduce memory: lazy-create metadata driver on first use instead of eagerly on connect (~30-50 MB per connection) +- Reduce memory: eliminate metadata driver entirely, multiplex all queries on main driver (~30-50 MB per connection) - Reduce CPU: filter NSWindow.didUpdateNotification by editor's own window (N editors → 1 handler per update cycle) - Split DatabaseManager.sessionVersion into fine-grained connectionListVersion and connectionStatusVersion to reduce cascade re-renders - Extract AppState property reads into local lets in view bodies for explicit granular observation tracking diff --git a/TablePro/Core/Database/DatabaseManager.swift b/TablePro/Core/Database/DatabaseManager.swift index b4fc38e8..a749d22e 100644 --- a/TablePro/Core/Database/DatabaseManager.swift +++ b/TablePro/Core/Database/DatabaseManager.swift @@ -40,8 +40,6 @@ final class DatabaseManager { /// Health monitors for active connections (MySQL/PostgreSQL only) private var healthMonitors: [UUID: ConnectionHealthMonitor] = [:] - private var metadataCreationTasks: [UUID: Task] = [:] - /// Current session (computed from currentSessionId) var currentSession: ConnectionSession? { guard let sessionId = currentSessionId else { return nil } @@ -53,76 +51,11 @@ final class DatabaseManager { currentSession?.driver } - /// Dedicated driver for metadata queries (columns, FKs, count). - /// Runs on a separate serial queue so metadata fetches don't block the main query. - var activeMetadataDriver: DatabaseDriver? { - currentSession?.metadataDriver - } - /// Resolve the driver for a specific connection (session-scoped, no global state) func driver(for connectionId: UUID) -> DatabaseDriver? { activeSessions[connectionId]?.driver } - /// Resolve the metadata driver for a specific connection - func metadataDriver(for connectionId: UUID) -> DatabaseDriver? { - activeSessions[connectionId]?.metadataDriver - } - - /// Lazily create the metadata driver for a connection if it doesn't exist yet. - /// Returns the metadata driver on success, or nil if creation fails (non-fatal). - /// Callers should fall back to the main driver when this returns nil. - @discardableResult - func ensureMetadataDriver(for connectionId: UUID) async -> DatabaseDriver? { - // Already created — return immediately - if let existing = activeSessions[connectionId]?.metadataDriver { - return existing - } - - // Already being created — wait for the in-flight task - if let existingTask = metadataCreationTasks[connectionId] { - await existingTask.value - return activeSessions[connectionId]?.metadataDriver - } - - guard let session = activeSessions[connectionId] else { return nil } - - let effectiveConnection = session.effectiveConnection ?? session.connection - let timeoutSeconds = AppSettingsManager.shared.general.queryTimeoutSeconds - let startupCmds = session.connection.startupCommands - let connName = session.connection.name - - let task = Task { [weak self] in - guard let self else { return } - defer { self.metadataCreationTasks.removeValue(forKey: connectionId) } - do { - let metaDriver = try DatabaseDriverFactory.createDriver(for: effectiveConnection) - try await metaDriver.connect() - if timeoutSeconds > 0 { - try? await metaDriver.applyQueryTimeout(timeoutSeconds) - } - await self.executeStartupCommands( - startupCmds, on: metaDriver, connectionName: connName - ) - if let savedSchema = self.activeSessions[connectionId]?.currentSchema, - let schemaMetaDriver = metaDriver as? SchemaSwitchable { - try? await schemaMetaDriver.switchSchema(to: savedSchema) - } - if let savedDatabase = self.activeSessions[connectionId]?.currentDatabase, - let adapter = metaDriver as? PluginDriverAdapter { - try? await adapter.switchDatabase(to: savedDatabase) - } - self.activeSessions[connectionId]?.metadataDriver = metaDriver - } catch { - Self.logger.warning("Metadata connection failed: \(error.localizedDescription)") - } - } - - metadataCreationTasks[connectionId] = task - await task.value - return activeSessions[connectionId]?.metadataDriver - } - /// Resolve a session by explicit connection ID func session(for connectionId: UUID) -> ConnectionSession? { activeSessions[connectionId] @@ -261,9 +194,6 @@ final class DatabaseManager { await startHealthMonitor(for: connection.id) } - // Metadata driver is created lazily on first access via ensureMetadataDriver(for:) - // to avoid allocating ~30-50 MB for a second C-level driver when no metadata - // queries are needed (e.g. query-only tabs, short-lived connections). } catch { // Close tunnel if connection failed if connection.sshConfig.enabled { @@ -308,14 +238,9 @@ final class DatabaseManager { try? await SSHTunnelManager.shared.closeTunnel(connectionId: session.connection.id) } - // Cancel any in-flight metadata driver creation - metadataCreationTasks[sessionId]?.cancel() - metadataCreationTasks.removeValue(forKey: sessionId) - // Stop health monitoring await stopHealthMonitor(for: sessionId) - session.metadataDriver?.disconnect() session.driver?.disconnect() activeSessions.removeValue(forKey: sessionId) @@ -344,9 +269,6 @@ final class DatabaseManager { await stopHealthMonitor(for: sessionId) } - for task in metadataCreationTasks.values { task.cancel() } - metadataCreationTasks.removeAll() - let sessionIds = Array(activeSessions.keys) for sessionId in sessionIds { await disconnectSession(sessionId) @@ -628,7 +550,6 @@ final class DatabaseManager { do { // Disconnect existing drivers - session.metadataDriver?.disconnect() session.driver?.disconnect() // Recreate SSH tunnel if needed and build effective connection @@ -666,9 +587,6 @@ final class DatabaseManager { session.effectiveConnection = effectiveConnection } - // Metadata driver is created lazily via ensureMetadataDriver(for:). - // Previous metadata driver was already disconnected above. - // Restart health monitoring if session.connection.type != .sqlite { await startHealthMonitor(for: sessionId) diff --git a/TablePro/Models/Connection/ConnectionSession.swift b/TablePro/Models/Connection/ConnectionSession.swift index 15d2d257..42d31787 100644 --- a/TablePro/Models/Connection/ConnectionSession.swift +++ b/TablePro/Models/Connection/ConnectionSession.swift @@ -14,7 +14,6 @@ struct ConnectionSession: Identifiable { /// The connection used to create the driver (may differ from `connection` for SSH tunneled connections) var effectiveConnection: DatabaseConnection? var driver: DatabaseDriver? - var metadataDriver: DatabaseDriver? var status: ConnectionStatus = .disconnected var lastError: String? @@ -69,7 +68,7 @@ struct ConnectionSession: Identifiable { } /// Compares fields used by ContentView's body to avoid unnecessary SwiftUI re-renders. - /// Excludes: driver/metadataDriver (protocol, non-comparable), + /// Excludes: driver (protocol, non-comparable), /// lastActiveAt (volatile), lastError, effectiveConnection. func isContentViewEquivalent(to other: ConnectionSession) -> Bool { id == other.id diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift index 3486560c..f8d248cd 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift @@ -308,11 +308,6 @@ extension MainContentCoordinator { if connection.type == .mysql || connection.type == .mariadb || connection.type == .clickhouse { _ = try await driver.execute(query: "USE `\(database)`") - // Also switch metadata driver's database - if let metaDriver = DatabaseManager.shared.metadataDriver(for: connectionId) { - _ = try? await metaDriver.execute(query: "USE `\(database)`") - } - // Update session with new database DatabaseManager.shared.updateSession(connectionId) { session in session.currentDatabase = database @@ -354,10 +349,6 @@ extension MainContentCoordinator { guard let schemaDriver = driver as? SchemaSwitchable else { return } try await schemaDriver.switchSchema(to: database) - if let schemaMeta = DatabaseManager.shared.metadataDriver(for: connectionId) as? SchemaSwitchable { - try? await schemaMeta.switchSchema(to: database) - } - // Update session DatabaseManager.shared.updateSession(connectionId) { session in session.currentSchema = database @@ -383,10 +374,6 @@ extension MainContentCoordinator { guard let schemaDriver = driver as? SchemaSwitchable else { return } try await schemaDriver.switchSchema(to: database) - if let schemaMeta = DatabaseManager.shared.metadataDriver(for: connectionId) as? SchemaSwitchable { - try? await schemaMeta.switchSchema(to: database) - } - DatabaseManager.shared.updateSession(connectionId) { session in session.currentSchema = database session.tables = [] @@ -406,10 +393,6 @@ extension MainContentCoordinator { try await adapter.switchDatabase(to: database) } - if let metaAdapter = DatabaseManager.shared.metadataDriver(for: connectionId) as? PluginDriverAdapter { - try? await metaAdapter.switchDatabase(to: database) - } - DatabaseManager.shared.updateSession(connectionId) { session in session.currentDatabase = database session.currentSchema = "dbo" @@ -432,11 +415,6 @@ extension MainContentCoordinator { try await adapter.switchDatabase(to: database) } - // Also update metadata driver if present - if let metaAdapter = DatabaseManager.shared.metadataDriver(for: connectionId) as? PluginDriverAdapter { - try? await metaAdapter.switchDatabase(to: database) - } - DatabaseManager.shared.updateSession(connectionId) { session in session.currentDatabase = database session.tables = [] @@ -461,10 +439,6 @@ extension MainContentCoordinator { try await adapter.switchDatabase(to: String(dbIndex)) } - if let metaAdapter = DatabaseManager.shared.metadataDriver(for: connectionId) as? PluginDriverAdapter { - try? await metaAdapter.switchDatabase(to: String(dbIndex)) - } - DatabaseManager.shared.updateSession(connectionId) { session in session.currentDatabase = database session.tables = [] @@ -499,10 +473,6 @@ extension MainContentCoordinator { guard let schemaDriver = driver as? SchemaSwitchable else { return } try await schemaDriver.switchSchema(to: schema) - if let schemaMeta = DatabaseManager.shared.metadataDriver(for: connectionId) as? SchemaSwitchable { - try? await schemaMeta.switchSchema(to: schema) - } - DatabaseManager.shared.updateSession(connectionId) { session in session.currentSchema = schema session.tables = [] @@ -544,9 +514,6 @@ extension MainContentCoordinator { navigationLogger.error("Failed to SELECT Redis db\(dbIndex): \(error.localizedDescription, privacy: .public)") return } - if let metaAdapter = DatabaseManager.shared.metadataDriver(for: connId) as? PluginDriverAdapter { - try? await metaAdapter.switchDatabase(to: String(dbIndex)) - } DatabaseManager.shared.updateSession(connId) { session in session.currentDatabase = database } diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 1c55bca9..73a9fe0a 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -545,14 +545,18 @@ final class MainContentCoordinator { let cached = isMetadataCached(tabId: tabId, tableName: tableName) needsMetadataFetch = !cached - // If metadata is NOT cached, lazily create (or reuse) the dedicated - // metadata driver so Phase 2 queries run in parallel with the main query. - if needsMetadataFetch, let metaDriver = await DatabaseManager.shared.ensureMetadataDriver(for: connectionId) { + // Metadata queries run on the main driver. They serialize behind any + // in-flight query at the C-level DispatchQueue and execute immediately after. + if needsMetadataFetch { + let connId = connectionId parallelSchemaTask = Task { - async let cols = metaDriver.fetchColumns(table: tableName) - async let fks = metaDriver.fetchForeignKeys(table: tableName) + guard let driver = DatabaseManager.shared.driver(for: connId) else { + throw DatabaseError.notConnected + } + async let cols = driver.fetchColumns(table: tableName) + async let fks = driver.fetchForeignKeys(table: tableName) let result = try await (columnInfo: cols, fkInfo: fks) - let approxCount = try? await metaDriver.fetchApproximateRowCount(table: tableName) + let approxCount = try? await driver.fetchApproximateRowCount(table: tableName) return (columnInfo: result.columnInfo, fkInfo: result.fkInfo, approximateRowCount: approxCount) } } @@ -1387,8 +1391,7 @@ private extension MainContentCoordinator { } // Phase 2b: Fetch enum/set values - let enumDriver = DatabaseManager.shared.metadataDriver(for: connectionId) - ?? DatabaseManager.shared.driver(for: connectionId) + let enumDriver = DatabaseManager.shared.driver(for: connectionId) guard let enumDriver else { return } Task { [weak self] in diff --git a/TableProTests/Core/Database/DatabaseManagerTests.swift b/TableProTests/Core/Database/DatabaseManagerTests.swift index ecb48c0c..049d492b 100644 --- a/TableProTests/Core/Database/DatabaseManagerTests.swift +++ b/TableProTests/Core/Database/DatabaseManagerTests.swift @@ -24,12 +24,6 @@ struct DatabaseManagerSessionTests { #expect(DatabaseManager.shared.session(for: unknownId) == nil) } - @Test("metadataDriver(for:) returns nil for unknown connection ID") - func metadataDriverReturnsNilForUnknown() { - let unknownId = UUID() - #expect(DatabaseManager.shared.metadataDriver(for: unknownId) == nil) - } - @Test("activeSessions is accessible and starts empty for unknown IDs") func activeSessionsAccessible() { let unknownId = UUID() diff --git a/docs/development/MEMORY-AUDIT.md b/docs/development/MEMORY-AUDIT.md index f1a3ab2b..309ad210 100644 --- a/docs/development/MEMORY-AUDIT.md +++ b/docs/development/MEMORY-AUDIT.md @@ -28,9 +28,9 @@ Every connection creates three separate C-level driver instances: Each carries its own C library state (libmariadb internal buffers, TLS context, TCP socket). For MySQL this is ~30–50 MB per connection × 3 = the bulk of the +110 MB jump on connect. **Fix options:** -- [ ] Multiplex metadata queries on the main driver when idle (eliminates 1 driver) +- [x] Multiplex metadata queries on the main driver when idle (eliminates 1 driver) — **Done**: metadata driver eliminated entirely; all queries multiplex on main driver via C-level DispatchQueue serialization - [x] Use a lightweight ping mechanism (e.g., TCP keepalive or `mysql_ping` on main driver with a mutex) instead of a dedicated ping driver — **Done**: removed dedicated `pingDrivers` dict, health checks now use main driver -- [x] Lazy-create the metadata driver only when needed, release after use — **Done**: `ensureMetadataDriver(for:)` creates on first Phase 2 request; deduplicates in-flight creation tasks +- [x] ~~Lazy-create the metadata driver only when needed~~ → Superseded: metadata driver eliminated entirely (see multiplex fix above) --- @@ -184,7 +184,7 @@ This is inherent to the native-tab architecture and not easily reducible without | 2 | `InMemoryRowProvider` references `RowBuffer` instead of copying | -3–10 MB per tab | Medium | **Done** | | 9 | Remove `RowBuffer.sourceQuery`, use `tab.query` | -0–500 KB per tab | Low | **Done** | | 4 | Lazy plugin loading | -20–30 MB at launch | Low-Medium | **Done** | -| 1 | Eliminate dedicated ping driver + lazy metadata driver | -60–100 MB per connection | Medium | **Done** | +| 1 | Eliminate dedicated ping driver + metadata driver | -60–100 MB per connection | Medium | **Done** | --- From a30aa1e63bdcaba0cc86e4a871ed35747e32a45e Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 9 Mar 2026 20:13:28 +0700 Subject: [PATCH 4/5] perf: lazy AI view model, deduplicate connections, fix tab persistence, consolidate event monitors --- CHANGELOG.md | 5 +- TablePro/ContentView.swift | 20 --- TablePro/Models/UI/RightPanelState.swift | 10 +- TablePro/Views/Editor/EditorEventRouter.swift | 158 ++++++++++++++++++ .../Views/Editor/SQLEditorCoordinator.swift | 115 ++----------- .../Views/Main/MainContentCoordinator.swift | 45 ++++- 6 files changed, 229 insertions(+), 124 deletions(-) create mode 100644 TablePro/Views/Editor/EditorEventRouter.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 9758e116..d3b44291 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Reduce memory: remove duplicate sourceQuery string from RowBuffer - Reduce memory: InMemoryRowProvider references RowBuffer directly instead of copying rows (~3-10 MB per tab) - Reduce memory: eliminate metadata driver entirely, multiplex all queries on main driver (~30-50 MB per connection) -- Reduce CPU: filter NSWindow.didUpdateNotification by editor's own window (N editors → 1 handler per update cycle) +- Reduce memory: lazy AIChatViewModel initialization (deferred until AI panel is first opened) +- Reduce memory: remove duplicate connections array from ContentView (use ConnectionStorage.shared directly) +- Reduce CPU: consolidate per-editor NSEvent monitors into shared EditorEventRouter singleton (O(n) → O(1) per event) +- Fix tab persistence: aggregate tabs from all windows at quit time instead of last-write-wins per-coordinator save - Split DatabaseManager.sessionVersion into fine-grained connectionListVersion and connectionStatusVersion to reduce cascade re-renders - Extract AppState property reads into local lets in view bodies for explicit granular observation tracking - Reorganized project directory structure: Services, Utilities, Models split into domain-specific subdirectories diff --git a/TablePro/ContentView.swift b/TablePro/ContentView.swift index 09966355..6a7a94f9 100644 --- a/TablePro/ContentView.swift +++ b/TablePro/ContentView.swift @@ -17,14 +17,12 @@ struct ContentView: View { let payload: EditorTabPayload? @State private var currentSession: ConnectionSession? - @State private var connections: [DatabaseConnection] = [] @State private var columnVisibility: NavigationSplitViewVisibility = .all @State private var showNewConnectionSheet = false @State private var showEditConnectionSheet = false @State private var connectionToEdit: DatabaseConnection? @State private var connectionToDelete: DatabaseConnection? @State private var showDeleteConfirmation = false - @State private var hasLoaded = false @State private var rightPanelState: RightPanelState? @State private var sessionState: SessionStateFactory.SessionState? @State private var inspectorContext = InspectorContext.empty @@ -68,9 +66,6 @@ struct ContentView: View { } message: { connection in Text("Are you sure you want to delete \"\(connection.name)\"?") } - .onAppear { - loadConnections() - } .onReceive(NotificationCenter.default.publisher(for: .newConnection)) { _ in openWindow(id: "connection-form", value: nil as UUID?) } @@ -364,19 +359,6 @@ struct ContentView: View { // MARK: - Persistence - private func loadConnections() { - guard !hasLoaded else { return } - - let saved = storage.loadConnections() - if saved.isEmpty { - connections = DatabaseConnection.sampleConnections - storage.saveConnections(connections) - } else { - connections = saved - } - hasLoaded = true - } - private func deleteConnection(_ connection: DatabaseConnection) { if DatabaseManager.shared.activeSessions[connection.id] != nil { Task { @@ -384,9 +366,7 @@ struct ContentView: View { } } - connections.removeAll { $0.id == connection.id } storage.deleteConnection(connection) - storage.saveConnections(connections) } private func showAllTablesMetadata() { diff --git a/TablePro/Models/UI/RightPanelState.swift b/TablePro/Models/UI/RightPanelState.swift index 1784ad08..4bcaa688 100644 --- a/TablePro/Models/UI/RightPanelState.swift +++ b/TablePro/Models/UI/RightPanelState.swift @@ -33,7 +33,13 @@ import os // Owned objects — lifted from MainContentView @StateObject let editState = MultiRowEditState() - let aiViewModel = AIChatViewModel() + private var _aiViewModel: AIChatViewModel? + var aiViewModel: AIChatViewModel { + if _aiViewModel == nil { + _aiViewModel = AIChatViewModel() + } + return _aiViewModel! + } init() { self.isPresented = UserDefaults.standard.bool(forKey: Self.isPresentedKey) @@ -51,7 +57,7 @@ import os guard !_didTeardown.withLock({ $0 }) else { return } _didTeardown.withLock { $0 = true } onSave = nil - aiViewModel.clearSessionData() + _aiViewModel?.clearSessionData() editState.releaseData() NotificationCenter.default.removeObserver(self) } diff --git a/TablePro/Views/Editor/EditorEventRouter.swift b/TablePro/Views/Editor/EditorEventRouter.swift new file mode 100644 index 00000000..f1456c34 --- /dev/null +++ b/TablePro/Views/Editor/EditorEventRouter.swift @@ -0,0 +1,158 @@ +// +// EditorEventRouter.swift +// TablePro +// +// Shared event router that installs one set of process-global monitors +// and dispatches to the correct editor by window, replacing per-editor monitors. +// + +import AppKit +import CodeEditTextView + +@MainActor +internal final class EditorEventRouter { + internal static let shared = EditorEventRouter() + + private struct EditorRef { + weak var coordinator: SQLEditorCoordinator? + weak var textView: TextView? + } + + private var editors: [ObjectIdentifier: EditorRef] = [:] + private var rightClickMonitor: Any? + private var clipboardMonitor: Any? + private var windowUpdateObserver: NSObjectProtocol? + + private init() {} + + // MARK: - Registration + + internal func register(_ coordinator: SQLEditorCoordinator, textView: TextView) { + let key = ObjectIdentifier(coordinator) + editors[key] = EditorRef(coordinator: coordinator, textView: textView) + + if rightClickMonitor == nil { + installMonitors() + } + } + + internal func unregister(_ coordinator: SQLEditorCoordinator) { + editors.removeValue(forKey: ObjectIdentifier(coordinator)) + purgeStaleEntries() + + if editors.isEmpty { + removeMonitors() + } + } + + // MARK: - Lookup + + private func editor(for window: NSWindow?) -> (SQLEditorCoordinator, TextView)? { + guard let window else { return nil } + for ref in editors.values { + guard let coordinator = ref.coordinator, let textView = ref.textView, + textView.window === window else { continue } + return (coordinator, textView) + } + return nil + } + + private func purgeStaleEntries() { + editors = editors.filter { $0.value.coordinator != nil && $0.value.textView != nil } + } + + // MARK: - Monitor Installation + + private func installMonitors() { + rightClickMonitor = NSEvent.addLocalMonitorForEvents(matching: .rightMouseDown) { [weak self] event in + guard let self else { return event } + return MainActor.assumeIsolated { + self.handleRightClick(event) + } + } + + clipboardMonitor = NSEvent.addLocalMonitorForEvents(matching: .keyDown) { [weak self] event in + guard let self else { return event } + return MainActor.assumeIsolated { + self.handleKeyDown(event) + } + } + + windowUpdateObserver = NotificationCenter.default.addObserver( + forName: NSWindow.didUpdateNotification, + object: nil, + queue: .main + ) { [weak self] notification in + guard let self else { return } + MainActor.assumeIsolated { + self.handleWindowUpdate(notification) + } + } + } + + private func removeMonitors() { + if let monitor = rightClickMonitor { + NSEvent.removeMonitor(monitor) + rightClickMonitor = nil + } + if let monitor = clipboardMonitor { + NSEvent.removeMonitor(monitor) + clipboardMonitor = nil + } + if let observer = windowUpdateObserver { + NotificationCenter.default.removeObserver(observer) + windowUpdateObserver = nil + } + } + + // MARK: - Event Handlers + + private func handleRightClick(_ event: NSEvent) -> NSEvent? { + guard let (coordinator, textView) = editor(for: event.window) else { return event } + + let locationInView = textView.convert(event.locationInWindow, from: nil) + guard textView.bounds.contains(locationInView) else { return event } + + coordinator.showContextMenu(for: event, in: textView) + return nil + } + + private func handleKeyDown(_ event: NSEvent) -> NSEvent? { + guard let (_, textView) = editor(for: event.window), + textView.window?.firstResponder === textView else { + return event + } + + let mods = event.modifierFlags.intersection(.deviceIndependentFlagsMask) + guard mods.contains(.command), + !mods.contains(.shift), !mods.contains(.option), !mods.contains(.control) else { + return event + } + + let range = textView.selectedRange() + guard range.length > 0 else { return event } + let text = (textView.string as NSString).substring(with: range) + + switch event.keyCode { + case 8: // Cmd+C + NSPasteboard.general.clearContents() + NSPasteboard.general.setString(text, forType: .string) + return nil + case 7: // Cmd+X + NSPasteboard.general.clearContents() + NSPasteboard.general.setString(text, forType: .string) + textView.replaceCharacters(in: range, with: "") + return nil + default: + break + } + + return event + } + + private func handleWindowUpdate(_ notification: Notification) { + guard let window = notification.object as? NSWindow, + let (coordinator, _) = editor(for: window) else { return } + coordinator.checkFirstResponderChange() + } +} diff --git a/TablePro/Views/Editor/SQLEditorCoordinator.swift b/TablePro/Views/Editor/SQLEditorCoordinator.swift index 9b188d1f..5cf6be4f 100644 --- a/TablePro/Views/Editor/SQLEditorCoordinator.swift +++ b/TablePro/Views/Editor/SQLEditorCoordinator.swift @@ -24,14 +24,11 @@ final class SQLEditorCoordinator: TextViewCoordinator { /// Shared schema provider for inline AI suggestions (avoids duplicate schema fetches) @ObservationIgnored var schemaProvider: SQLSchemaProvider? @ObservationIgnored private var contextMenu: AIEditorContextMenu? - @ObservationIgnored private var rightClickMonitor: Any? @ObservationIgnored private var inlineSuggestionManager: InlineSuggestionManager? @ObservationIgnored private var editorSettingsObserver: NSObjectProtocol? /// Debounce work item for frame-change notification to avoid /// triggering syntax highlight viewport recalculation on every keystroke. @ObservationIgnored private var frameChangeWorkItem: DispatchWorkItem? - @ObservationIgnored private var clipboardMonitor: Any? - @ObservationIgnored private var firstResponderObserver: NSObjectProtocol? @ObservationIgnored private var wasEditorFocused = false @ObservationIgnored private var didDestroy = false @@ -58,40 +55,19 @@ final class SQLEditorCoordinator: TextViewCoordinator { } deinit { - if let monitor = rightClickMonitor { - NSEvent.removeMonitor(monitor) - } if let observer = editorSettingsObserver { NotificationCenter.default.removeObserver(observer) } - if let observer = firstResponderObserver { - NotificationCenter.default.removeObserver(observer) - } frameChangeWorkItem?.cancel() - if let monitor = clipboardMonitor { - NSEvent.removeMonitor(monitor) - } } private func cleanupMonitors() { - if let monitor = rightClickMonitor { - NSEvent.removeMonitor(monitor) - rightClickMonitor = nil - } if let observer = editorSettingsObserver { NotificationCenter.default.removeObserver(observer) editorSettingsObserver = nil } - if let observer = firstResponderObserver { - NotificationCenter.default.removeObserver(observer) - firstResponderObserver = nil - } frameChangeWorkItem?.cancel() frameChangeWorkItem = nil - if let monitor = clipboardMonitor { - NSEvent.removeMonitor(monitor) - clipboardMonitor = nil - } } // MARK: - TextViewCoordinator @@ -102,14 +78,15 @@ final class SQLEditorCoordinator: TextViewCoordinator { // Deferred to next run loop because prepareCoordinator runs during // TextViewController.init, before the view hierarchy is fully loaded. DispatchQueue.main.async { [weak self] in - guard self != nil else { return } - self?.fixFindPanelHitTesting(controller: controller) - self?.applyHorizontalScrollFix(controller: controller) - self?.installAIContextMenu(controller: controller) - self?.installInlineSuggestionManager(controller: controller) - self?.installVimModeIfEnabled(controller: controller) - self?.installClipboardMonitor(controller: controller) - self?.installFirstResponderObserver() + guard let self else { return } + self.fixFindPanelHitTesting(controller: controller) + self.applyHorizontalScrollFix(controller: controller) + self.installAIContextMenu(controller: controller) + self.installInlineSuggestionManager(controller: controller) + self.installVimModeIfEnabled(controller: controller) + if let textView = controller.textView { + EditorEventRouter.shared.register(self, textView: textView) + } } } @@ -166,6 +143,7 @@ final class SQLEditorCoordinator: TextViewCoordinator { inlineSuggestionManager?.uninstall() inlineSuggestionManager = nil + EditorEventRouter.shared.unregister(self) cleanupMonitors() } @@ -185,20 +163,12 @@ final class SQLEditorCoordinator: TextViewCoordinator { return (textView.string as NSString).substring(with: range) } contextMenu = menu + } - // CodeEditTextView's TextView overrides menu(for:) with a hardcoded - // Cut/Copy/Paste menu, ignoring the stored `menu` property. Intercept - // right-clicks via a local event monitor and show our custom menu instead. - rightClickMonitor = NSEvent.addLocalMonitorForEvents(matching: .rightMouseDown) { [weak textView, weak menu] event in - guard let textView, let menu, - event.window === textView.window else { return event } - - let locationInView = textView.convert(event.locationInWindow, from: nil) - guard textView.bounds.contains(locationInView) else { return event } - - NSMenu.popUpContextMenu(menu, with: event, for: textView) - return nil // Consume event to prevent default menu - } + /// Called by EditorEventRouter when a right-click is detected in this editor's text view. + func showContextMenu(for event: NSEvent, in textView: TextView) { + guard let menu = contextMenu else { return } + NSMenu.popUpContextMenu(menu, with: event, for: textView) } // MARK: - Inline Suggestion Manager @@ -270,20 +240,7 @@ final class SQLEditorCoordinator: TextViewCoordinator { // MARK: - First Responder Tracking - private func installFirstResponderObserver() { - firstResponderObserver = NotificationCenter.default.addObserver( - forName: NSWindow.didUpdateNotification, - object: nil, - queue: .main - ) { [weak self] notification in - MainActor.assumeIsolated { - guard let self, notification.object as? NSWindow == self.controller?.textView?.window else { return } - self.checkFirstResponderChange() - } - } - } - - private func checkFirstResponderChange() { + func checkFirstResponderChange() { let focused = isEditorFirstResponder guard focused != wasEditorFocused else { return } wasEditorFocused = focused @@ -297,46 +254,6 @@ final class SQLEditorCoordinator: TextViewCoordinator { } } - // MARK: - Clipboard Monitor - - private func installClipboardMonitor(controller: TextViewController) { - guard let textView = controller.textView else { return } - - clipboardMonitor = NSEvent.addLocalMonitorForEvents(matching: .keyDown) { [weak textView] event in - guard let textView, - event.window === textView.window, - textView.window?.firstResponder === textView else { - return event - } - - let mods = event.modifierFlags.intersection(.deviceIndependentFlagsMask) - guard mods.contains(.command), - !mods.contains(.shift), !mods.contains(.option), !mods.contains(.control) else { - return event - } - - let range = textView.selectedRange() - guard range.length > 0 else { return event } - let text = (textView.string as NSString).substring(with: range) - - switch event.keyCode { - case 8: // Cmd+C - NSPasteboard.general.clearContents() - NSPasteboard.general.setString(text, forType: .string) - return nil - case 7: // Cmd+X - NSPasteboard.general.clearContents() - NSPasteboard.general.setString(text, forType: .string) - textView.replaceCharacters(in: range, with: "") - return nil - default: - break - } - - return event - } - } - // MARK: - Horizontal Scrolling Fix /// Enable horizontal scrolling when word wrap is off. diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 73a9fe0a..ed1f09d1 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -117,6 +117,40 @@ final class MainContentCoordinator { set { _isAppTerminating.withLock { $0 = newValue } } } + /// Registry of active coordinators for aggregated quit-time persistence. + /// Keyed by ObjectIdentifier of each coordinator instance. + private static var activeCoordinators: [ObjectIdentifier: MainContentCoordinator] = [:] + + /// Register this coordinator so quit-time persistence can aggregate tabs. + private func registerForPersistence() { + Self.activeCoordinators[ObjectIdentifier(self)] = self + } + + /// Unregister this coordinator from quit-time aggregation. + private func unregisterFromPersistence() { + Self.activeCoordinators.removeValue(forKey: ObjectIdentifier(self)) + } + + /// Collect all tabs from all active coordinators for a given connectionId. + private static func aggregatedTabs(for connectionId: UUID) -> [QueryTab] { + activeCoordinators.values + .filter { $0.connectionId == connectionId } + .flatMap { $0.tabManager.tabs } + } + + /// Get selected tab ID from any coordinator for a given connectionId. + private static func aggregatedSelectedTabId(for connectionId: UUID) -> UUID? { + activeCoordinators.values + .first { $0.connectionId == connectionId && $0.tabManager.selectedTabId != nil }? + .tabManager.selectedTabId + } + + /// Check if this coordinator is the first registered for its connection. + private func isFirstCoordinatorForConnection() -> Bool { + Self.activeCoordinators.values + .first { $0.connectionId == self.connectionId } === self + } + private static let registerTerminationObserver: Void = { NotificationCenter.default.addObserver( forName: NSApplication.willTerminateNotification, @@ -182,13 +216,19 @@ final class MainContentCoordinator { ) { [weak self] _ in MainActor.assumeIsolated { guard let self, !self.isTearingDown else { return } + // Only the first coordinator for this connection saves, + // aggregating tabs from all windows to fix last-write-wins bug + guard self.isFirstCoordinatorForConnection() else { return } + let allTabs = Self.aggregatedTabs(for: self.connectionId) + let selectedId = Self.aggregatedSelectedTabId(for: self.connectionId) self.persistence.saveNowSync( - tabs: self.tabManager.tabs, - selectedTabId: self.tabManager.selectedTabId + tabs: allTabs, + selectedTabId: selectedId ) } } + registerForPersistence() _ = Self.registerTerminationObserver } @@ -208,6 +248,7 @@ final class MainContentCoordinator { /// synchronously on MainActor so we don't depend on deinit + Task scheduling. func teardown() { _didTeardown.withLock { $0 = true } + unregisterFromPersistence() for observer in urlFilterObservers { NotificationCenter.default.removeObserver(observer) } From 2fe403ac25f3e3fadcb4fa86a44d58d23c008a34 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 9 Mar 2026 20:44:13 +0700 Subject: [PATCH 5/5] perf: eliminate rowCache from InMemoryRowProvider, add zero-allocation direct access --- TablePro/Models/Query/RowProvider.swift | 58 +++---- .../Views/Main/MainContentCoordinator.swift | 3 +- .../Views/Results/DataGridCellFactory.swift | 3 +- .../Results/DataGridView+RowActions.swift | 18 +- .../Results/DataGridView+TypePicker.swift | 8 +- .../Extensions/DataGridView+Click.swift | 7 +- .../Extensions/DataGridView+Columns.swift | 5 +- .../Extensions/DataGridView+Editing.swift | 14 +- .../Extensions/DataGridView+Popovers.swift | 37 ++-- .../Views/Results/KeyHandlingTableView.swift | 2 +- TableProTests/Models/RowProviderTests.swift | 164 +++++++----------- .../Views/Results/RowProviderSyncTests.swift | 74 ++++---- docs/development/MEMORY-AUDIT.md | 6 +- 13 files changed, 162 insertions(+), 237 deletions(-) diff --git a/TablePro/Models/Query/RowProvider.swift b/TablePro/Models/Query/RowProvider.swift index c004ecf3..8966da3f 100644 --- a/TablePro/Models/Query/RowProvider.swift +++ b/TablePro/Models/Query/RowProvider.swift @@ -59,20 +59,16 @@ final class TableRowData { // MARK: - In-Memory Row Provider /// Row provider that keeps all data in memory (for existing QueryResultRow data). -/// Uses lazy TableRowData creation to avoid O(n) heap allocations on init. -/// Cache is bounded to `maxCacheSize` entries; proximity-based eviction keeps -/// the half closest to the current access point when the limit is exceeded. -/// /// References `RowBuffer` directly to avoid duplicating row data. /// An optional `sortIndices` array maps display indices to source-row indices, /// so sorted views don't need a reordered copy of the rows. +/// +/// Direct-access methods `value(atRow:column:)` and `rowValues(at:)` avoid +/// heap allocations by reading straight from the source `QueryResultRow`. final class InMemoryRowProvider: RowProvider { - private static let maxCacheSize = 5_000 - private let rowBuffer: RowBuffer private var sortIndices: [Int]? private var appendedRows: [QueryResultRow] = [] - private var rowCache: [Int: TableRowData] = [:] private(set) var columns: [String] private(set) var columnDefaults: [String: String?] private(set) var columnTypes: [ColumnType] @@ -139,7 +135,7 @@ final class InMemoryRowProvider: RowProvider { var result: [TableRowData] = [] result.reserveCapacity(endIndex - offset) for i in offset.. TableRowData? { guard index >= 0 && index < totalRowCount else { return nil } - return materializeRow(at: index) + return TableRowData(index: index, values: sourceRow(at: index).values) + } + + /// O(1) cell value access — no heap allocation. + func value(atRow rowIndex: Int, column columnIndex: Int) -> String? { + guard rowIndex >= 0 && rowIndex < totalRowCount else { return nil } + let src = sourceRow(at: rowIndex) + guard columnIndex >= 0 && columnIndex < src.values.count else { return nil } + return src.values[columnIndex] + } + + /// Returns the source values array for a display row. No copy until caller stores it. + func rowValues(at rowIndex: Int) -> [String?]? { + guard rowIndex >= 0 && rowIndex < totalRowCount else { return nil } + return sourceRow(at: rowIndex).values } /// Update rows by replacing the buffer contents and clearing appended rows @@ -177,7 +185,6 @@ final class InMemoryRowProvider: RowProvider { rowBuffer.rows = newRows appendedRows.removeAll() sortIndices = nil - rowCache.removeAll() } /// Append a new row with given values @@ -185,8 +192,6 @@ final class InMemoryRowProvider: RowProvider { func appendRow(values: [String?]) -> Int { let newIndex = totalRowCount appendedRows.append(QueryResultRow(id: newIndex, values: values)) - let rowData = TableRowData(index: newIndex, values: values) - rowCache[newIndex] = rowData return newIndex } @@ -213,8 +218,6 @@ final class InMemoryRowProvider: RowProvider { rowBuffer.rows.remove(at: index) } } - // Clear entire cache since indices shift - rowCache.removeAll() } /// Remove multiple rows at indices (used when discarding new rows) @@ -252,27 +255,6 @@ final class InMemoryRowProvider: RowProvider { return rowBuffer.rows[displayIndex] } - private func materializeRow(at index: Int) -> TableRowData { - if let cached = rowCache[index] { - return cached - } - // Evict distant entries when cache exceeds the threshold. - // Keeps entries closest to the current index for locality. - if rowCache.count >= Self.maxCacheSize { - evictCacheIfNeeded(nearIndex: index) - } - let rowData = TableRowData(index: index, values: sourceRow(at: index).values) - rowCache[index] = rowData - return rowData - } - - /// Evict entries furthest from the current access point. - /// Keeps the half closest to `nearIndex` and discards the rest. - private func evictCacheIfNeeded(nearIndex: Int) { - guard rowCache.count > Self.maxCacheSize / 2 else { return } - let halfSize = Self.maxCacheSize / 2 - rowCache = rowCache.filter { abs($0.key - nearIndex) <= halfSize } - } } // MARK: - Database Row Provider (for virtualized access via driver) diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index ed1f09d1..07db829e 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -1367,7 +1367,8 @@ private extension MainContentCoordinator { } else if conn.type == .redis { resolvedPK = "Key" } else { - resolvedPK = nil + // Preserve existing PK when metadata is cached and not re-fetched + resolvedPK = tabManager.tabs[idx].primaryKeyColumn } if let pk = resolvedPK { diff --git a/TablePro/Views/Results/DataGridCellFactory.swift b/TablePro/Views/Results/DataGridCellFactory.swift index c55b2482..cee0ad0b 100644 --- a/TablePro/Views/Results/DataGridCellFactory.swift +++ b/TablePro/Views/Results/DataGridCellFactory.swift @@ -442,8 +442,7 @@ final class DataGridCellFactory { let charWidth = Self.monoCharWidth for i in stride(from: 0, to: totalRows, by: step) { - guard let row = rowProvider.row(at: i), - let value = row.value(at: columnIndex) else { continue } + guard let value = rowProvider.value(atRow: i, column: columnIndex) else { continue } let charCount = min((value as NSString).length, Self.maxMeasureChars) let cellWidth = CGFloat(charCount) * charWidth + 16 diff --git a/TablePro/Views/Results/DataGridView+RowActions.swift b/TablePro/Views/Results/DataGridView+RowActions.swift index 2cbb45ac..9795d596 100644 --- a/TablePro/Views/Results/DataGridView+RowActions.swift +++ b/TablePro/Views/Results/DataGridView+RowActions.swift @@ -36,8 +36,8 @@ extension TableViewCoordinator { var lines: [String] = [] for index in sortedIndices { - guard let rowData = rowProvider.row(at: index) else { continue } - let line = rowData.values.map { $0 ?? "NULL" }.joined(separator: "\t") + guard let values = rowProvider.rowValues(at: index) else { continue } + let line = values.map { $0 ?? "NULL" }.joined(separator: "\t") lines.append(line) } @@ -53,8 +53,8 @@ extension TableViewCoordinator { lines.append(rowProvider.columns.joined(separator: "\t")) for index in sortedIndices { - guard let rowData = rowProvider.row(at: index) else { continue } - let line = rowData.values.map { $0 ?? "NULL" }.joined(separator: "\t") + guard let values = rowProvider.rowValues(at: index) else { continue } + let line = values.map { $0 ?? "NULL" }.joined(separator: "\t") lines.append(line) } @@ -76,8 +76,8 @@ extension TableViewCoordinator { guard columnIndex >= 0 && columnIndex < rowProvider.columns.count else { return } let columnName = rowProvider.columns[columnIndex] - let oldValue = rowProvider.row(at: rowIndex)?.value(at: columnIndex) - let originalRow = rowProvider.row(at: rowIndex)?.values ?? [] + let oldValue = rowProvider.value(atRow: rowIndex, column: columnIndex) + let originalRow = rowProvider.rowValues(at: rowIndex) ?? [] changeManager.recordCellChange( rowIndex: rowIndex, @@ -99,9 +99,7 @@ extension TableViewCoordinator { func copyCellValue(at rowIndex: Int, columnIndex: Int) { guard columnIndex >= 0 && columnIndex < rowProvider.columns.count else { return } - if let rowData = rowProvider.row(at: rowIndex) { - let value = rowData.value(at: columnIndex) ?? "NULL" - ClipboardService.shared.writeText(value) - } + let value = rowProvider.value(atRow: rowIndex, column: columnIndex) ?? "NULL" + ClipboardService.shared.writeText(value) } } diff --git a/TablePro/Views/Results/DataGridView+TypePicker.swift b/TablePro/Views/Results/DataGridView+TypePicker.swift index d60737fd..15b97b88 100644 --- a/TablePro/Views/Results/DataGridView+TypePicker.swift +++ b/TablePro/Views/Results/DataGridView+TypePicker.swift @@ -15,10 +15,9 @@ extension TableViewCoordinator { column: Int, columnIndex: Int ) { - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil, - let rowData = rowProvider.row(at: row) else { return } + guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } - let currentValue = rowData.value(at: columnIndex) ?? "" + let currentValue = rowProvider.value(atRow: row, column: columnIndex) ?? "" let dbType = databaseType ?? .mysql let cellRect = tableView.rect(ofRow: row).intersection(tableView.rect(ofColumn: column)) @@ -31,8 +30,7 @@ extension TableViewCoordinator { currentValue: currentValue, onCommit: { newValue in guard let self else { return } - guard let rowData = self.rowProvider.row(at: row) else { return } - let oldValue = rowData.value(at: columnIndex) + let oldValue = self.rowProvider.value(atRow: row, column: columnIndex) guard oldValue != newValue else { return } self.rowProvider.updateValue(newValue, at: row, columnIndex: columnIndex) diff --git a/TablePro/Views/Results/Extensions/DataGridView+Click.swift b/TablePro/Views/Results/Extensions/DataGridView+Click.swift index 42540cfe..46b4facf 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Click.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Click.swift @@ -105,7 +105,7 @@ extension TableViewCoordinator { } // Multiline values use the overlay editor instead of inline field editor - if let value = rowProvider.row(at: row)?.value(at: columnIndex), + if let value = rowProvider.value(atRow: row, column: columnIndex), value.containsLineBreak { showOverlayEditor(tableView: sender, row: row, column: column, columnIndex: columnIndex, value: value) return @@ -123,13 +123,12 @@ extension TableViewCoordinator { let columnIndex = button.fkColumnIndex guard row >= 0 && row < cachedRowCount, - columnIndex >= 0 && columnIndex < rowProvider.columns.count, - let rowData = rowProvider.row(at: row) else { return } + columnIndex >= 0 && columnIndex < rowProvider.columns.count else { return } let columnName = rowProvider.columns[columnIndex] guard let fkInfo = rowProvider.columnForeignKeys[columnName] else { return } - let value = rowData.value(at: columnIndex) + let value = rowProvider.value(atRow: row, column: columnIndex) guard let value = value, !value.isEmpty else { return } onNavigateFK?(value, fkInfo) diff --git a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift index ab7d33aa..0ebd8f1a 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift @@ -24,12 +24,11 @@ extension TableViewCoordinator { guard columnId.hasPrefix("col_"), let columnIndex = Int(columnId.dropFirst(4)) else { return nil } guard row >= 0 && row < cachedRowCount, - columnIndex >= 0 && columnIndex < cachedColumnCount, - let rowData = rowProvider.row(at: row) else { + columnIndex >= 0 && columnIndex < cachedColumnCount else { return nil } - let value = rowData.value(at: columnIndex) + let value = rowProvider.value(atRow: row, column: columnIndex) let state = visualState(for: row) // Get column type for date formatting diff --git a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift index 0799b5e5..272cb5fd 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Editing.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Editing.swift @@ -44,7 +44,7 @@ extension TableViewCoordinator { } // Multiline values use overlay editor — block inline field editor - if let value = rowProvider.row(at: row)?.value(at: columnIndex), + if let value = rowProvider.value(atRow: row, column: columnIndex), value.containsLineBreak { let tableColumnIdx = tableView.column(withIdentifier: tableColumn.identifier) guard tableColumnIdx >= 0 else { return false } @@ -74,8 +74,7 @@ extension TableViewCoordinator { } func commitOverlayEdit(row: Int, columnIndex: Int, newValue: String) { - guard let rowData = rowProvider.row(at: row) else { return } - let oldValue = rowData.value(at: columnIndex) + let oldValue = rowProvider.value(atRow: row, column: columnIndex) guard oldValue != newValue else { return } let columnName = rowProvider.columns[columnIndex] @@ -85,7 +84,7 @@ extension TableViewCoordinator { columnName: columnName, oldValue: oldValue, newValue: newValue, - originalRow: rowData.values + originalRow: rowProvider.rowValues(at: row) ?? [] ) rowProvider.updateValue(newValue, at: row, columnIndex: columnIndex) @@ -126,7 +125,7 @@ extension TableViewCoordinator { // Check if next cell is also multiline → open overlay there let nextColumnIndex = nextColumn - 1 if nextColumnIndex >= 0, nextColumnIndex < rowProvider.columns.count, - let value = rowProvider.row(at: nextRow)?.value(at: nextColumnIndex), + let value = rowProvider.value(atRow: nextRow, column: nextColumnIndex), value.containsLineBreak { showOverlayEditor(tableView: tableView, row: nextRow, column: nextColumn, columnIndex: nextColumnIndex, value: value) } else { @@ -145,8 +144,7 @@ extension TableViewCoordinator { let columnIndex = column - 1 let newValue: String? = textField.stringValue - guard let rowData = rowProvider.row(at: row) else { return true } - let oldValue = rowData.value(at: columnIndex) + let oldValue = rowProvider.value(atRow: row, column: columnIndex) guard oldValue != newValue else { return true } @@ -157,7 +155,7 @@ extension TableViewCoordinator { columnName: columnName, oldValue: oldValue, newValue: newValue, - originalRow: rowData.values + originalRow: rowProvider.rowValues(at: row) ?? [] ) rowProvider.updateValue(newValue, at: row, columnIndex: columnIndex) diff --git a/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift b/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift index f8cd1222..9b29375f 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Popovers.swift @@ -10,8 +10,7 @@ import SwiftUI extension TableViewCoordinator { func showDatePickerPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { - guard let rowData = rowProvider.row(at: row) else { return } - let currentValue = rowData.value(at: columnIndex) + let currentValue = rowProvider.value(atRow: row, column: columnIndex) let columnType = rowProvider.columnTypes[columnIndex] guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } @@ -24,8 +23,7 @@ extension TableViewCoordinator { columnType: columnType ) { [weak self] newValue in guard let self = self else { return } - guard let rowData = self.rowProvider.row(at: row) else { return } - let oldValue = rowData.value(at: columnIndex) + let oldValue = self.rowProvider.value(atRow: row, column: columnIndex) guard oldValue != newValue else { return } let columnName = self.rowProvider.columns[columnIndex] @@ -35,7 +33,7 @@ extension TableViewCoordinator { columnName: columnName, oldValue: oldValue, newValue: newValue, - originalRow: rowData.values + originalRow: self.rowProvider.rowValues(at: row) ?? [] ) self.rowProvider.updateValue(newValue, at: row, columnIndex: columnIndex) @@ -46,8 +44,7 @@ extension TableViewCoordinator { } func showForeignKeyPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int, fkInfo: ForeignKeyInfo) { - guard let rowData = rowProvider.row(at: row) else { return } - let currentValue = rowData.value(at: columnIndex) + let currentValue = rowProvider.value(atRow: row, column: columnIndex) guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } guard let databaseType, let connectionId else { return } @@ -78,8 +75,7 @@ extension TableViewCoordinator { } func showJSONEditorPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { - guard let rowData = rowProvider.row(at: row) else { return } - let currentValue = rowData.value(at: columnIndex) + let currentValue = rowProvider.value(atRow: row, column: columnIndex) guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } @@ -106,12 +102,11 @@ extension TableViewCoordinator { } func showEnumPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil, - let rowData = rowProvider.row(at: row) else { return } + guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } let columnName = rowProvider.columns[columnIndex] guard let allowedValues = rowProvider.columnEnumValues[columnName] else { return } - let currentValue = rowData.value(at: columnIndex) + let currentValue = rowProvider.value(atRow: row, column: columnIndex) let isNullable = rowProvider.columnNullable[columnName] ?? true var values: [String] = [] @@ -138,12 +133,11 @@ extension TableViewCoordinator { } func showSetPopover(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil, - let rowData = rowProvider.row(at: row) else { return } + guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } let columnName = rowProvider.columns[columnIndex] guard let allowedValues = rowProvider.columnEnumValues[columnName] else { return } - let currentValue = rowData.value(at: columnIndex) + let currentValue = rowProvider.value(atRow: row, column: columnIndex) let currentSet: Set if let value = currentValue { @@ -173,10 +167,9 @@ extension TableViewCoordinator { } func showDropdownMenu(tableView: NSTableView, row: Int, column: Int, columnIndex: Int) { - guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil, - let rowData = rowProvider.row(at: row) else { return } + guard tableView.view(atColumn: column, row: row, makeIfNecessary: false) != nil else { return } - let currentValue = rowData.value(at: columnIndex) + let currentValue = rowProvider.value(atRow: row, column: columnIndex) pendingDropdownRow = row pendingDropdownColumn = columnIndex @@ -196,15 +189,13 @@ extension TableViewCoordinator { @objc func dropdownMenuItemSelected(_ sender: NSMenuItem) { let newValue = sender.title - guard let rowData = rowProvider.row(at: pendingDropdownRow) else { return } - let oldValue = rowData.value(at: pendingDropdownColumn) + let oldValue = rowProvider.value(atRow: pendingDropdownRow, column: pendingDropdownColumn) guard oldValue != newValue else { return } onCellEdit?(pendingDropdownRow, pendingDropdownColumn, newValue) } func commitPopoverEdit(tableView: NSTableView, row: Int, column: Int, columnIndex: Int, newValue: String?) { - guard let rowData = rowProvider.row(at: row) else { return } - let oldValue = rowData.value(at: columnIndex) + let oldValue = rowProvider.value(atRow: row, column: columnIndex) guard oldValue != newValue else { return } let columnName = rowProvider.columns[columnIndex] @@ -214,7 +205,7 @@ extension TableViewCoordinator { columnName: columnName, oldValue: oldValue, newValue: newValue, - originalRow: rowData.values + originalRow: rowProvider.rowValues(at: row) ?? [] ) rowProvider.updateValue(newValue, at: row, columnIndex: columnIndex) diff --git a/TablePro/Views/Results/KeyHandlingTableView.swift b/TablePro/Views/Results/KeyHandlingTableView.swift index 8b8cefa3..e3ac2e51 100644 --- a/TablePro/Views/Results/KeyHandlingTableView.swift +++ b/TablePro/Views/Results/KeyHandlingTableView.swift @@ -235,7 +235,7 @@ final class KeyHandlingTableView: NSTableView { // Multiline values use overlay editor instead of field editor let columnIndex = focusedColumn - 1 - if let value = coordinator?.rowProvider.row(at: row)?.value(at: columnIndex), + if let value = coordinator?.rowProvider.value(atRow: row, column: columnIndex), value.containsLineBreak { coordinator?.showOverlayEditor(tableView: self, row: row, column: focusedColumn, columnIndex: columnIndex, value: value) return diff --git a/TableProTests/Models/RowProviderTests.swift b/TableProTests/Models/RowProviderTests.swift index fc2ed518..6e023f87 100644 --- a/TableProTests/Models/RowProviderTests.swift +++ b/TableProTests/Models/RowProviderTests.swift @@ -213,14 +213,6 @@ struct InMemoryRowProviderTests { #expect(provider.row(at: 100) == nil) } - @Test("row(at:) returns cached instance on second call") - func rowAtCachedIdentity() { - let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) - let first = provider.row(at: 1) - let second = provider.row(at: 1) - #expect(first === second) - } - // MARK: - fetchRows @Test("fetchRows returns full range") @@ -289,21 +281,18 @@ struct InMemoryRowProviderTests { // MARK: - updateValue - @Test("updateValue changes source and cache") + @Test("updateValue changes value") func updateValueChanges() { let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) - let _ = provider.row(at: 1) provider.updateValue("updated", at: 1, columnIndex: 0) - let row = provider.row(at: 1) - #expect(row?.value(at: 0) == "updated") + #expect(provider.value(atRow: 1, column: 0) == "updated") } @Test("updateValue sets value to nil") func updateValueNil() { let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) - let _ = provider.row(at: 0) provider.updateValue(nil, at: 0, columnIndex: 1) - #expect(provider.row(at: 0)?.value(at: 1) == nil) + #expect(provider.value(atRow: 0, column: 1) == nil) } @Test("updateValue out-of-bounds row is no-op") @@ -313,14 +302,12 @@ struct InMemoryRowProviderTests { #expect(provider.totalRowCount == 3) } - @Test("updateValue refreshes cached row") - func updateValueRefreshesCache() { + @Test("updateValue reflects in direct access") + func updateValueReflectsInDirectAccess() { let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) - let before = provider.row(at: 0) - #expect(before?.value(at: 0) == "id_0") + #expect(provider.value(atRow: 0, column: 0) == "id_0") provider.updateValue("changed", at: 0, columnIndex: 0) - let after = provider.row(at: 0) - #expect(after?.value(at: 0) == "changed") + #expect(provider.value(atRow: 0, column: 0) == "changed") } // MARK: - appendRow @@ -339,13 +326,12 @@ struct InMemoryRowProviderTests { #expect(index == 5) } - @Test("Appended row is accessible via row(at:)") + @Test("Appended row is accessible via value(atRow:column:)") func appendRowAccessible() { let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 1) let index = provider.appendRow(values: ["x", "y", "z"]) - let row = provider.row(at: index) - #expect(row?.value(at: 0) == "x") - #expect(row?.value(at: 2) == "z") + #expect(provider.value(atRow: index, column: 0) == "x") + #expect(provider.value(atRow: index, column: 2) == "z") } @Test("Multiple appends work correctly") @@ -366,7 +352,7 @@ struct InMemoryRowProviderTests { let index = provider.appendRow(values: ["val"]) #expect(index == 0) #expect(provider.totalRowCount == 1) - #expect(provider.row(at: 0)?.value(at: 0) == "val") + #expect(provider.value(atRow: 0, column: 0) == "val") } // MARK: - removeRow @@ -392,15 +378,6 @@ struct InMemoryRowProviderTests { #expect(provider.totalRowCount == 3) } - @Test("removeRow invalidates cache") - func removeRowInvalidatesCache() { - let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) - let before = provider.row(at: 0) - provider.removeRow(at: 1) - let after = provider.row(at: 0) - #expect(before !== after) - } - // MARK: - removeRows @Test("removeRows removes multiple rows") @@ -433,21 +410,11 @@ struct InMemoryRowProviderTests { // MARK: - invalidateCache - @Test("invalidateCache clears cache - new ref identity") - func invalidateCacheClearsCache() { - let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) - let before = provider.row(at: 0) - provider.invalidateCache() - let after = provider.row(at: 0) - #expect(before !== after) - } - @Test("invalidateCache preserves data") func invalidateCachePreservesData() { let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) provider.invalidateCache() - let row = provider.row(at: 0) - #expect(row?.value(at: 0) == "id_0") + #expect(provider.value(atRow: 0, column: 0) == "id_0") #expect(provider.totalRowCount == 3) } @@ -459,7 +426,7 @@ struct InMemoryRowProviderTests { let newRows = [QueryResultRow(id: 0, values: ["new_a", "new_b", "new_c"])] provider.updateRows(newRows) #expect(provider.totalRowCount == 1) - #expect(provider.row(at: 0)?.value(at: 0) == "new_a") + #expect(provider.value(atRow: 0, column: 0) == "new_a") } @Test("updateRows with empty array sets count to 0") @@ -469,75 +436,64 @@ struct InMemoryRowProviderTests { #expect(provider.totalRowCount == 0) } - @Test("updateRows invalidates cache") - func updateRowsInvalidatesCache() { + // MARK: - Direct Access Methods + + @Test("value(atRow:column:) returns correct value") + func valueAtRowColumn() { let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) - let before = provider.row(at: 0) - let newRows = TestFixtures.makeQueryResultRows(count: 3) - provider.updateRows(newRows) - let after = provider.row(at: 0) - #expect(before !== after) + #expect(provider.value(atRow: 0, column: 0) == "id_0") + #expect(provider.value(atRow: 1, column: 1) == "name_1") + #expect(provider.value(atRow: 2, column: 2) == "email_2") } - // MARK: - Cache eviction + @Test("value(atRow:column:) returns nil for out-of-bounds row") + func valueAtRowOutOfBounds() { + let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) + #expect(provider.value(atRow: -1, column: 0) == nil) + #expect(provider.value(atRow: 3, column: 0) == nil) + } - @Test("6000 rows can be accessed without crash") - func largeRowCountNoCrash() { - let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 6000) - for i in 0..<6000 { - let _ = provider.row(at: i) - } - #expect(provider.totalRowCount == 6000) + @Test("value(atRow:column:) returns nil for out-of-bounds column") + func valueAtColumnOutOfBounds() { + let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) + #expect(provider.value(atRow: 0, column: -1) == nil) + #expect(provider.value(atRow: 0, column: 100) == nil) } - @Test("Recent rows are accessible after eviction") - func recentRowsAccessible() { - let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 6000) - for i in 0..<6000 { - let _ = provider.row(at: i) - } - let row = provider.row(at: 5999) - #expect(row != nil) - #expect(row?.value(at: 0) == "id_5999") + @Test("rowValues(at:) returns correct array") + func rowValuesAt() { + let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) + let values = provider.rowValues(at: 1) + #expect(values == ["id_1", "name_1", "email_1"]) } - @Test("Data integrity preserved after cache eviction") - func dataIntegrityAfterEviction() { - let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 6000) - for i in 0..<6000 { - let _ = provider.row(at: i) - } - let row = provider.row(at: 0) - #expect(row != nil) - #expect(row?.value(at: 0) == "id_0") + @Test("rowValues(at:) returns nil for out-of-bounds") + func rowValuesAtOutOfBounds() { + let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) + #expect(provider.rowValues(at: -1) == nil) + #expect(provider.rowValues(at: 3) == nil) } - @Test("Eviction keeps rows closest to access point") - func evictionKeepsClosestRows() { - let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 6000) - for i in 0 ..< 6000 { - let _ = provider.row(at: i) - } - let _ = provider.row(at: 4000) - let nearby = provider.row(at: 4001) - #expect(nearby != nil) - #expect(nearby?.value(at: 0) == "id_4001") + @Test("value(atRow:column:) reflects updateValue") + func valueReflectsUpdate() { + let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 3) + provider.updateValue("changed", at: 1, columnIndex: 0) + #expect(provider.value(atRow: 1, column: 0) == "changed") } - @Test("Eviction preserves data integrity across multiple eviction cycles") - func evictionMultipleCycles() { - let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 12000) - for i in 0 ..< 6000 { - let _ = provider.row(at: i) - } - for i in 6000 ..< 12000 { - let _ = provider.row(at: i) - } - let early = provider.row(at: 100) - #expect(early != nil) - #expect(early?.value(at: 0) == "id_100") - let late = provider.row(at: 11999) - #expect(late != nil) - #expect(late?.value(at: 0) == "id_11999") + @Test("rowValues(at:) reflects appendRow") + func rowValuesReflectsAppend() { + let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 2) + let index = provider.appendRow(values: ["a", "b", "c"]) + let values = provider.rowValues(at: index) + #expect(values == ["a", "b", "c"]) + } + + @Test("Large row count direct access works") + func largeRowCountDirectAccess() { + let provider = TestFixtures.makeInMemoryRowProvider(rowCount: 10000) + #expect(provider.value(atRow: 0, column: 0) == "id_0") + #expect(provider.value(atRow: 9999, column: 0) == "id_9999") + #expect(provider.totalRowCount == 10000) } } diff --git a/TableProTests/Views/Results/RowProviderSyncTests.swift b/TableProTests/Views/Results/RowProviderSyncTests.swift index 865988da..ed738c19 100644 --- a/TableProTests/Views/Results/RowProviderSyncTests.swift +++ b/TableProTests/Views/Results/RowProviderSyncTests.swift @@ -46,7 +46,7 @@ struct RowProviderSyncTests { @Test("Single cell edit syncs to new provider") func singleCellEditSyncsToNewProvider() { let (manager, providerA) = makeScenario() - let originalRow = providerA.row(at: 1)!.values + let originalRow = providerA.rowValues(at: 1)! // Edit row 1, col 1: "name_1" → "new" manager.recordCellChange( @@ -62,16 +62,16 @@ struct RowProviderSyncTests { // Simulate SwiftUI providing a stale cached provider let rows = TestFixtures.makeQueryResultRows(count: 3, columns: ["id", "name", "email"]) let providerB = InMemoryRowProvider(rows: rows, columns: ["id", "name", "email"]) - #expect(providerB.row(at: 1)?.value(at: 1) == "name_1") + #expect(providerB.value(atRow: 1, column: 1) == "name_1") reapplyChanges(from: manager, to: providerB) - #expect(providerB.row(at: 1)?.value(at: 1) == "new") + #expect(providerB.value(atRow: 1, column: 1) == "new") } @Test("Multiple cell edits on same row sync correctly") func multipleCellEditsSameRowSync() { let (manager, providerA) = makeScenario() - let originalRow = providerA.row(at: 0)!.values + let originalRow = providerA.rowValues(at: 0)! manager.recordCellChange( rowIndex: 0, @@ -94,15 +94,15 @@ struct RowProviderSyncTests { let providerB = InMemoryRowProvider(rows: rows, columns: ["id", "name", "email"]) reapplyChanges(from: manager, to: providerB) - #expect(providerB.row(at: 0)?.value(at: 1) == "updated_name") - #expect(providerB.row(at: 0)?.value(at: 2) == "updated_email") + #expect(providerB.value(atRow: 0, column: 1) == "updated_name") + #expect(providerB.value(atRow: 0, column: 2) == "updated_email") } @Test("Multiple cell edits on different rows sync correctly") func multipleCellEditsDifferentRowsSync() { let (manager, providerA) = makeScenario() - let originalRow0 = providerA.row(at: 0)!.values - let originalRow2 = providerA.row(at: 2)!.values + let originalRow0 = providerA.rowValues(at: 0)! + let originalRow2 = providerA.rowValues(at: 2)! manager.recordCellChange( rowIndex: 0, @@ -125,14 +125,14 @@ struct RowProviderSyncTests { let providerB = InMemoryRowProvider(rows: rows, columns: ["id", "name", "email"]) reapplyChanges(from: manager, to: providerB) - #expect(providerB.row(at: 0)?.value(at: 1) == "new_name_0") - #expect(providerB.row(at: 2)?.value(at: 2) == "new_email_2") + #expect(providerB.value(atRow: 0, column: 1) == "new_name_0") + #expect(providerB.value(atRow: 2, column: 2) == "new_email_2") } @Test("Edit then undo leaves provider unchanged") func editThenUndoLeavesProviderUnchanged() { let (manager, providerA) = makeScenario() - let originalRow = providerA.row(at: 1)!.values + let originalRow = providerA.rowValues(at: 1)! manager.recordCellChange( rowIndex: 1, @@ -149,13 +149,13 @@ struct RowProviderSyncTests { let providerB = InMemoryRowProvider(rows: rows, columns: ["id", "name", "email"]) reapplyChanges(from: manager, to: providerB) - #expect(providerB.row(at: 1)?.value(at: 1) == "name_1") + #expect(providerB.value(atRow: 1, column: 1) == "name_1") } @Test("Edit, undo, redo syncs correctly") func editUndoRedoSyncsCorrectly() { let (manager, providerA) = makeScenario() - let originalRow = providerA.row(at: 1)!.values + let originalRow = providerA.rowValues(at: 1)! manager.recordCellChange( rowIndex: 1, @@ -173,7 +173,7 @@ struct RowProviderSyncTests { let providerB = InMemoryRowProvider(rows: rows, columns: ["id", "name", "email"]) reapplyChanges(from: manager, to: providerB) - #expect(providerB.row(at: 1)?.value(at: 1) == "new") + #expect(providerB.value(atRow: 1, column: 1) == "new") } @Test("Inserted row cell edit syncs to new provider") @@ -201,13 +201,13 @@ struct RowProviderSyncTests { let providerB = InMemoryRowProvider(rows: rows, columns: columns) reapplyChanges(from: manager, to: providerB) - #expect(providerB.row(at: 3)?.value(at: 1) == "inserted_val") + #expect(providerB.value(atRow: 3, column: 1) == "inserted_val") } @Test("Deleted row does not affect sync") func deletedRowDoesNotAffectSync() { let (manager, providerA) = makeScenario() - let originalRow = providerA.row(at: 1)!.values + let originalRow = providerA.rowValues(at: 1)! manager.recordRowDeletion(rowIndex: 1, originalRow: originalRow) @@ -218,15 +218,15 @@ struct RowProviderSyncTests { reapplyChanges(from: manager, to: providerB) // ProviderB values remain unchanged - #expect(providerB.row(at: 0)?.value(at: 0) == "id_0") - #expect(providerB.row(at: 1)?.value(at: 1) == "name_1") - #expect(providerB.row(at: 2)?.value(at: 2) == "email_2") + #expect(providerB.value(atRow: 0, column: 0) == "id_0") + #expect(providerB.value(atRow: 1, column: 1) == "name_1") + #expect(providerB.value(atRow: 2, column: 2) == "email_2") } @Test("Multiple edits to same cell — last value wins") func multipleEditsToSameCellLastValueWins() { let (manager, providerA) = makeScenario() - let originalRow = providerA.row(at: 0)!.values + let originalRow = providerA.rowValues(at: 0)! manager.recordCellChange( rowIndex: 0, @@ -249,13 +249,13 @@ struct RowProviderSyncTests { let providerB = InMemoryRowProvider(rows: rows, columns: ["id", "name", "email"]) reapplyChanges(from: manager, to: providerB) - #expect(providerB.row(at: 0)?.value(at: 1) == "c") + #expect(providerB.value(atRow: 0, column: 1) == "c") } @Test("Reapply is idempotent") func reapplyIsIdempotent() { let (manager, providerA) = makeScenario() - let originalRow = providerA.row(at: 0)!.values + let originalRow = providerA.rowValues(at: 0)! manager.recordCellChange( rowIndex: 0, @@ -270,17 +270,17 @@ struct RowProviderSyncTests { let providerB = InMemoryRowProvider(rows: rows, columns: ["id", "name", "email"]) reapplyChanges(from: manager, to: providerB) - #expect(providerB.row(at: 0)?.value(at: 1) == "updated") + #expect(providerB.value(atRow: 0, column: 1) == "updated") // Apply again — should remain correct, no corruption reapplyChanges(from: manager, to: providerB) - #expect(providerB.row(at: 0)?.value(at: 1) == "updated") + #expect(providerB.value(atRow: 0, column: 1) == "updated") } @Test("Null value syncs correctly") func nullValueSyncsCorrectly() { let (manager, providerA) = makeScenario() - let originalRow = providerA.row(at: 0)!.values + let originalRow = providerA.rowValues(at: 0)! manager.recordCellChange( rowIndex: 0, @@ -295,7 +295,7 @@ struct RowProviderSyncTests { let providerB = InMemoryRowProvider(rows: rows, columns: ["id", "name", "email"]) reapplyChanges(from: manager, to: providerB) - #expect(providerB.row(at: 0)?.value(at: 1) == nil) + #expect(providerB.value(atRow: 0, column: 1) == nil) } @Test("Reapply with no changes is a no-op") @@ -307,19 +307,19 @@ struct RowProviderSyncTests { reapplyChanges(from: manager, to: providerB) - #expect(providerB.row(at: 0)?.value(at: 0) == "id_0") - #expect(providerB.row(at: 0)?.value(at: 1) == "name_0") - #expect(providerB.row(at: 0)?.value(at: 2) == "email_0") - #expect(providerB.row(at: 1)?.value(at: 0) == "id_1") - #expect(providerB.row(at: 1)?.value(at: 1) == "name_1") - #expect(providerB.row(at: 2)?.value(at: 2) == "email_2") + #expect(providerB.value(atRow: 0, column: 0) == "id_0") + #expect(providerB.value(atRow: 0, column: 1) == "name_0") + #expect(providerB.value(atRow: 0, column: 2) == "email_0") + #expect(providerB.value(atRow: 1, column: 0) == "id_1") + #expect(providerB.value(atRow: 1, column: 1) == "name_1") + #expect(providerB.value(atRow: 2, column: 2) == "email_2") } @Test("Batch delete does not crash") func batchDeleteDoesNotCrash() { let (manager, providerA) = makeScenario() - let originalRow0 = providerA.row(at: 0)!.values - let originalRow1 = providerA.row(at: 1)!.values + let originalRow0 = providerA.rowValues(at: 0)! + let originalRow1 = providerA.rowValues(at: 1)! manager.recordBatchRowDeletion(rows: [ (rowIndex: 0, originalRow: originalRow0), @@ -333,8 +333,8 @@ struct RowProviderSyncTests { reapplyChanges(from: manager, to: providerB) // Values remain unchanged - #expect(providerB.row(at: 0)?.value(at: 0) == "id_0") - #expect(providerB.row(at: 1)?.value(at: 1) == "name_1") - #expect(providerB.row(at: 2)?.value(at: 2) == "email_2") + #expect(providerB.value(atRow: 0, column: 0) == "id_0") + #expect(providerB.value(atRow: 1, column: 1) == "name_1") + #expect(providerB.value(atRow: 2, column: 2) == "email_2") } } diff --git a/docs/development/MEMORY-AUDIT.md b/docs/development/MEMORY-AUDIT.md index 309ad210..76e30135 100644 --- a/docs/development/MEMORY-AUDIT.md +++ b/docs/development/MEMORY-AUDIT.md @@ -51,7 +51,7 @@ For sorted tabs, `sortedRows(for:)` at `MainEditorContentView.swift:406` calls ` **Fix options:** - [x] Make `InMemoryRowProvider` reference `RowBuffer` directly instead of copying rows — **Done**: primary init takes `rowBuffer: RowBuffer` + optional `sortIndices: [Int]?`; convenience init wraps rows for backward compat -- [ ] Replace `rowCache` with index-based access into `sourceRows` (avoid `TableRowData` wrapper objects) +- [x] Replace `rowCache` with index-based access into `sourceRows` (avoid `TableRowData` wrapper objects) — **Done**: added `value(atRow:column:)` and `rowValues(at:)` for zero-allocation direct access; removed 5,000-entry `rowCache` dictionary and `TableRowData` materialization - [x] For sorted tabs, store only the index permutation and let the provider apply it lazily — **Done**: `sortIndicesForTab(_:)` returns `[Int]?` permutation; `InMemoryRowProvider` resolves display→source indices via `resolveSourceIndex()` --- @@ -140,6 +140,7 @@ Each `SQLEditorCoordinator` registers for `NSWindow.didUpdateNotification` with **Fix options:** - [x] Filter by `object: textView.window` after the editor's window is known — **Done**: notification handler early-returns when `notification.object` doesn't match the editor's own window +- [x] Consolidate all per-editor monitors into a shared `EditorEventRouter` singleton — **Done**: right-click, clipboard, and window-update monitors reduced from O(n) to O(1) per event - [ ] Or use KVO on the specific window's `firstResponder` instead --- @@ -171,6 +172,9 @@ Each macOS native tab creates a full `NSWindow` with its own: This is inherent to the native-tab architecture and not easily reducible without moving to in-process tabs. **Fix options:** +- [x] Lazy `AIChatViewModel` — defer creation until AI panel is first opened — **Done**: backed by optional, instantiated on first access +- [x] Remove duplicate `connections` array from `ContentView` — **Done**: use `ConnectionStorage.shared` directly +- [x] Fix tab persistence last-write-wins — **Done**: aggregate tabs from all coordinators at quit via static registry; only first coordinator saves - [ ] Consider lightweight in-process tab bar (shared NSWindow) for table tabs — major architectural change - [ ] Or accept this as the cost of native tabs and focus on reducing per-tab data overhead (items 2, 3, 5 above)