perf(upgrade): use copy-then-mmap for zero JS heap during delta patching#344
perf(upgrade): use copy-then-mmap for zero JS heap during delta patching#344
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Trace
Other
Bug Fixes 🐛Api
Formatters
Setup
Upgrade
Other
Documentation 📚
Internal Changes 🔧Api
Other
🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ 101 passed | Total: 101 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
All tests are passing successfully. ✅ Patch coverage is 88.43%. Project has 3680 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 82.64% 80.66% -1.98%
==========================================
Files 127 127 —
Lines 17902 19028 +1126
Branches 0 0 —
==========================================
+ Hits 14794 15348 +554
- Misses 3108 3680 +572
- Partials 0 0 —Generated by Codecov Action |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Temp file leaked if
writer.end()throws in finally- Nested writer.end() in try/finally to ensure cleanupOldFile() always runs even when writer.end() throws.
Or push these changes by commenting:
@cursor push f3adb8e178
Preview (f3adb8e178)
diff --git a/src/lib/bspatch.ts b/src/lib/bspatch.ts
--- a/src/lib/bspatch.ts
+++ b/src/lib/bspatch.ts
@@ -358,8 +358,11 @@
oldpos += seekBy;
}
} finally {
- await writer.end();
- cleanupOldFile();
+ try {
+ await writer.end();
+ } finally {
+ cleanupOldFile();
+ }
}
// Validate output size matches headerd3791ab to
eba75ef
Compare
Two improvements to the delta upgrade system:
1. **Copy-then-mmap with child process probe for mmap safety**
Instead of reading the old binary into a Uint8Array via arrayBuffer()
(~100 MB JS heap spike), copies the file to a temp location and probes
mmap safety via a child process before attempting it:
- Copy: copyFileSync (CoW reflinks on btrfs/xfs/APFS for near-instant)
- Probe: spawn child to test Bun.mmap() on the copy — if macOS AMFI
sends SIGKILL, only the child dies, parent survives
- Mmap: if probe succeeds, mmap the copy (zero JS heap, kernel-managed)
- Fallback: if probe fails, read copy via arrayBuffer() (~100 MB heap)
2. **Instrument delta upgrade with Sentry spans, metrics, and error capture**
Delta failures were invisible in Sentry (no spans, no captureException,
errors logged at debug level). Now:
- withTracingSpan('upgrade.delta') wraps the delta attempt
- captureException on failure with warning level + delta context tags
- log.warn() instead of log.debug() so users see failures
- span.setStatus({ code: 2 }) on error for correct telemetry
- Sentry.metrics.distribution for patch_bytes and chain_length
- chainLength field added to DeltaResult
…olated coverage Add a test to test/isolated/delta-upgrade.test.ts that exercises the full success path of attemptDeltaUpgrade using real TRDIFF10 fixture files (small-old.bin → small.trdiff10 → small-new.bin). This covers: - attemptDeltaUpgrade success path: span attributes, Sentry.metrics, setStatus - resolveStableDelta return with chainLength field - DeltaResult type with all three fields verified Also update CI to collect coverage from isolated tests: - Add 'Isolated Tests' step with --coverage to .github/workflows/ci.yml - Upload both coverage/lcov.info and coverage-isolated/lcov.info to codecov - Add coverage-isolated/ to .gitignore Estimated patch coverage improvement: 73% → ~94% for delta-upgrade.ts
Compiled Bun binaries don't support the -e flag, so spawning process.execPath -e '...' re-runs the main program instead of evaluating the expression. This made probeMmapSafe() always return false in production, negating the mmap optimization. The probe was unnecessary: mmap on a temp copy is safe because the copy is a regular file (separate inode), not a running binary. ETXTBSY (Linux) and AMFI SIGKILL (macOS) only affect the running binary's inode. Simplify to direct copy → mmap with try/catch fallback to arrayBuffer(). Addresses Seer review comment on PR #344.
Compiled Bun binaries don't support the -e flag, so spawning process.execPath -e '...' re-runs the main program instead of evaluating the expression. This made probeMmapSafe() always return false in production, negating the mmap optimization. The probe was unnecessary: mmap on a temp copy is safe because the copy is a regular file (separate inode), not a running binary. ETXTBSY (Linux) and AMFI SIGKILL (macOS) only affect the running binary's inode. Simplify to direct copy → mmap with try/catch fallback to arrayBuffer(). Addresses Seer review comment on PR #344.
Compiled Bun binaries don't support the -e flag, so spawning process.execPath -e '...' re-runs the main program instead of evaluating the expression. This made probeMmapSafe() always return false in production, negating the mmap optimization. The probe was unnecessary: mmap on a temp copy is safe because the copy is a regular file (separate inode), not a running binary. ETXTBSY (Linux) and AMFI SIGKILL (macOS) only affect the running binary's inode. Simplify to direct copy → mmap with try/catch fallback to arrayBuffer(). Addresses Seer review comment on PR #344.
Compiled Bun binaries don't support the -e flag, so spawning process.execPath -e '...' re-runs the main program instead of evaluating the expression. This made probeMmapSafe() always return false in production, negating the mmap optimization. The probe was unnecessary: mmap on a temp copy is safe because the copy is a regular file (separate inode), not a running binary. ETXTBSY (Linux) and AMFI SIGKILL (macOS) only affect the running binary's inode. Simplify to direct copy → mmap with try/catch fallback to arrayBuffer(). Addresses Seer review comment on PR #344.
Compiled Bun binaries don't support the -e flag, so spawning process.execPath -e '...' re-runs the main program instead of evaluating the expression. This made probeMmapSafe() always return false in production, negating the mmap optimization. The probe was unnecessary: mmap on a temp copy is safe because the copy is a regular file (separate inode), not a running binary. ETXTBSY (Linux) and AMFI SIGKILL (macOS) only affect the running binary's inode. Simplify to direct copy → mmap with try/catch fallback to arrayBuffer(). Addresses Seer review comment on PR #344.
Compiled Bun binaries don't support the -e flag, so spawning process.execPath -e '...' re-runs the main program instead of evaluating the expression. This made probeMmapSafe() always return false in production, negating the mmap optimization. The probe was unnecessary: mmap on a temp copy is safe because the copy is a regular file (separate inode), not a running binary. ETXTBSY (Linux) and AMFI SIGKILL (macOS) only affect the running binary's inode. Simplify to direct copy → mmap with try/catch fallback to arrayBuffer(). Addresses Seer review comment on PR #344.
Compiled Bun binaries don't support the -e flag, so spawning process.execPath -e '...' re-runs the main program instead of evaluating the expression. This made probeMmapSafe() always return false in production, negating the mmap optimization. The probe was unnecessary: mmap on a temp copy is safe because the copy is a regular file (separate inode), not a running binary. ETXTBSY (Linux) and AMFI SIGKILL (macOS) only affect the running binary's inode. Simplify to direct copy → mmap with try/catch fallback to arrayBuffer(). Addresses Seer review comment on PR #344.
Compiled Bun binaries don't support the -e flag, so spawning process.execPath -e '...' re-runs the main program instead of evaluating the expression. This made probeMmapSafe() always return false in production, negating the mmap optimization. The probe was unnecessary: mmap on a temp copy is safe because the copy is a regular file (separate inode), not a running binary. ETXTBSY (Linux) and AMFI SIGKILL (macOS) only affect the running binary's inode. Simplify to direct copy → mmap with try/catch fallback to arrayBuffer(). Addresses Seer review comment on PR #344.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Compiled Bun binaries don't support the -e flag, so spawning process.execPath -e '...' re-runs the main program instead of evaluating the expression. This made probeMmapSafe() always return false in production, negating the mmap optimization. The probe was unnecessary: mmap on a temp copy is safe because the copy is a regular file (separate inode), not a running binary. ETXTBSY (Linux) and AMFI SIGKILL (macOS) only affect the running binary's inode. Simplify to direct copy → mmap with try/catch fallback to arrayBuffer(). Addresses Seer review comment on PR #344.

Changes
1. Copy-then-mmap with child process probe for mmap safety
Bun.mmap()on the running binary fails fatally on macOS (SIGKILL from AMFI) andLinux (ETXTBSY). PR #343 fixed this by using
arrayBuffer()unconditionally, but thatspikes ~100 MB onto the JS heap.
This PR restores zero-heap mmap while handling the macOS SIGKILL:
copyFileSync(process.execPath, tmpdir/sentry-patch-old-{pid})CoW reflinks on btrfs/xfs/APFS for near-instant zero-I/O copy
Bun.mmap(copy)If macOS AMFI sends SIGKILL, only the child dies — parent survives and knows mmap is unsafe
arrayBuffer()(~100 MB heap)2. Instrument delta upgrade with Sentry spans and error capture
Delta failures were completely invisible in Sentry — no spans, no
captureException,errors logged at debug level. The ETXTBSY/SIGKILL bugs (PRs #339–#343) were only
discoverable through code analysis and local reproduction.
attemptDeltaUpgradeinwithTracingSpan('upgrade.delta')delta.from_version,delta.to_version,delta.channel,delta.patch_bytescaptureExceptionwith warning level + delta context tagslog.debug()tolog.warn()so users see failures