Skip to content

Honor backpressure during ZIP finalization - #227

Draft
jeff-an wants to merge 1 commit into
archiverjs:masterfrom
jeff-an:codex/zip-finalize-backpressure
Draft

Honor backpressure during ZIP finalization#227
jeff-an wants to merge 1 commit into
archiverjs:masterfrom
jeff-an:codex/zip-finalize-backpressure

Conversation

@jeff-an

@jeff-an jeff-an commented Aug 25, 2026

Copy link
Copy Markdown

[written by Codex]

Problem

ZIP finalization writes every central-directory field synchronously without honoring write() backpressure. Large archives can therefore enqueue hundreds of thousands of tiny buffers at once and exhaust memory, as reported in #121.

Solution

  • Serialize each central file header into one buffer instead of issuing a write for every field.
  • Stop finalization when write() returns false and resume from the next entry on drain.
  • Keep completion on the stream lifecycle rather than making _finish() return an unobserved promise, and release retained entries as their headers are emitted.

PR #92 identified the right backpressure signal, but its async _finish() completion is not observed by callers and it still retains every entry and writes every header field separately. This implementation keeps the existing public API and storage contract.

Result

The benchmark below uses 50,000 empty stored entries, switches a highWaterMark: 1 sink to asynchronous writes immediately before finish(), samples RSS every millisecond, and runs each revision in three fresh Node.js processes. Values are medians on macOS with Node.js v22.13.1.

Metric Before (28f1a73) After (8635e95)
Peak RSS 378.9 MiB 108.8 MiB
RSS increase during finalize 270.7 MiB 0.9 MiB
Finalize time 12.14 s 0.76 s
Output size 5,000,022 bytes 5,000,022 bytes
Reproduce the benchmark

Save this as benchmark.mjs, then run it against clean checkouts of the base and this branch:

node --expose-gc benchmark.mjs /path/to/base-checkout 50000
node --expose-gc benchmark.mjs /path/to/patched-checkout 50000
import { once } from 'events';
import { pathToFileURL } from 'url';
import { Writable } from 'stream';

var root = process.argv[2];
var entryCount = Number(process.argv[3] || 50000);
var moduleUrl = pathToFileURL(root + '/lib/compress-commons.js');
var { ZipArchiveEntry, ZipArchiveOutputStream } = await import(moduleUrl);

class SlowSink extends Writable {
  constructor() {
    super({ highWaterMark: 1 });
    this.bytes = 0;
    this.slow = false;
  }

  _write(chunk, encoding, callback) {
    this.bytes += chunk.length;
    this.slow ? setImmediate(callback) : callback();
  }

  _writev(chunks, callback) {
    for (var i = 0; i < chunks.length; i++) {
      this.bytes += chunks[i].chunk.length;
    }
    setImmediate(callback);
  }
}

var archive = new ZipArchiveOutputStream();
var sink = new SlowSink();
archive.pipe(sink);
var empty = Buffer.alloc(0);
var fixedTime = new Date('2024-01-02T03:04:06.000Z');
for (var i = 0; i < entryCount; i++) {
  var entry = new ZipArchiveEntry('entry-' + i.toString().padStart(6, '0'));
  entry.setMethod(0);
  entry.setTime(fixedTime);
  archive.entry(entry, empty);
}

global.gc();
var baselineRss = process.memoryUsage().rss;
var peakRss = baselineRss;
var sampler = setInterval(function () {
  peakRss = Math.max(peakRss, process.memoryUsage().rss);
}, 1);
sink.slow = true;
var started = process.hrtime.bigint();
archive.finish();
peakRss = Math.max(peakRss, process.memoryUsage().rss);
await once(sink, 'finish');
clearInterval(sampler);
var elapsedMs = Number(process.hrtime.bigint() - started) / 1e6;

console.log({
  baselineRssMiB: baselineRss / 1024 / 1024,
  peakRssMiB: peakRss / 1024 / 1024,
  rssIncreaseMiB: (peakRss - baselineRss) / 1024 / 1024,
  elapsedMs,
  outputBytes: sink.bytes,
});

Tests

  • npm test (43 passing, 20 pending)
  • New 2,000-entry regression test forces a false return and drain for every central header, then parses the completed central directory and validates all entry names and offsets.
  • Deterministic regular ZIP and forced ZIP64 outputs are byte-for-byte identical to the base revision; both pass unzip -t.

Compatibility

The patch does not add filesystem I/O or change the public API. Entry metadata remains retained until finalization begins to preserve existing behavior, then is released incrementally as central headers are written.

Fixes #121

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash caused by missing backpressure support

1 participant