perf(config): cache parsed config + lean serialize + drop load() from log flush (long-run lag)
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) <noreply@anthropic.com>
This commit is contained in:
parent
939d30abfe
commit
29d1944328
@ -54,6 +54,7 @@ const nodeGlobals = {
|
||||
URLSearchParams: 'readonly',
|
||||
fetch: 'readonly',
|
||||
crypto: 'readonly',
|
||||
structuredClone: 'readonly',
|
||||
};
|
||||
|
||||
export default [
|
||||
|
||||
@ -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);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
34
main.js
34
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');
|
||||
|
||||
@ -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({
|
||||
|
||||
Loading…
Reference in New Issue
Block a user