15a4509ad5
Records the 18-agent line-by-line audit of every line written this session: the reported lag was v3.3.87 (recent-panel cliff); the audit found no second cause affecting this user. doodstream _debugLog sync-fs gated (shipped). config-store load()/serialize history-scaling costs are real but sub-ms at this user's scale and the fix is risky persistence surgery — deferred, documented with measurements. Lesson: "audit every line" = look + measure + risk-appropriate decision, NOT fix-everything; the load() perf win and its corruption risk are the same coin (shared batch refs), so there is no safe version — defer, don't ship. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
51 lines
4.4 KiB
Markdown
51 lines
4.4 KiB
Markdown
# Lag audit of every line written this session (Goal: "schau dir JEDE zeile an … ob es solche probleme gibt o. geben könnte")
|
||
|
||
Method: self-review of every hot-path line I changed (config-store T1, main.js T3, renderer append-evict,
|
||
diagnostics) + an 18-agent adversarial line-by-line audit (each finding double-verified for real + causes-
|
||
perceptible-lag) + Blink microbenchmarks. Reference config = THIS user's real one: 8 batches / 4.8 KB
|
||
history / 52 KB total — measured, not assumed.
|
||
|
||
## Conclusion: v3.3.87 was the fix. The rest is a clean bill of health, not a to-do list.
|
||
The reported "laggy after long runtime" was the recent-panel rebuild cliff — found, fixed (append-evict,
|
||
80 ms → 7.4 ms Blink-verified), released v3.3.87. The audit surfaced NO second cause that affects this user.
|
||
|
||
## SHIPPED this round (v3.3.88) — the one finding that bites in the real upload scenario
|
||
- doodstream-upload.js `_debugLog`: ran ungated SYNCHRONOUS statSync + appendFileSync (~8–15× per upload)
|
||
on the main-process event loop WHILE uploading — delaying IPC / progress-batch forwarding for every other
|
||
concurrent upload. The user runs doodstream, so this fired in practice. Fix: gate it behind the EXISTING
|
||
`globalSettings.logVerbose` setting (default off), mirroring main.js `logDebug`/`_logVerbose`; wired via the
|
||
single `setLogVerbose` chokepoint (boot + save + toggle). Near-zero risk (early-return when verbose off),
|
||
removes all per-upload sync fs in normal operation. 397/397 tests pass, lint clean, wiring verified (shared
|
||
module instance, default-off, toggles).
|
||
|
||
## DEFERRED — documented, conscious (real mechanisms, but wrong risk/reward to ship)
|
||
These are in MY T1 code (lib/config-store.js, commit 29d1944). They are REAL and scale with history size,
|
||
but at THIS user's scale (8 batches) they are tens of MICROSECONDS, on a path that fires ~once/20s. The
|
||
verifiers said "not urgent / negligible (~0.1% duty) / latent main-process hygiene, not a renderer-lag fix."
|
||
load() is the most dangerous code in the repo (config + credentials; corruption = data loss) and I was
|
||
already bitten once here by cache semantics. So: recorded, not shipped.
|
||
- F1 `_serializeForDisk` JSON.stringify(whole config incl. unbounded history, null,2) on every save +
|
||
copyFileSync of the growing file per commit. Scales with historySize. Root fix = split history into its own
|
||
file (history.json) so the ~20s queue-persist stops dragging history. Higher-risk persistence surgery.
|
||
- F2 `load()` deep-clones the WHOLE config (incl. history) on every call (even cache hit), and `_atomicWrite`
|
||
nulls the cache per write so write-interleaved loads are full misses. Measured @8000 batches: old read+parse
|
||
9.65 ms → new miss 22.96 ms (a 2.2–2.4× regression I introduced in T1) → new hit 12.94 ms. A "fix" via
|
||
shallow `history.slice()` is UNSAFE: the speedup comes only from sharing batch objects by reference, which
|
||
is exactly a silent-cache-corruption hazard that depends on a global "nothing ever deep-mutates a history
|
||
batch" invariant I can't enforce across future code + every getHistory consumer. Perf win and risk are the
|
||
same coin → no safe version → wrong tool for safety-critical code. The genuinely safe fixes (history-split,
|
||
bounded default retention) are out of scope (history-split = risk; bounded default = could drop user history).
|
||
- F3 (renderer, low) `_completedUploadKeys` grows unbounded per session, fully iterated in
|
||
buildPersistedQueueState on the ~20s persist. Sub-ms even for thousands of keys; the Set is the re-queue
|
||
dedup so capping it risks correctness. Not worth it.
|
||
- F4 (operator, low) diagnostics serverHealth does ~6–8 configStore.load() per request, each cloning history.
|
||
Cold path (only when diagnostics is queried), and largely a function of F2; defer with F2.
|
||
|
||
## Verified clean (no action)
|
||
- T1 structuredClone-per-load does NOT hit the upload hot path: shouldLogHosterToFile uses LIVE
|
||
uploadManager.hosterSettings during uploads (its load() fallback is unreachable mid-batch); the 8×/s log
|
||
flush uses the O(1) _getLogSettings cache (T3); remaining load() sites are user-triggered IPC / boot.
|
||
- T3 _getLogSettings: O(1) on cache hit, invalidated on every settings-write path.
|
||
- renderer append-evict (v3.3.87): no new per-frame cost; correctness re-verified (5000-completion sim).
|
||
- diagnostics renderer additions (0c6c502): all inside renderSettings() — cold, settings panel only.
|