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',
|
URLSearchParams: 'readonly',
|
||||||
fetch: 'readonly',
|
fetch: 'readonly',
|
||||||
crypto: 'readonly',
|
crypto: 'readonly',
|
||||||
|
structuredClone: 'readonly',
|
||||||
};
|
};
|
||||||
|
|
||||||
export default [
|
export default [
|
||||||
|
|||||||
@ -185,6 +185,8 @@ class ConfigStore {
|
|||||||
: path.join(__dirname, '..');
|
: path.join(__dirname, '..');
|
||||||
this.filePath = path.join(dir, 'electron-config.json');
|
this.filePath = path.join(dir, 'electron-config.json');
|
||||||
this._writeQueue = Promise.resolve(); // Serializes all writes to prevent race conditions
|
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
|
// Migrate config from old location if current doesn't exist
|
||||||
if (!fs.existsSync(this.filePath) && app && app.isPackaged) {
|
if (!fs.existsSync(this.filePath) && app && app.isPackaged) {
|
||||||
@ -221,8 +223,29 @@ class ConfigStore {
|
|||||||
return JSON.parse(raw);
|
return JSON.parse(raw);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
_clone(obj) {
|
||||||
|
try { return structuredClone(obj); }
|
||||||
|
catch { return JSON.parse(JSON.stringify(obj)); }
|
||||||
|
}
|
||||||
|
|
||||||
load() {
|
load() {
|
||||||
try {
|
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;
|
let data = null;
|
||||||
// Try main config
|
// Try main config
|
||||||
try { data = this._readAndParse(this.filePath); } catch {}
|
try { data = this._readAndParse(this.filePath); } catch {}
|
||||||
@ -309,7 +332,11 @@ class ConfigStore {
|
|||||||
// Decrypt credentials stored with safeStorage so the rest of the app
|
// Decrypt credentials stored with safeStorage so the rest of the app
|
||||||
// keeps working with plaintext in memory.
|
// keeps working with plaintext in memory.
|
||||||
secretStore.decryptCredentials(result);
|
secretStore.decryptCredentials(result);
|
||||||
return result;
|
if (stat) {
|
||||||
|
this._cache = result;
|
||||||
|
this._cacheKey = statKey;
|
||||||
|
}
|
||||||
|
return this._clone(result);
|
||||||
} catch {
|
} catch {
|
||||||
const fresh = JSON.parse(JSON.stringify(DEFAULTS));
|
const fresh = JSON.parse(JSON.stringify(DEFAULTS));
|
||||||
fresh.globalSettings.logMode = normalizeLogMode(fresh.globalSettings);
|
fresh.globalSettings.logMode = normalizeLogMode(fresh.globalSettings);
|
||||||
@ -317,12 +344,19 @@ class ConfigStore {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Deep-clone a config and encrypt its credential fields. Never mutate the
|
// Encrypt credential fields without mutating the caller's plaintext object.
|
||||||
// caller's object — the rest of the app holds plaintext references.
|
// 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) {
|
_serializeForDisk(config) {
|
||||||
const clone = JSON.parse(JSON.stringify(config));
|
const hosters = this._clone(config.hosters || {});
|
||||||
secretStore.encryptCredentials(clone);
|
secretStore.encryptCredentials({ hosters });
|
||||||
return JSON.stringify(clone, null, 2);
|
return JSON.stringify({ ...config, hosters }, null, 2);
|
||||||
|
}
|
||||||
|
|
||||||
|
_commit(config) {
|
||||||
|
return this._atomicWrite(this._serializeForDisk(config));
|
||||||
}
|
}
|
||||||
|
|
||||||
_enqueueWrite(fn) {
|
_enqueueWrite(fn) {
|
||||||
@ -336,7 +370,7 @@ class ConfigStore {
|
|||||||
if (config.hosters) current.hosters = config.hosters;
|
if (config.hosters) current.hosters = config.hosters;
|
||||||
if (config.hosterSettings) current.hosterSettings = config.hosterSettings;
|
if (config.hosterSettings) current.hosterSettings = config.hosterSettings;
|
||||||
if (config.globalSettings) current.globalSettings = config.globalSettings;
|
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) => {
|
fs.writeFile(tmpPath, data, 'utf-8', (err) => {
|
||||||
if (err) return reject(err);
|
if (err) return reject(err);
|
||||||
try {
|
try {
|
||||||
// Refresh .bak from the previous live file. Wrapped in try/catch
|
// Refresh .bak from the previous live file with a raw byte copy —
|
||||||
// so an AV/indexer briefly locking the file doesn't fail the whole
|
// no read+JSON.parse+write. The live file was itself written through
|
||||||
// save — the rename to the live path is the part that matters,
|
// this atomic path, so re-validating it by parsing the whole (growing)
|
||||||
// a stale .bak is preferable to losing the new write entirely.
|
// 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 {
|
try {
|
||||||
if (fs.existsSync(this.filePath)) {
|
if (fs.existsSync(this.filePath)) {
|
||||||
const existing = fs.readFileSync(this.filePath, 'utf-8');
|
fs.copyFileSync(this.filePath, backupPath);
|
||||||
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');
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
} catch {}
|
} catch {}
|
||||||
fs.renameSync(tmpPath, this.filePath);
|
fs.renameSync(tmpPath, this.filePath);
|
||||||
} catch (e) { return reject(e); }
|
} 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();
|
resolve();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@ -382,7 +415,7 @@ class ConfigStore {
|
|||||||
config.history.push(entry);
|
config.history.push(entry);
|
||||||
const retention = (config.globalSettings && config.globalSettings.historyRetention) || 'all';
|
const retention = (config.globalSettings && config.globalSettings.historyRetention) || 'all';
|
||||||
config.history = applyHistoryRetention(config.history, retention, Date.now());
|
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;
|
if (dryRun) return result;
|
||||||
config.history = pruned;
|
config.history = pruned;
|
||||||
if (config.globalSettings) config.globalSettings.historyRetention = String(retention || 'all');
|
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(() => {
|
return this._enqueueWrite(() => {
|
||||||
const config = this.load();
|
const config = this.load();
|
||||||
config.history = [];
|
config.history = [];
|
||||||
return this._atomicWrite(this._serializeForDisk(config));
|
return this._commit(config);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -418,7 +451,7 @@ class ConfigStore {
|
|||||||
return this._enqueueWrite(() => {
|
return this._enqueueWrite(() => {
|
||||||
const config = this.load();
|
const config = this.load();
|
||||||
config.rotationCursors = (cursors && typeof cursors === 'object' && !Array.isArray(cursors)) ? cursors : {};
|
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');
|
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() {
|
function getBaseLogFilePath() {
|
||||||
const config = configStore.load();
|
const customPath = _getLogSettings().logFilePath;
|
||||||
const customPath = config && config.globalSettings
|
|
||||||
? String(config.globalSettings.logFilePath || '').trim()
|
|
||||||
: '';
|
|
||||||
return customPath || getDefaultLogFilePath();
|
return customPath || getDefaultLogFilePath();
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -433,8 +448,7 @@ let _activeLogKey = null; // remembers (mode + date-or-session) so cache rolls
|
|||||||
let _activeLogPath = null;
|
let _activeLogPath = null;
|
||||||
|
|
||||||
function getLogFilePath() {
|
function getLogFilePath() {
|
||||||
const config = configStore.load();
|
const mode = _getLogSettings().logMode;
|
||||||
const mode = (config && config.globalSettings && config.globalSettings.logMode) || 'single';
|
|
||||||
const base = getBaseLogFilePath();
|
const base = getBaseLogFilePath();
|
||||||
const dir = path.dirname(base);
|
const dir = path.dirname(base);
|
||||||
const ext = path.extname(base);
|
const ext = path.extname(base);
|
||||||
@ -454,8 +468,7 @@ function getLogFilePath() {
|
|||||||
function buildFallbackLogName(dir) {
|
function buildFallbackLogName(dir) {
|
||||||
// Match the active log-mode's naming so the fallback file is consistent with
|
// Match the active log-mode's naming so the fallback file is consistent with
|
||||||
// what the primary write would have produced.
|
// what the primary write would have produced.
|
||||||
const config = configStore.load();
|
const mode = _getLogSettings().logMode;
|
||||||
const mode = (config && config.globalSettings && config.globalSettings.logMode) || 'single';
|
|
||||||
return path.join(dir, resolveLogFileName({ baseName: 'fileuploader', ext: '.log', mode, date: new Date(), sessionId: SESSION_ID }));
|
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;
|
cfg.globalSettings = gs;
|
||||||
configStore.save({ globalSettings: gs }).catch(() => {});
|
configStore.save({ globalSettings: gs }).catch(() => {});
|
||||||
_invalidateUploadLogTargetCache();
|
_invalidateUploadLogTargetCache();
|
||||||
|
_invalidateLogSettings();
|
||||||
safeSend('log-path-auto-updated', { logFilePath: toSave });
|
safeSend('log-path-auto-updated', { logFilePath: toSave });
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
debugLog(`persist fallback logpath failed: ${err.message}`);
|
debugLog(`persist fallback logpath failed: ${err.message}`);
|
||||||
@ -1315,6 +1329,7 @@ ipcMain.handle('get-config', () => {
|
|||||||
|
|
||||||
ipcMain.handle('save-config', async (_event, config) => {
|
ipcMain.handle('save-config', async (_event, config) => {
|
||||||
await configStore.save(config);
|
await configStore.save(config);
|
||||||
|
if (config && config.globalSettings) _invalidateLogSettings();
|
||||||
try {
|
try {
|
||||||
if (config && config.globalSettings && Object.prototype.hasOwnProperty.call(config.globalSettings, 'logVerbose')) {
|
if (config && config.globalSettings && Object.prototype.hasOwnProperty.call(config.globalSettings, 'logVerbose')) {
|
||||||
setLogVerbose(!!config.globalSettings.logVerbose);
|
setLogVerbose(!!config.globalSettings.logVerbose);
|
||||||
@ -2152,6 +2167,7 @@ ipcMain.handle('import-backup', async (_event, legacyPassword) => {
|
|||||||
history: []
|
history: []
|
||||||
};
|
};
|
||||||
await configStore._atomicWrite(configStore._serializeForDisk(merged));
|
await configStore._atomicWrite(configStore._serializeForDisk(merged));
|
||||||
|
_invalidateLogSettings();
|
||||||
return { ok: true, config: configStore.load() };
|
return { ok: true, config: configStore.load() };
|
||||||
});
|
});
|
||||||
|
|
||||||
@ -2286,6 +2302,7 @@ function _preserveDiagSubtree(globalSettings) {
|
|||||||
ipcMain.handle('save-global-settings', async (_event, globalSettings) => {
|
ipcMain.handle('save-global-settings', async (_event, globalSettings) => {
|
||||||
globalSettings = _preserveDiagSubtree(globalSettings);
|
globalSettings = _preserveDiagSubtree(globalSettings);
|
||||||
await configStore.save({ globalSettings });
|
await configStore.save({ globalSettings });
|
||||||
|
_invalidateLogSettings();
|
||||||
if (uploadManager) uploadManager.updateSettings(null, globalSettings);
|
if (uploadManager) uploadManager.updateSettings(null, globalSettings);
|
||||||
return true;
|
return true;
|
||||||
});
|
});
|
||||||
@ -2328,6 +2345,7 @@ ipcMain.on('save-global-settings-sync', (event, globalSettings) => {
|
|||||||
const _diskDiag = current.globalSettings && current.globalSettings.diagnostics;
|
const _diskDiag = current.globalSettings && current.globalSettings.diagnostics;
|
||||||
current.globalSettings = globalSettings;
|
current.globalSettings = globalSettings;
|
||||||
if (_diskDiag) current.globalSettings.diagnostics = _diskDiag;
|
if (_diskDiag) current.globalSettings.diagnostics = _diskDiag;
|
||||||
|
_invalidateLogSettings();
|
||||||
const data = configStore._serializeForDisk(current);
|
const data = configStore._serializeForDisk(current);
|
||||||
const backupPath = configStore.filePath + '.bak';
|
const backupPath = configStore.filePath + '.bak';
|
||||||
fs.writeFileSync(tmpPath, data, 'utf-8');
|
fs.writeFileSync(tmpPath, data, 'utf-8');
|
||||||
|
|||||||
@ -253,6 +253,32 @@ describe('ConfigStore', () => {
|
|||||||
assert.equal(config.globalSettings.alwaysOnTop, true);
|
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', () => {
|
it('backup recovery when main file is corrupted', () => {
|
||||||
// Write valid config first
|
// Write valid config first
|
||||||
fs.writeFileSync(store.filePath, JSON.stringify({
|
fs.writeFileSync(store.filePath, JSON.stringify({
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user