perf(upgrade): apply delta patch chains in memory with cached base reads#1127
Conversation
Multi-patch delta upgrades previously wrote each intermediate binary to disk, copied it to a temp file, and recomputed its SHA-256 — N-1 redundant disk round-trips and hash passes per chain. Patch application now keeps intermediates in memory and only persists and hashes the final binary.
The base ("old") binary is read on demand via positional reads (pread) through an OldReader abstraction instead of being loaded whole into the JS heap, dropping the single-patch peak heap from ~100 MB to ~KB. A bounded 1 MiB read-ahead cache coalesces bsdiff's many small windowed reads into a handful of block reads, avoiding a per-window syscall storm. A redundant Uint8Array copy of the base is also dropped.
- bspatch.ts: extract transformPatch core; add OldReader (fd-backed pread with block cache + in-memory) and applyPatchChainInMemory; applyPatch is now a thin wrapper
- delta-upgrade.ts: applyPatchChain delegates to applyPatchChainInMemory; remove disk-intermediate alternation and scratch-file cleanup
- tests: multi-hop chain, memory/pread equivalence, block-cache boundary coverage, no-intermediate-files assertions, e2e two-hop chain
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 348a7fe. Configure here.
| // Data is in JS heap — no temp file to clean up | ||
| }, | ||
| }; | ||
| return new MemoryOldReader(await readFile(oldPath)); |
There was a problem hiding this comment.
Temp copy discarded on open failure
Medium Severity
After a successful CoW copy to the temp path, if open or stat fails, the catch block unlinks that temp file and loads the base via readFile(oldPath). The previous path always read the temp copy when copy succeeded, which avoids reading the running executable and matches the ETXTBSY/AMFI rationale.
Reviewed by Cursor Bugbot for commit 348a7fe. Configure here.
There was a problem hiding this comment.
Fixed in d0572ec. The catch now closes the open FileHandle (tracked outside the try) before falling back to the in-memory read of the original, so the fd is no longer leaked and the temp copy is still unlinked.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Codecov Results 📊✅ Patch coverage is 89.11%. Project has 5051 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.31% 81.33% +0.02%
==========================================
Files 392 392 —
Lines 27009 27061 +52
Branches 17545 17558 +13
==========================================
+ Hits 21963 22010 +47
- Misses 5046 5051 +5
- Partials 1824 1832 +8Generated by Codecov Action |
If open() succeeded but a later step (stat) threw, the catch block fell back to an in-memory read without closing the already-open FileHandle, leaking a file descriptor. Track the handle outside the try and close it in the catch before falling back. Flagged by Cursor Bugbot and Seer on #1127.
1 participant
Why
Delta upgrades that span multiple releases were slower than they should be. The patch-application path wrote every intermediate binary to disk, copied it to a temp file, and recomputed its SHA-256 —
N-1redundant disk round-trips and full-binary hash passes per chain — even though only the final binary needs verifying. The base binary was also loaded whole (~100 MB) into the JS heap.What changed
Three layered improvements to patch application, all behind the existing
applyPatchChainAPI (no caller changes):In-memory patch chains. Intermediate hops are kept in memory and fed straight into the next patch; only the final binary is streamed to disk and hashed. No more
.patching.a/.bscratch files, per-hop temp copies, or discarded intermediate hashes. Because reads and writes never target the same path, the read/write truncation hazard is gone too.On-demand base reads (
pread). The base ("old") binary is read through a newOldReaderabstraction instead of being loaded whole into the heap.FileOldReaderuses positional reads against the reflink temp copy; intermediates use a cachelessMemoryOldReader. Single-patch peak heap drops from ~100 MB to ~KB. A redundantnew Uint8Array(readFile())copy of the base is also dropped.Bounded read-ahead cache.
FileOldReaderkeeps a single reused 1 MiB block, refilling only on a miss, with a direct-read bypass for windows larger than the block. This coalesces bsdiff's many small windowed reads (mostly forward) into ~fileSize / 1 MiBreads — neutralizing the per-window syscall overhead that on-demand reads would otherwise add to the common single-patch path. Memory stays bounded at ~1 MiB, and only for the first hop.Files
src/lib/bspatch.ts— extractedtransformPatchcore (callback sink); addedOldReader(FileOldReaderwith block cache +MemoryOldReader),applyPatchToMemory, andapplyPatchChainInMemory;applyPatchis now a thin wrapper.src/lib/delta-upgrade.ts—applyPatchChaindelegates toapplyPatchChainInMemory; removed the disk-intermediate alternation and scratch-file cleanup.Testing
zstdCompressSync, no externalzig-bsdiffneeded) — exercises both reader paths and the chaining loop; asserts no scratch files on disk.MemoryOldReadervs the cachedFileOldReaderagainst the real fixture patch (diff + extra + seek).delta-upgrade.test.ts— rewrote the multi-step test to assert intermediates never touch disk.old → new → new) usingzig-bsdiff.blockCoversrange check is broken.pnpm run typecheck,pnpm run lint, and the affected suites (bspatch, delta-upgrade, upgrade — 233 tests) all pass.Notes