From 7f636258d4d9d6fec2b4e3f5158b151129265a9c Mon Sep 17 00:00:00 2001 From: Administrator Date: Sun, 21 Jun 2026 20:53:48 +0200 Subject: [PATCH] =?UTF-8?q?perf(uploads):=20make=20the=20batch-start=20fil?= =?UTF-8?q?e-stat=20non-blocking=20=E2=80=94=20kill=20the=20336ms=20main?= =?UTF-8?q?=20stall=20(v3.3.103)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- lib/upload-manager.js | 13 ++++----- package.json | 2 +- tasks/todo.md | 35 +++++++++++++++++++++++++ tests/suspect-reject-alternates.test.js | 14 +++++++--- tests/upload-manager.test.js | 17 ++++++++---- 5 files changed, 65 insertions(+), 16 deletions(-) diff --git a/lib/upload-manager.js b/lib/upload-manager.js index fc5fb9d..c95f1aa 100644 --- a/lib/upload-manager.js +++ b/lib/upload-manager.js @@ -364,16 +364,17 @@ class UploadManager extends EventEmitter { for (let i = 0; i < tasks.length; i += DEDUP_CHUNK) { if (signal.aborted) break; const end = Math.min(i + DEDUP_CHUNK, tasks.length); + const toStat = []; for (let j = i; j < end; j++) { const task = tasks[j]; if (!results.has(task.file)) { - const fileName = path.basename(task.file); - let size = 0; - try { size = fs.statSync(task.file).size; } catch {} - results.set(task.file, { name: fileName, size, results: [] }); + results.set(task.file, { name: path.basename(task.file), size: 0, results: [] }); + toStat.push(task.file); } } - 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(); @@ -425,7 +426,7 @@ class UploadManager extends EventEmitter { if (cachedResult && typeof cachedResult.size === 'number' && cachedResult.size > 0) { fileSize = cachedResult.size; } 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); diff --git a/package.json b/package.json index 1df1430..d8ecd46 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "multi-hoster-uploader", - "version": "3.3.102", + "version": "3.3.103", "description": "Upload files to doodstream, voe, vidmoly, byse simultaneously", "main": "main.js", "scripts": { diff --git a/tasks/todo.md b/tasks/todo.md index 4139559..817434c 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -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.101's gate killed the get-history PARSE on tab switch, but the v3.3.101 log showed a RESIDUAL: tab diff --git a/tests/suspect-reject-alternates.test.js b/tests/suspect-reject-alternates.test.js index 81e860a..75139ea 100644 --- a/tests/suspect-reject-alternates.test.js +++ b/tests/suspect-reject-alternates.test.js @@ -26,14 +26,20 @@ describe('suspect-reject alternate accounts', () => { fileProbe.probeFileHead = (...a) => mockProbe(...a); 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; fs.statSync = function (p) { - if (typeof p === 'string' && p.startsWith('/test/')) { - const m = /-(\d+)gb/i.exec(p); - return { size: (m ? parseInt(m[1], 10) : 3) * 1024 * 1024 * 1024 }; - } + if (typeof p === 'string' && p.startsWith('/test/')) return fakeSize(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'); }); diff --git a/tests/upload-manager.test.js b/tests/upload-manager.test.js index ed454c6..d37d617 100644 --- a/tests/upload-manager.test.js +++ b/tests/upload-manager.test.js @@ -33,7 +33,7 @@ describe('UploadManager', () => { hosters.uploadFile = mockUploadFile; 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 origStatSync = fs.statSync; fs.statSync = function(p) { @@ -42,6 +42,13 @@ describe('UploadManager', () => { } 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'); }); @@ -331,10 +338,10 @@ describe('UploadManager', () => { }); 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 origStat = fs.statSync; - fs.statSync = function(p) { + const origStat = fs.promises.stat; + fs.promises.stat = async function(p) { if (p === '/test/deleted.mp4') throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); return origStat.call(this, p); }; @@ -347,7 +354,7 @@ describe('UploadManager', () => { { 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(', ')}`); });