test(coalesce): extract done-removal coalescer + 11 unit tests
The microtask-coalesce path from 3.3.1 (queueMicrotask + Set so 500
finishing jobs become one queueJobs.filter pass instead of 500) lived
inline in renderer/app.js. Pulled out into lib/coalesced-set.js with
an injectable scheduler so a Node test can drive timing without
async waits.
API: makeCoalescedSet({ apply, scheduler? }) returns
add(id) — queue an id for the next batch
drainSync() — flush synchronously (used by beforeunload)
pendingSize() — diagnostics
isScheduled() — diagnostics
Renderer rewires the previous _pendingDoneRemovalIds + manual
queueMicrotask plumbing to the new helper. Optional-chained: if the
script fails to load, a slower per-event filter runs as fallback.
Coverage:
- multiple adds same tick → 1 apply, all ids deduped
- duplicate ids deduped
- batches between flushes stay independent
- add after flush re-schedules
- drainSync flushes synchronously, queued microtask becomes a no-op
- empty drainSync is a no-op
- throwing apply doesn't lock out subsequent batches
- default scheduler (queueMicrotask) runs eventually
- 5000-id burst still coalesces to 1 apply
137/137 green.
This commit is contained in:
parent
f5256c437f
commit
166a49dd0c
73
lib/coalesced-set.js
Normal file
73
lib/coalesced-set.js
Normal file
@ -0,0 +1,73 @@
|
|||||||
|
// Microtask-coalesced set. Adds are O(1); the apply callback runs once per
|
||||||
|
// scheduler tick with every id collected since the last flush.
|
||||||
|
//
|
||||||
|
// Used by the renderer to merge a burst of done-jobs (e.g. 500 jobs all
|
||||||
|
// finishing within milliseconds) into a single queueJobs.filter() pass —
|
||||||
|
// without this each event was its own O(N) sweep, so 500 finishes were
|
||||||
|
// O(N²) and visibly froze the UI on completion.
|
||||||
|
//
|
||||||
|
// Loaded both as a CommonJS module (Node tests) and as a browser global
|
||||||
|
// (renderer/app.js via index.html script tag).
|
||||||
|
|
||||||
|
(function (root) {
|
||||||
|
'use strict';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Build a coalesced set.
|
||||||
|
* @param {{ apply: (Set) => void, scheduler?: (cb: () => void) => void }} opts
|
||||||
|
* apply: called once per scheduler tick with the accumulated ids.
|
||||||
|
* scheduler: defaults to queueMicrotask. Tests can pass a synchronous
|
||||||
|
* stand-in to avoid async waits.
|
||||||
|
*/
|
||||||
|
function makeCoalescedSet(opts) {
|
||||||
|
if (!opts || typeof opts.apply !== 'function') {
|
||||||
|
throw new TypeError('makeCoalescedSet: { apply: fn } required');
|
||||||
|
}
|
||||||
|
const apply = opts.apply;
|
||||||
|
const scheduler = typeof opts.scheduler === 'function'
|
||||||
|
? opts.scheduler
|
||||||
|
: (typeof queueMicrotask === 'function' ? queueMicrotask : (cb) => Promise.resolve().then(cb));
|
||||||
|
let pending = new Set();
|
||||||
|
let scheduled = false;
|
||||||
|
|
||||||
|
function flush() {
|
||||||
|
scheduled = false;
|
||||||
|
if (pending.size === 0) return;
|
||||||
|
const drop = pending;
|
||||||
|
pending = new Set();
|
||||||
|
try { apply(drop); } catch (e) {
|
||||||
|
// Don't let a failing apply lock out the next batch — surface it
|
||||||
|
// but keep the coalescer usable.
|
||||||
|
if (typeof console !== 'undefined' && console.error) console.error(e);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
add(id) {
|
||||||
|
pending.add(id);
|
||||||
|
if (!scheduled) {
|
||||||
|
scheduled = true;
|
||||||
|
scheduler(flush);
|
||||||
|
}
|
||||||
|
},
|
||||||
|
/**
|
||||||
|
* Synchronously consume any pending ids. Used by beforeunload paths
|
||||||
|
* where we can't wait for the next microtask before persisting.
|
||||||
|
*/
|
||||||
|
drainSync() {
|
||||||
|
if (pending.size === 0) return;
|
||||||
|
const drop = pending;
|
||||||
|
pending = new Set();
|
||||||
|
scheduled = false;
|
||||||
|
apply(drop);
|
||||||
|
},
|
||||||
|
/** Introspection for tests + diagnostics. */
|
||||||
|
pendingSize() { return pending.size; },
|
||||||
|
isScheduled() { return scheduled; }
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
const api = { makeCoalescedSet };
|
||||||
|
if (typeof module !== 'undefined' && module.exports) module.exports = api;
|
||||||
|
else if (root) root.CoalescedSet = api;
|
||||||
|
})(typeof window !== 'undefined' ? window : this);
|
||||||
@ -48,9 +48,16 @@ const _sessionDoneJobs = new Set(); // Job IDs already counted for uploadedBytes
|
|||||||
const _completedUploadKeys = new Set(); // 'filepath|hoster' keys for done uploads (survives removeFromQueueOnDone)
|
const _completedUploadKeys = new Set(); // 'filepath|hoster' keys for done uploads (survives removeFromQueueOnDone)
|
||||||
const _deletedJobIds = new Set(); // IDs of jobs explicitly deleted by user (prevents re-creation from stale progress callbacks)
|
const _deletedJobIds = new Set(); // IDs of jobs explicitly deleted by user (prevents re-creation from stale progress callbacks)
|
||||||
// Coalesce removeFromQueueOnDone removals into one filter pass per microtask
|
// Coalesce removeFromQueueOnDone removals into one filter pass per microtask
|
||||||
// to avoid O(N²) behaviour when a burst of jobs finish at once.
|
// to avoid O(N²) behaviour when a burst of jobs finish at once. Logic now
|
||||||
let _pendingDoneRemovalIds = new Set();
|
// lives in lib/coalesced-set.js so it can be unit-tested with a manual
|
||||||
let _doneRemovalScheduled = false;
|
// scheduler. Optional-chained so the renderer still works if the script
|
||||||
|
// failed to load — falls back to immediate per-event filter (legacy slow
|
||||||
|
// path), better than crashing.
|
||||||
|
const _doneRemovalCoalescer = window.CoalescedSet
|
||||||
|
? window.CoalescedSet.makeCoalescedSet({
|
||||||
|
apply: (drop) => { queueJobs = queueJobs.filter(j => !drop.has(j.id)); }
|
||||||
|
})
|
||||||
|
: null;
|
||||||
const queueSortState = { key: 'filename', direction: 'asc' };
|
const queueSortState = { key: 'filename', direction: 'asc' };
|
||||||
|
|
||||||
// History state
|
// History state
|
||||||
@ -1950,16 +1957,11 @@ function handleProgress(data) {
|
|||||||
if (job.status === 'done' && config.globalSettings && config.globalSettings.removeFromQueueOnDone) {
|
if (job.status === 'done' && config.globalSettings && config.globalSettings.removeFromQueueOnDone) {
|
||||||
removeJobFromIndex(job);
|
removeJobFromIndex(job);
|
||||||
selectedJobIds.delete(job.id);
|
selectedJobIds.delete(job.id);
|
||||||
_pendingDoneRemovalIds.add(job.id);
|
if (_doneRemovalCoalescer) {
|
||||||
if (!_doneRemovalScheduled) {
|
_doneRemovalCoalescer.add(job.id);
|
||||||
_doneRemovalScheduled = true;
|
} else {
|
||||||
queueMicrotask(() => {
|
// Legacy slow path: immediate filter when the lib script didn't load.
|
||||||
_doneRemovalScheduled = false;
|
queueJobs = queueJobs.filter(j => j !== job);
|
||||||
if (_pendingDoneRemovalIds.size === 0) return;
|
|
||||||
const drop = _pendingDoneRemovalIds;
|
|
||||||
_pendingDoneRemovalIds = new Set();
|
|
||||||
queueJobs = queueJobs.filter(j => !drop.has(j.id));
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -3717,11 +3719,7 @@ window.addEventListener('beforeunload', () => {
|
|||||||
// Drain pending done-removals synchronously before persisting so jobs the
|
// Drain pending done-removals synchronously before persisting so jobs the
|
||||||
// user expected to disappear (removeFromQueueOnDone=true) don't reappear
|
// user expected to disappear (removeFromQueueOnDone=true) don't reappear
|
||||||
// on next launch. Microtask wouldn't run before the sync IPC below.
|
// on next launch. Microtask wouldn't run before the sync IPC below.
|
||||||
if (_pendingDoneRemovalIds.size > 0) {
|
if (_doneRemovalCoalescer) _doneRemovalCoalescer.drainSync();
|
||||||
const drop = _pendingDoneRemovalIds;
|
|
||||||
_pendingDoneRemovalIds = new Set();
|
|
||||||
queueJobs = queueJobs.filter(j => !drop.has(j.id));
|
|
||||||
}
|
|
||||||
const globalSettings = {
|
const globalSettings = {
|
||||||
...(config.globalSettings || {}),
|
...(config.globalSettings || {}),
|
||||||
pendingQueue: buildPersistedQueueState()
|
pendingQueue: buildPersistedQueueState()
|
||||||
|
|||||||
@ -332,6 +332,7 @@
|
|||||||
|
|
||||||
<script src="../lib/queue-prune.js"></script>
|
<script src="../lib/queue-prune.js"></script>
|
||||||
<script src="../lib/throttled-cache.js"></script>
|
<script src="../lib/throttled-cache.js"></script>
|
||||||
|
<script src="../lib/coalesced-set.js"></script>
|
||||||
<script src="app.js"></script>
|
<script src="app.js"></script>
|
||||||
</body>
|
</body>
|
||||||
</html>
|
</html>
|
||||||
|
|||||||
@ -18,14 +18,15 @@
|
|||||||
- ✅ 3.3.14 — Parser-null-payload guard: `uploadFile` normalisiert payload zu `{}` falls `JSON.parse('null')` o.ä.; `parseDoodstreamResult` + `parseByseResult` haben defensive guards für direct callers + 7 neue Unit-Tests (null/non-object, malformed entries, fileRejected/accountError flips, valid filecode happy path)
|
- ✅ 3.3.14 — Parser-null-payload guard: `uploadFile` normalisiert payload zu `{}` falls `JSON.parse('null')` o.ä.; `parseDoodstreamResult` + `parseByseResult` haben defensive guards für direct callers + 7 neue Unit-Tests (null/non-object, malformed entries, fileRejected/accountError flips, valid filecode happy path)
|
||||||
- ✅ 3.3.15 — Cancellation latency fix: nach `_sleep(800)` in der rotation-while-loop wird `signal.aborted`/`stopAfterActive` re-checkt bevor das ganze override-resolution-Setup läuft (deep-audit MED-5)
|
- ✅ 3.3.15 — Cancellation latency fix: nach `_sleep(800)` in der rotation-while-loop wird `signal.aborted`/`stopAfterActive` re-checkt bevor das ganze override-resolution-Setup läuft (deep-audit MED-5)
|
||||||
- ✅ 3.3.16 — Auto-Rotation für die anderen 3 internen Logs (`upload-debug.log` 25 MB, `account-rotation.log` 10 MB, `doodstream-debug.log` 10 MB), je 2 Backups — alle nutzen `lib/log-rotation.js` (zuvor nur `fileuploader.log` rotiert)
|
- ✅ 3.3.16 — Auto-Rotation für die anderen 3 internen Logs (`upload-debug.log` 25 MB, `account-rotation.log` 10 MB, `doodstream-debug.log` 10 MB), je 2 Backups — alle nutzen `lib/log-rotation.js` (zuvor nur `fileuploader.log` rotiert)
|
||||||
|
- ✅ 3.3.17 — `npm audit fix --force` (User authorized): `electron-builder 25 → 26`, `electron 33 → 41`, alle 12 verbleibenden Vulns geschlossen (12 → 0). Build verifiziert (NSIS+portable laufen mit electron 41), 126/126 grün.
|
||||||
|
- ✅ 3.3.18 — Microtask-Coalescer extrahiert nach `lib/coalesced-set.js` mit injectable scheduler (für Tests) + 11 Unit-Tests (single/multi-add coalesce, dedup, sequential batches, drainSync vs scheduler-noop, throwing-apply-recovery, 5000-burst). 137/137 grün.
|
||||||
|
|
||||||
## Open items (priorisiert)
|
## Open items (priorisiert)
|
||||||
|
|
||||||
(alle stabilitäts-items aus deep-audit erledigt)
|
(alle stabilitäts-items aus deep-audit erledigt)
|
||||||
|
|
||||||
### Code-Qualität (deferred)
|
### Code-Qualität
|
||||||
- [ ] removeFromQueueOnDone microtask-coalesce (3.3.1) — Microtask-Timing schwer zu testen ohne fake-timer setup
|
(alle erledigt)
|
||||||
- [ ] 12 weitere Vulnerabilities (10 high, 2 low) in electron-builder dev-chain — bräuchten `npm audit fix --force` mit Major-Bump electron-builder@26.8.1 (breaking). Skip bis User explizit ein Major-Update erlaubt.
|
|
||||||
|
|
||||||
### Loop-Status
|
### Loop-Status
|
||||||
Alle initial im 3.3.0-Audit identifizierten Items sind nun adressiert. Beide verbliebenen open items sind explizit deferred (microtask-fake-timer-Setup ist Refactor, audit-fix --force ist Major-Bump und braucht User-OK).
|
Alle initial im 3.3.0-Audit identifizierten Items sind nun adressiert. Beide verbliebenen open items sind explizit deferred (microtask-fake-timer-Setup ist Refactor, audit-fix --force ist Major-Bump und braucht User-OK).
|
||||||
|
|||||||
144
tests/coalesced-set.test.js
Normal file
144
tests/coalesced-set.test.js
Normal file
@ -0,0 +1,144 @@
|
|||||||
|
const { test } = require('node:test');
|
||||||
|
const assert = require('node:assert/strict');
|
||||||
|
|
||||||
|
const { makeCoalescedSet } = require('../lib/coalesced-set');
|
||||||
|
|
||||||
|
// Synchronous scheduler stand-in: collects callbacks instead of running
|
||||||
|
// them, so tests can drive the timing explicitly.
|
||||||
|
function makeManualScheduler() {
|
||||||
|
const queue = [];
|
||||||
|
const fn = (cb) => queue.push(cb);
|
||||||
|
fn.flush = () => {
|
||||||
|
while (queue.length) {
|
||||||
|
const cb = queue.shift();
|
||||||
|
cb();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
fn.queueLength = () => queue.length;
|
||||||
|
return fn;
|
||||||
|
}
|
||||||
|
|
||||||
|
test('throws if apply callback missing', () => {
|
||||||
|
assert.throws(() => makeCoalescedSet());
|
||||||
|
assert.throws(() => makeCoalescedSet({}));
|
||||||
|
assert.throws(() => makeCoalescedSet({ apply: 'not-a-fn' }));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('multiple adds in one tick coalesce into one apply call', () => {
|
||||||
|
const sched = makeManualScheduler();
|
||||||
|
const calls = [];
|
||||||
|
const cs = makeCoalescedSet({
|
||||||
|
apply: (drop) => calls.push([...drop].sort()),
|
||||||
|
scheduler: sched
|
||||||
|
});
|
||||||
|
|
||||||
|
cs.add('a'); cs.add('b'); cs.add('c');
|
||||||
|
assert.equal(sched.queueLength(), 1, 'only one microtask scheduled');
|
||||||
|
assert.equal(cs.pendingSize(), 3);
|
||||||
|
|
||||||
|
sched.flush();
|
||||||
|
assert.deepEqual(calls, [['a', 'b', 'c']]);
|
||||||
|
assert.equal(cs.pendingSize(), 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('duplicate adds are deduplicated', () => {
|
||||||
|
const sched = makeManualScheduler();
|
||||||
|
const calls = [];
|
||||||
|
const cs = makeCoalescedSet({ apply: (d) => calls.push([...d]), scheduler: sched });
|
||||||
|
cs.add('a'); cs.add('a'); cs.add('a');
|
||||||
|
sched.flush();
|
||||||
|
assert.deepEqual(calls, [['a']]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('two batches in series stay independent', () => {
|
||||||
|
const sched = makeManualScheduler();
|
||||||
|
const calls = [];
|
||||||
|
const cs = makeCoalescedSet({ apply: (d) => calls.push([...d]), scheduler: sched });
|
||||||
|
|
||||||
|
cs.add('x'); cs.add('y');
|
||||||
|
sched.flush();
|
||||||
|
cs.add('z');
|
||||||
|
sched.flush();
|
||||||
|
|
||||||
|
assert.deepEqual(calls, [['x', 'y'], ['z']]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('add after flush re-schedules a new microtask', () => {
|
||||||
|
const sched = makeManualScheduler();
|
||||||
|
const cs = makeCoalescedSet({ apply: () => {}, scheduler: sched });
|
||||||
|
cs.add('a');
|
||||||
|
assert.equal(sched.queueLength(), 1);
|
||||||
|
sched.flush();
|
||||||
|
assert.equal(sched.queueLength(), 0);
|
||||||
|
assert.equal(cs.isScheduled(), false);
|
||||||
|
cs.add('b');
|
||||||
|
assert.equal(sched.queueLength(), 1, 'new add → new microtask');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('drainSync flushes synchronously without waiting for scheduler', () => {
|
||||||
|
const sched = makeManualScheduler();
|
||||||
|
const calls = [];
|
||||||
|
const cs = makeCoalescedSet({ apply: (d) => calls.push([...d]), scheduler: sched });
|
||||||
|
cs.add('p'); cs.add('q');
|
||||||
|
cs.drainSync();
|
||||||
|
assert.deepEqual(calls, [['p', 'q']]);
|
||||||
|
assert.equal(cs.pendingSize(), 0);
|
||||||
|
|
||||||
|
// Pending microtask was for the same ids — when it runs, pending is empty
|
||||||
|
// → apply NOT called twice.
|
||||||
|
sched.flush();
|
||||||
|
assert.equal(calls.length, 1, 'queued microtask is a no-op after drainSync');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('drainSync on empty set is a no-op', () => {
|
||||||
|
let called = 0;
|
||||||
|
const cs = makeCoalescedSet({ apply: () => called++ });
|
||||||
|
cs.drainSync();
|
||||||
|
assert.equal(called, 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('throwing apply does not lock out subsequent batches', () => {
|
||||||
|
const sched = makeManualScheduler();
|
||||||
|
let attempt = 0;
|
||||||
|
const cs = makeCoalescedSet({
|
||||||
|
apply: () => { attempt++; if (attempt === 1) throw new Error('boom'); },
|
||||||
|
scheduler: sched
|
||||||
|
});
|
||||||
|
cs.add('a');
|
||||||
|
// First flush throws inside apply but is swallowed; coalescer must still work.
|
||||||
|
sched.flush();
|
||||||
|
cs.add('b');
|
||||||
|
sched.flush();
|
||||||
|
assert.equal(attempt, 2, 'second batch still ran despite first throwing');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('default scheduler is queueMicrotask (or Promise fallback) — runs eventually', async () => {
|
||||||
|
const calls = [];
|
||||||
|
const cs = makeCoalescedSet({ apply: (d) => calls.push([...d]) });
|
||||||
|
cs.add('z');
|
||||||
|
// Wait one microtask
|
||||||
|
await Promise.resolve();
|
||||||
|
assert.deepEqual(calls, [['z']]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('no-op tick: scheduler fires while pending is empty (e.g. drained)', () => {
|
||||||
|
const sched = makeManualScheduler();
|
||||||
|
let called = 0;
|
||||||
|
const cs = makeCoalescedSet({ apply: () => called++, scheduler: sched });
|
||||||
|
cs.add('a');
|
||||||
|
cs.drainSync();
|
||||||
|
assert.equal(called, 1);
|
||||||
|
// Pending microtask still in queue → flush; pending is empty → apply NOT called again.
|
||||||
|
sched.flush();
|
||||||
|
assert.equal(called, 1);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('large burst of 5000 adds coalesces to one apply call', () => {
|
||||||
|
const sched = makeManualScheduler();
|
||||||
|
const calls = [];
|
||||||
|
const cs = makeCoalescedSet({ apply: (d) => calls.push(d.size), scheduler: sched });
|
||||||
|
for (let i = 0; i < 5000; i++) cs.add('id-' + i);
|
||||||
|
assert.equal(sched.queueLength(), 1);
|
||||||
|
sched.flush();
|
||||||
|
assert.deepEqual(calls, [5000]);
|
||||||
|
});
|
||||||
Loading…
Reference in New Issue
Block a user