perf(uploads): make the batch-start file-stat non-blocking — kill the 336ms main stall (v3.3.103)
The v3.3.102 log (real 224-job batch) confirmed History virtualization works and
steady-state uploads are pristine (fps=32, event-loop mean 11.8ms). The remaining
residual was a ~6s spin-up burst at batch start: the main event loop blocked for
336ms with cpu=0%core (i.e. blocked on I/O, not computing) and the renderer janked
196-391ms, then everything settled clean.
A multi-agent investigation plus an adversarial review corrected the obvious-looking
hypothesis. The renderer's uncapped progress-batch drain is NOT the cause:
handleProgress only mutates plain JS state and schedules already-coalesced renders
(one per frame), and main coalesces progress to ~50 latest-per-job entries per
100ms. Chunking that drain would fix nothing — and the reviewer showed it would
REGRESS correctness: requestAnimationFrame throttles to ~0 when the window is
minimized (the common state for a background uploader), so a rAF-chunked drain would
grow an unbounded backlog and defer persistQueueStateSoon for every buffered item,
losing terminal 'done' events on close (the queue-persistence ghost-fix class). So
that path is deliberately not taken.
The real cause (cpu=0%core = blocked on I/O) is a synchronous fs.statSync storm in
UploadManager.startBatch: the dedup loop ran up to DEDUP_CHUNK=200 synchronous
fs.statSync calls in a single tick before yielding (200 x ~1.68ms on the user's VM
= the exact 336ms), on a disk already saturated by the 1MB read-ahead.
Fix — make the batch-start stats non-blocking:
- The dedup loop now dedupes synchronously (cheap Map work) and then stats the
unique files in parallel via await Promise.all(fs.promises.stat ...) per chunk, so
the stat I/O runs on the libuv threadpool and the main thread never blocks. The
results-Map shape ({name,size,results:[]}) and dedup semantics (size 0 on failure)
are unchanged.
- The per-job statSync fallback is converted to await fs.promises.stat for
consistency (it sits in an async function before the first real await; the cached
size from dedup already lets nearly every job skip it).
Tests: the upload-manager mocks override fs.statSync; they now also override
fs.promises.stat with the same fake sizes (upload-manager.test.js x2,
suspect-reject-alternates.test.js). 407 tests pass; clean Electron boot.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d5bb97aefe
commit
7f636258d4
@ -364,16 +364,17 @@ class UploadManager extends EventEmitter {
|
|||||||
for (let i = 0; i < tasks.length; i += DEDUP_CHUNK) {
|
for (let i = 0; i < tasks.length; i += DEDUP_CHUNK) {
|
||||||
if (signal.aborted) break;
|
if (signal.aborted) break;
|
||||||
const end = Math.min(i + DEDUP_CHUNK, tasks.length);
|
const end = Math.min(i + DEDUP_CHUNK, tasks.length);
|
||||||
|
const toStat = [];
|
||||||
for (let j = i; j < end; j++) {
|
for (let j = i; j < end; j++) {
|
||||||
const task = tasks[j];
|
const task = tasks[j];
|
||||||
if (!results.has(task.file)) {
|
if (!results.has(task.file)) {
|
||||||
const fileName = path.basename(task.file);
|
results.set(task.file, { name: path.basename(task.file), size: 0, results: [] });
|
||||||
let size = 0;
|
toStat.push(task.file);
|
||||||
try { size = fs.statSync(task.file).size; } catch {}
|
|
||||||
results.set(task.file, { name: fileName, size, results: [] });
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (end < tasks.length) await new Promise(setImmediate);
|
await Promise.all(toStat.map(async (f) => {
|
||||||
|
try { const st = await fs.promises.stat(f); const e = results.get(f); if (e) e.size = st.size; } catch {}
|
||||||
|
}));
|
||||||
}
|
}
|
||||||
|
|
||||||
this._startStatsTimer();
|
this._startStatsTimer();
|
||||||
@ -425,7 +426,7 @@ class UploadManager extends EventEmitter {
|
|||||||
if (cachedResult && typeof cachedResult.size === 'number' && cachedResult.size > 0) {
|
if (cachedResult && typeof cachedResult.size === 'number' && cachedResult.size > 0) {
|
||||||
fileSize = cachedResult.size;
|
fileSize = cachedResult.size;
|
||||||
} else {
|
} else {
|
||||||
try { fileSize = fs.statSync(task.file).size; } catch { fileNotFound = true; }
|
try { fileSize = (await fs.promises.stat(task.file)).size; } catch { fileNotFound = true; }
|
||||||
}
|
}
|
||||||
|
|
||||||
const maxAttempts = Math.max(1, (settings.retries || 0) + 1);
|
const maxAttempts = Math.max(1, (settings.retries || 0) + 1);
|
||||||
|
|||||||
@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "multi-hoster-uploader",
|
"name": "multi-hoster-uploader",
|
||||||
"version": "3.3.102",
|
"version": "3.3.103",
|
||||||
"description": "Upload files to doodstream, voe, vidmoly, byse simultaneously",
|
"description": "Upload files to doodstream, voe, vidmoly, byse simultaneously",
|
||||||
"main": "main.js",
|
"main": "main.js",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
|
|||||||
@ -1,3 +1,38 @@
|
|||||||
|
# v3.3.103 — kill the batch-start 336ms main stall (synchronous statSync storm)
|
||||||
|
|
||||||
|
v3.3.102 log (real 224-job batch): History virtualization CONFIRMED (no get-history on tab switch),
|
||||||
|
steady-state pristine (fps=32, ELD 11.8ms). Residual = a ~6s BATCH-START spin-up burst: main ELD
|
||||||
|
max=336ms @cpu=0%core + renderer-longtasks 196-391ms; settles to clean by +6s. Workflow w3bzkumo8
|
||||||
|
(4 agents + adversarial verify) CORRECTED my hypothesis:
|
||||||
|
- My "uncapped renderer progress-drain" theory was WRONG: handleProgress only mutates JS + SCHEDULES
|
||||||
|
coalesced renders (rAF/200ms); render is already one-per-frame; main coalesces to ~50 latest-per-job/100ms.
|
||||||
|
Chunking the drain fixes nothing. ADVERSARY FOUND IT WOULD REGRESS: rAF throttles to ~0 when the window is
|
||||||
|
minimized (the common background-uploader state) → unbounded _pBuf backlog AND deferred persistQueueStateSoon
|
||||||
|
(last line of _handleProgressImpl) → terminal 'done' lost on close = the queue-persistence-ghost-fix class.
|
||||||
|
DEFERRED/REJECTED as sketched.
|
||||||
|
- REAL cause (cpu=0%core = blocked on I/O): synchronous fs.statSync storm in UploadManager.startBatch dedup
|
||||||
|
loop (lib/upload-manager.js:363-377): up to DEDUP_CHUNK=200 fs.statSync in ONE tick before yielding. 200 ×
|
||||||
|
~1.68ms (measured on the VM) = the exact 336ms. Plus a per-job statSync (428). On a disk already saturated
|
||||||
|
by the 1MB read-ahead.
|
||||||
|
|
||||||
|
SHIPPED v3.3.103 (lib/upload-manager.js — adversary's zero-risk headline fix, but the more thorough async form):
|
||||||
|
- Dedup loop: dedup synchronously (cheap Map ops), then stat the unique files in PARALLEL via
|
||||||
|
`await Promise.all(toStat.map(f => fs.promises.stat(f)))` per chunk → stats run on the libuv threadpool,
|
||||||
|
main thread NEVER blocks. Preserves the exact results-Map shape {name,size,results:[]} + dedup semantics
|
||||||
|
(size=0 on failure). The 336ms sync block → 0 main-thread block.
|
||||||
|
- Per-job statSync (428) → `await fs.promises.stat` (in an async fn before the first real await; cachedResult
|
||||||
|
fast-path already skips it for ~all jobs — consistency only).
|
||||||
|
- Tests: updated the fs.statSync mocks in upload-manager.test.js (2 sites) + suspect-reject-alternates.test.js
|
||||||
|
to also mock fs.promises.stat (returns the same fake sizes). 407 tests pass, clean boot.
|
||||||
|
|
||||||
|
The renderer-longtasks (391/280/299ms) during the burst were (medium-confidence) the user's OWN tab clicks
|
||||||
|
landing while the main thread was stalled — fixing the main stall frees IPC so those clicks stay responsive.
|
||||||
|
DEFERRED still (only if needed): first-get-history-after-batch parse-cache/JSONL; the renderer chunked drain
|
||||||
|
ONLY if ever needed for interaction-responsiveness AND gated on a high-water-mark sync drain + a
|
||||||
|
document.hidden setTimeout fallback (never rAF-only).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
# v3.3.102 — virtualize the History table (the last tab-switch layout cost)
|
# v3.3.102 — virtualize the History table (the last tab-switch layout cost)
|
||||||
|
|
||||||
v3.3.101's gate killed the get-history PARSE on tab switch, but the v3.3.101 log showed a RESIDUAL: tab
|
v3.3.101's gate killed the get-history PARSE on tab switch, but the v3.3.101 log showed a RESIDUAL: tab
|
||||||
|
|||||||
@ -26,14 +26,20 @@ describe('suspect-reject alternate accounts', () => {
|
|||||||
fileProbe.probeFileHead = (...a) => mockProbe(...a);
|
fileProbe.probeFileHead = (...a) => mockProbe(...a);
|
||||||
|
|
||||||
const fs = require('fs');
|
const fs = require('fs');
|
||||||
|
const fakeSize = (p) => {
|
||||||
|
const m = /-(\d+)gb/i.exec(p);
|
||||||
|
return { size: (m ? parseInt(m[1], 10) : 3) * 1024 * 1024 * 1024 };
|
||||||
|
};
|
||||||
const origStatSync = fs.statSync;
|
const origStatSync = fs.statSync;
|
||||||
fs.statSync = function (p) {
|
fs.statSync = function (p) {
|
||||||
if (typeof p === 'string' && p.startsWith('/test/')) {
|
if (typeof p === 'string' && p.startsWith('/test/')) return fakeSize(p);
|
||||||
const m = /-(\d+)gb/i.exec(p);
|
|
||||||
return { size: (m ? parseInt(m[1], 10) : 3) * 1024 * 1024 * 1024 };
|
|
||||||
}
|
|
||||||
return origStatSync.call(this, p);
|
return origStatSync.call(this, p);
|
||||||
};
|
};
|
||||||
|
const origStat = fs.promises.stat;
|
||||||
|
fs.promises.stat = async function (p) {
|
||||||
|
if (typeof p === 'string' && p.startsWith('/test/')) return fakeSize(p);
|
||||||
|
return origStat.call(this, p);
|
||||||
|
};
|
||||||
|
|
||||||
UploadManager = require('../lib/upload-manager');
|
UploadManager = require('../lib/upload-manager');
|
||||||
});
|
});
|
||||||
|
|||||||
@ -33,7 +33,7 @@ describe('UploadManager', () => {
|
|||||||
hosters.uploadFile = mockUploadFile;
|
hosters.uploadFile = mockUploadFile;
|
||||||
hosters.prefetchBaseline = async () => null;
|
hosters.prefetchBaseline = async () => null;
|
||||||
|
|
||||||
// Mock fs.statSync for test file paths
|
// Mock fs.statSync + fs.promises.stat for test file paths
|
||||||
const fs = require('fs');
|
const fs = require('fs');
|
||||||
const origStatSync = fs.statSync;
|
const origStatSync = fs.statSync;
|
||||||
fs.statSync = function(p) {
|
fs.statSync = function(p) {
|
||||||
@ -42,6 +42,13 @@ describe('UploadManager', () => {
|
|||||||
}
|
}
|
||||||
return origStatSync.call(this, p);
|
return origStatSync.call(this, p);
|
||||||
};
|
};
|
||||||
|
const origStat = fs.promises.stat;
|
||||||
|
fs.promises.stat = async function(p) {
|
||||||
|
if (typeof p === 'string' && p.startsWith('/test/')) {
|
||||||
|
return { size: fakeFileSize };
|
||||||
|
}
|
||||||
|
return origStat.call(this, p);
|
||||||
|
};
|
||||||
|
|
||||||
UploadManager = require('../lib/upload-manager');
|
UploadManager = require('../lib/upload-manager');
|
||||||
});
|
});
|
||||||
@ -331,10 +338,10 @@ describe('UploadManager', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('file not found produces descriptive error', async () => {
|
it('file not found produces descriptive error', async () => {
|
||||||
// Override fs.statSync to throw ENOENT for a specific path
|
// Override fs.promises.stat to throw ENOENT for a specific path
|
||||||
const fs = require('fs');
|
const fs = require('fs');
|
||||||
const origStat = fs.statSync;
|
const origStat = fs.promises.stat;
|
||||||
fs.statSync = function(p) {
|
fs.promises.stat = async function(p) {
|
||||||
if (p === '/test/deleted.mp4') throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
|
if (p === '/test/deleted.mp4') throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
|
||||||
return origStat.call(this, p);
|
return origStat.call(this, p);
|
||||||
};
|
};
|
||||||
@ -347,7 +354,7 @@ describe('UploadManager', () => {
|
|||||||
{ file: '/test/deleted.mp4', hoster: 'doodstream.com', apiKey: 'key1' }
|
{ file: '/test/deleted.mp4', hoster: 'doodstream.com', apiKey: 'key1' }
|
||||||
]);
|
]);
|
||||||
|
|
||||||
fs.statSync = origStat;
|
fs.promises.stat = origStat;
|
||||||
assert.ok(errors.some(e => e.includes('nicht gefunden')), `expected "nicht gefunden" error, got: ${errors.join(', ')}`);
|
assert.ok(errors.some(e => e.includes('nicht gefunden')), `expected "nicht gefunden" error, got: ${errors.join(', ')}`);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user