From 29d194432819bd9b9295be8896c43f4223d95e5d Mon Sep 17 00:00:00 2001 From: Administrator Date: Sun, 21 Jun 2026 00:49:55 +0200 Subject: [PATCH] perf(config): cache parsed config + lean serialize + drop load() from log flush (long-run lag) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 13-agent hunt pinned the "wird mit der Zeit laggy" symptom (CPU/RAM normal, UI sluggish after many uploads) to the main process re-doing config I/O that scales with the ever-growing history, stalling the synchronous main event loop so the renderer's IPC round-trips feel laggy. The renderer render path was already optimized (virtualized queue, capped panels) — confirmed clean. This commit lands the two contained fixes (T1 + T3); the queue-persist rewrite (T2) follows separately. config-store (T1): - load() now has an in-memory cache keyed on the file mtime+size. The processed config (merged + credential-decrypted) is re-read/re-parsed/re-DPAPI-decrypted ONLY when the file actually changes; our writes invalidate it, external edits change mtime/size so the cache misses. Eliminates a full disk read + JSON.parse of the whole growing history + per-credential decrypt on the vast majority of the ~38 load() call sites. load() always returns a structuredClone so callers can mutate freely without corrupting the cache. - _serializeForDisk clones ONLY the hosters subtree (the only thing encryptCredentials touches) instead of JSON.parse(JSON.stringify(whole config)) — no more deep-cloning an 8 MB history on every write. - _atomicWrite refreshes the .bak with a raw fs.copyFileSync instead of read + JSON.parse + write (it was re-parsing the full config a 2nd time per write). main.js (T3): - The log-flush paths resolved the log file via configStore.load() ~8x/second during uploads (re-reading + cloning the whole config just to read logMode/logFilePath). Cache those two strings in module scope, invalidate on the settings-save handlers. Verified: 26 config-store tests (incl. new cache-correctness: independent clones, external-change invalidation, save invalidation) + full 394-test suite green, lint 0 errors. Benchmark (8000-batch / 4.6 MB history): log flush no longer calls load() at all; the remaining per-write history serialize is what T2 removes from the hot path. Co-Authored-By: Claude Opus 4.8 (1M context) --- eslint.config.mjs | 1 + lib/config-store.js | 81 +++++++++++++++++++++++++++----------- main.js | 34 ++++++++++++---- tests/config-store.test.js | 26 ++++++++++++ 4 files changed, 110 insertions(+), 32 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index eaa0314..a080f80 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -54,6 +54,7 @@ const nodeGlobals = { URLSearchParams: 'readonly', fetch: 'readonly', crypto: 'readonly', + structuredClone: 'readonly', }; export default [ diff --git a/lib/config-store.js b/lib/config-store.js index 97a7698..c455f35 100644 --- a/lib/config-store.js +++ b/lib/config-store.js @@ -185,6 +185,8 @@ class ConfigStore { : path.join(__dirname, '..'); this.filePath = path.join(dir, 'electron-config.json'); this._writeQueue = Promise.resolve(); // Serializes all writes to prevent race conditions + this._cache = null; + this._cacheKey = ''; // Migrate config from old location if current doesn't exist if (!fs.existsSync(this.filePath) && app && app.isPackaged) { @@ -221,8 +223,29 @@ class ConfigStore { return JSON.parse(raw); } + _clone(obj) { + try { return structuredClone(obj); } + catch { return JSON.parse(JSON.stringify(obj)); } + } + load() { try { + // In-memory cache keyed on the file's mtime+size. The processed config + // (merged + credential-decrypted) is reparsed/re-decrypted from disk ONLY + // when the file actually changes. Our own writes refresh the cache (see + // _commit), and an external edit changes mtime/size so the cache misses + // and we reread. Without this, every one of the ~38 main.js load() call + // sites (incl. the per-500ms log-flush path) re-read disk + JSON.parse the + // whole growing history + DPAPI-decrypt every credential — the dominant + // long-running main-thread drag. load() always returns a CLONE so callers + // can mutate the result without corrupting the cache. + let stat = null; + try { stat = fs.statSync(this.filePath); } catch {} + const statKey = stat ? `${stat.mtimeMs}:${stat.size}` : ''; + if (stat && this._cache && this._cacheKey === statKey) { + return this._clone(this._cache); + } + let data = null; // Try main config try { data = this._readAndParse(this.filePath); } catch {} @@ -309,7 +332,11 @@ class ConfigStore { // Decrypt credentials stored with safeStorage so the rest of the app // keeps working with plaintext in memory. secretStore.decryptCredentials(result); - return result; + if (stat) { + this._cache = result; + this._cacheKey = statKey; + } + return this._clone(result); } catch { const fresh = JSON.parse(JSON.stringify(DEFAULTS)); fresh.globalSettings.logMode = normalizeLogMode(fresh.globalSettings); @@ -317,12 +344,19 @@ class ConfigStore { } } - // Deep-clone a config and encrypt its credential fields. Never mutate the - // caller's object — the rest of the app holds plaintext references. + // Encrypt credential fields without mutating the caller's plaintext object. + // Only `hosters` carries credentials, so we clone ONLY that subtree — the rest + // (history, globalSettings, …) is referenced read-only into the stringified + // object. Deep-cloning the whole config here (incl. an ever-growing history) + // on every write was a primary long-running main-thread stall. _serializeForDisk(config) { - const clone = JSON.parse(JSON.stringify(config)); - secretStore.encryptCredentials(clone); - return JSON.stringify(clone, null, 2); + const hosters = this._clone(config.hosters || {}); + secretStore.encryptCredentials({ hosters }); + return JSON.stringify({ ...config, hosters }, null, 2); + } + + _commit(config) { + return this._atomicWrite(this._serializeForDisk(config)); } _enqueueWrite(fn) { @@ -336,7 +370,7 @@ class ConfigStore { if (config.hosters) current.hosters = config.hosters; if (config.hosterSettings) current.hosterSettings = config.hosterSettings; if (config.globalSettings) current.globalSettings = config.globalSettings; - return this._atomicWrite(this._serializeForDisk(current)); + return this._commit(current); }); } @@ -352,25 +386,24 @@ class ConfigStore { fs.writeFile(tmpPath, data, 'utf-8', (err) => { if (err) return reject(err); try { - // Refresh .bak from the previous live file. Wrapped in try/catch - // so an AV/indexer briefly locking the file doesn't fail the whole - // save — the rename to the live path is the part that matters, - // a stale .bak is preferable to losing the new write entirely. + // Refresh .bak from the previous live file with a raw byte copy — + // no read+JSON.parse+write. The live file was itself written through + // this atomic path, so re-validating it by parsing the whole (growing) + // config on every write was pure waste. Wrapped in try/catch so an + // AV/indexer briefly locking the file doesn't fail the save — the + // rename to the live path is the part that matters. try { if (fs.existsSync(this.filePath)) { - const existing = fs.readFileSync(this.filePath, 'utf-8'); - if (existing && existing.trim().length > 2) { - let isValid = false; - try { - const parsed = JSON.parse(existing); - isValid = parsed && typeof parsed === 'object' && (parsed.hosters || parsed.hosterSettings || parsed.globalSettings); - } catch {} - if (isValid) fs.writeFileSync(backupPath, existing, 'utf-8'); - } + fs.copyFileSync(this.filePath, backupPath); } } catch {} fs.renameSync(tmpPath, this.filePath); } catch (e) { return reject(e); } + // Invalidate the read cache: the next load() re-reads + re-merges the + // freshly-written file (the on-disk format is sparse — load() fills + // defaults — so we must NOT serve a pre-merge in-memory object). + this._cache = null; + this._cacheKey = ''; resolve(); }); }); @@ -382,7 +415,7 @@ class ConfigStore { config.history.push(entry); const retention = (config.globalSettings && config.globalSettings.historyRetention) || 'all'; config.history = applyHistoryRetention(config.history, retention, Date.now()); - return this._atomicWrite(this._serializeForDisk(config)); + return this._commit(config); }); } @@ -402,7 +435,7 @@ class ConfigStore { if (dryRun) return result; config.history = pruned; if (config.globalSettings) config.globalSettings.historyRetention = String(retention || 'all'); - return this._atomicWrite(this._serializeForDisk(config)).then(() => result); + return this._commit(config).then(() => result); }); } @@ -410,7 +443,7 @@ class ConfigStore { return this._enqueueWrite(() => { const config = this.load(); config.history = []; - return this._atomicWrite(this._serializeForDisk(config)); + return this._commit(config); }); } @@ -418,7 +451,7 @@ class ConfigStore { return this._enqueueWrite(() => { const config = this.load(); config.rotationCursors = (cursors && typeof cursors === 'object' && !Array.isArray(cursors)) ? cursors : {}; - return this._atomicWrite(this._serializeForDisk(config)); + return this._commit(config); }); } } diff --git a/main.js b/main.js index b8b4994..4135528 100644 --- a/main.js +++ b/main.js @@ -414,11 +414,26 @@ function getDefaultLogFilePath() { return path.join(__dirname, 'fileuploader.log'); } +// The log flush paths resolve the log file ~8x/second during uploads. Going +// through configStore.load() there meant re-reading + cloning the whole config +// (incl. an 8 MB+ history) on every flush — a major long-running main-thread +// drag. logFilePath/logMode change only when the user saves settings, so cache +// the two strings and invalidate on those saves (see _invalidateLogSettings). +let _cachedLogSettings = null; +function _getLogSettings() { + if (!_cachedLogSettings) { + const gs = (configStore.load() || {}).globalSettings || {}; + _cachedLogSettings = { + logFilePath: String(gs.logFilePath || '').trim(), + logMode: gs.logMode || 'single' + }; + } + return _cachedLogSettings; +} +function _invalidateLogSettings() { _cachedLogSettings = null; } + function getBaseLogFilePath() { - const config = configStore.load(); - const customPath = config && config.globalSettings - ? String(config.globalSettings.logFilePath || '').trim() - : ''; + const customPath = _getLogSettings().logFilePath; return customPath || getDefaultLogFilePath(); } @@ -433,8 +448,7 @@ let _activeLogKey = null; // remembers (mode + date-or-session) so cache rolls let _activeLogPath = null; function getLogFilePath() { - const config = configStore.load(); - const mode = (config && config.globalSettings && config.globalSettings.logMode) || 'single'; + const mode = _getLogSettings().logMode; const base = getBaseLogFilePath(); const dir = path.dirname(base); const ext = path.extname(base); @@ -454,8 +468,7 @@ function getLogFilePath() { function buildFallbackLogName(dir) { // Match the active log-mode's naming so the fallback file is consistent with // what the primary write would have produced. - const config = configStore.load(); - const mode = (config && config.globalSettings && config.globalSettings.logMode) || 'single'; + const mode = _getLogSettings().logMode; return path.join(dir, resolveLogFileName({ baseName: 'fileuploader', ext: '.log', mode, date: new Date(), sessionId: SESSION_ID })); } @@ -596,6 +609,7 @@ function _persistFallbackLogPath(workingPath) { cfg.globalSettings = gs; configStore.save({ globalSettings: gs }).catch(() => {}); _invalidateUploadLogTargetCache(); + _invalidateLogSettings(); safeSend('log-path-auto-updated', { logFilePath: toSave }); } catch (err) { debugLog(`persist fallback logpath failed: ${err.message}`); @@ -1315,6 +1329,7 @@ ipcMain.handle('get-config', () => { ipcMain.handle('save-config', async (_event, config) => { await configStore.save(config); + if (config && config.globalSettings) _invalidateLogSettings(); try { if (config && config.globalSettings && Object.prototype.hasOwnProperty.call(config.globalSettings, 'logVerbose')) { setLogVerbose(!!config.globalSettings.logVerbose); @@ -2152,6 +2167,7 @@ ipcMain.handle('import-backup', async (_event, legacyPassword) => { history: [] }; await configStore._atomicWrite(configStore._serializeForDisk(merged)); + _invalidateLogSettings(); return { ok: true, config: configStore.load() }; }); @@ -2286,6 +2302,7 @@ function _preserveDiagSubtree(globalSettings) { ipcMain.handle('save-global-settings', async (_event, globalSettings) => { globalSettings = _preserveDiagSubtree(globalSettings); await configStore.save({ globalSettings }); + _invalidateLogSettings(); if (uploadManager) uploadManager.updateSettings(null, globalSettings); return true; }); @@ -2328,6 +2345,7 @@ ipcMain.on('save-global-settings-sync', (event, globalSettings) => { const _diskDiag = current.globalSettings && current.globalSettings.diagnostics; current.globalSettings = globalSettings; if (_diskDiag) current.globalSettings.diagnostics = _diskDiag; + _invalidateLogSettings(); const data = configStore._serializeForDisk(current); const backupPath = configStore.filePath + '.bak'; fs.writeFileSync(tmpPath, data, 'utf-8'); diff --git a/tests/config-store.test.js b/tests/config-store.test.js index edb7c55..809df3e 100644 --- a/tests/config-store.test.js +++ b/tests/config-store.test.js @@ -253,6 +253,32 @@ describe('ConfigStore', () => { assert.equal(config.globalSettings.alwaysOnTop, true); }); + it('load() returns independent clones — mutating one result must not leak into the cache', () => { + store.load(); // warm the cache + const a = store.load(); + a.globalSettings.alwaysOnTop = true; + a.hosters['voe.sx'].push({ id: 'mutant' }); + a.history.push({ id: 'ghost' }); + const b = store.load(); + assert.equal(b.globalSettings.alwaysOnTop, false, 'mutating a prior load() result must not corrupt the cache'); + assert.equal(b.hosters['voe.sx'].length, 0); + assert.equal(b.history.length, 0); + }); + + it('load() reflects an external file change (mtime/size cache invalidation)', () => { + store.load(); // warm cache on the no-file defaults + fs.writeFileSync(store.filePath, JSON.stringify({ globalSettings: { alwaysOnTop: true } }), 'utf-8'); + assert.equal(store.load().globalSettings.alwaysOnTop, true, 'an external write must invalidate the cache'); + fs.writeFileSync(store.filePath, JSON.stringify({ globalSettings: { alwaysOnTop: false } }), 'utf-8'); + assert.equal(store.load().globalSettings.alwaysOnTop, false, 'a second external write must be seen too'); + }); + + it('save() invalidates the cache so the next load() sees the new value', async () => { + assert.equal(store.load().globalSettings.alwaysOnTop, false); + await store.save({ globalSettings: { alwaysOnTop: true } }); + assert.equal(store.load().globalSettings.alwaysOnTop, true, 'load() after save() must reflect the write'); + }); + it('backup recovery when main file is corrupted', () => { // Write valid config first fs.writeFileSync(store.filePath, JSON.stringify({