Skip to content

Commit 86f2e27

Browse files
anonrigaduh95
anonrig
authored and
aduh95
committed
zlib: finish async convenience calls in one write
The asynchronous convenience methods (`zlib.deflate()`, `zlib.inflate()`, `zlib.gunzip()`, ...) wrote the input to a Transform stream and then ended it. That took two threadpool round trips, one for the data and one for the final flush, and ran the result through the stream machinery. The whole input is known up front, so unless `info` or a custom `flush` is requested, process it with the finish flush directly, the way the synchronous methods already do, and collect the output in the write callback. A call whose output fits in one buffer now takes a single threadpool round trip. The output is unchanged: the previous two-write sequence already produced the same bytes as the synchronous methods. `zstdCompress()` keeps the separate end-of-frame write, because a single `ZSTD_e_end` call can produce slightly larger frames. benchmark/compare.js, 40 runs, n=100000: confidence improvement zlib/inflate.js method='inflate' *** +94.91 % zlib/deflate.js method='deflate' *** +66.27 % Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> Assisted-by: Grok PR-URL: #66863 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 997d0d6 commit 86f2e27

3 files changed

Lines changed: 454 additions & 24 deletions

File tree

‎lib/zlib.js‎

Lines changed: 71 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ const {
8888

8989
const kFlushFlag = Symbol('kFlushFlag');
9090
const kError = Symbol('kError');
91+
const kBufferResult = Symbol('kBufferResult');
9192

9293
const constants = internalBinding('constants').zlib;
9394
const {
@@ -125,7 +126,7 @@ for (const ckey of ObjectKeys(codes)) {
125126
codes[codes[ckey]] = ckey;
126127
}
127128

128-
function zlibBuffer(engine, buffer, callback) {
129+
function zlibBuffer(engine, buffer, callback, finishInOneWrite) {
129130
validateFunction(callback, 'callback');
130131
// Streams do not support non-Uint8Array ArrayBufferViews yet. Convert it to a
131132
// Buffer without copying.
@@ -137,44 +138,81 @@ function zlibBuffer(engine, buffer, callback) {
137138
engine.buffers = null;
138139
engine.nread = 0;
139140
engine.cb = callback;
140-
engine.on('data', zlibBufferOnData);
141141
engine.on('error', zlibBufferOnError);
142+
143+
// The whole input is known up front. Unless the engine is handed to the
144+
// caller (`info`) or a custom flush is requested, process it with the finish
145+
// flush directly, like the synchronous methods do. This takes one threadpool
146+
// round trip instead of a write followed by a separate final flush, and skips
147+
// the stream machinery.
148+
if (finishInOneWrite && !engine._info &&
149+
engine._defaultFlushFlag === FLUSH_BOUND[engine._flushBoundIdx][0]) {
150+
if (typeof buffer === 'string') {
151+
// Encode strings like Writable#write() does.
152+
buffer = Buffer.from(buffer, engine._writableState.defaultEncoding);
153+
}
154+
if (isUint8Array(buffer)) {
155+
engine[kBufferResult] = true;
156+
processChunk(engine, buffer, engine._finishFlushFlag,
157+
zlibBufferOnWriteDone);
158+
return;
159+
}
160+
}
161+
162+
engine.on('data', zlibBufferOnData);
142163
engine.on('end', zlibBufferOnEnd);
143164
engine.end(buffer);
144165
}
145166

146-
function zlibBufferOnData(chunk) {
147-
if (!this.buffers) {
148-
this.buffers = [chunk];
167+
function zlibBufferAddChunk(engine, chunk) {
168+
if (!engine.buffers) {
169+
engine.buffers = [chunk];
149170
} else {
150-
this.buffers.push(chunk);
171+
engine.buffers.push(chunk);
151172
}
152-
this.nread += chunk.length;
153-
if (this.nread > this._maxOutputLength) {
154-
this.close();
155-
this.removeAllListeners('end');
156-
this.cb(new ERR_BUFFER_TOO_LARGE(this._maxOutputLength));
173+
engine.nread += chunk.length;
174+
if (engine.nread > engine._maxOutputLength) {
175+
engine.close();
176+
engine.removeAllListeners('end');
177+
engine.cb(new ERR_BUFFER_TOO_LARGE(engine._maxOutputLength));
157178
}
158179
}
159180

181+
function zlibBufferOnData(chunk) {
182+
zlibBufferAddChunk(this, chunk);
183+
}
184+
160185
function zlibBufferOnError(err) {
161186
this.removeAllListeners('end');
162187
this.cb(err);
163188
}
164189

165-
function zlibBufferOnEnd() {
190+
function zlibBufferFinish(engine) {
166191
let buf;
167-
if (this.nread === 0) {
192+
if (engine.nread === 0) {
168193
buf = new FastBuffer();
169194
} else {
170-
const bufs = this.buffers;
171-
buf = (bufs.length === 1 ? bufs[0] : Buffer.concat(bufs, this.nread));
195+
const bufs = engine.buffers;
196+
buf = (bufs.length === 1 ? bufs[0] : Buffer.concat(bufs, engine.nread));
172197
}
173-
this.close();
174-
if (this._info)
175-
this.cb(null, { buffer: buf, engine: this });
198+
engine.close();
199+
if (engine._info)
200+
engine.cb(null, { buffer: buf, engine });
176201
else
177-
this.cb(null, buf);
202+
engine.cb(null, buf);
203+
}
204+
205+
function zlibBufferOnEnd() {
206+
zlibBufferFinish(this);
207+
}
208+
209+
// Called by processCallback(), with the handle as `this`, once the whole input
210+
// has been processed. Errors have already been reported to the callback, either
211+
// through the 'error' event or by zlibBufferAddChunk().
212+
function zlibBufferOnWriteDone(err) {
213+
const engine = this[owner_symbol];
214+
if (err === undefined && !engine.destroyed)
215+
zlibBufferFinish(engine);
178216
}
179217

180218
function toSyncInput(buffer) {
@@ -638,7 +676,10 @@ function processCallback() {
638676
if (have > 0) {
639677
const out = self._outBuffer.slice(self._outOffset, self._outOffset + have);
640678
self._outOffset += have;
641-
streamBufferIsFull = !self.push(out);
679+
if (self[kBufferResult] === true)
680+
zlibBufferAddChunk(self, out);
681+
else
682+
streamBufferIsFull = !self.push(out);
642683
} else {
643684
assert(have === 0, 'have should not go down');
644685
}
@@ -704,7 +745,8 @@ function processCallback() {
704745
return;
705746
}
706747

707-
self.push(null);
748+
if (self[kBufferResult] !== true)
749+
self.push(null);
708750
}
709751

710752
// Finished with the chunk.
@@ -877,7 +919,8 @@ class Unzip extends Zlib {
877919
}
878920
}
879921

880-
function createConvenienceMethod(ctor, sync, prepareOpts) {
922+
function createConvenienceMethod(ctor, sync, prepareOpts,
923+
finishInOneWrite = true) {
881924
if (sync) {
882925
return function syncBufferWrapper(buffer, opts) {
883926
if (opts === undefined)
@@ -893,7 +936,7 @@ function createConvenienceMethod(ctor, sync, prepareOpts) {
893936
if (prepareOpts !== undefined) {
894937
opts = prepareOpts(buffer, opts);
895938
}
896-
return zlibBuffer(new ctor(opts), buffer, callback);
939+
return zlibBuffer(new ctor(opts), buffer, callback, finishInOneWrite);
897940
};
898941
}
899942

@@ -1272,7 +1315,11 @@ module.exports = {
12721315
brotliCompressSync: createConvenienceMethod(BrotliCompress, true),
12731316
brotliDecompress: createConvenienceMethod(BrotliDecompress, false),
12741317
brotliDecompressSync: createConvenienceMethod(BrotliDecompress, true),
1275-
zstdCompress: createConvenienceMethod(ZstdCompress, false, withPledgedSrcSize),
1318+
// A zstd frame written by a single ZSTD_e_end call can be a few bytes larger
1319+
// than one written by ZSTD_e_continue followed by ZSTD_e_end, so
1320+
// zstdCompress() keeps writing the input and ending the frame separately.
1321+
zstdCompress: createConvenienceMethod(ZstdCompress, false, withPledgedSrcSize,
1322+
false),
12761323
zstdCompressSync: createConvenienceMethod(ZstdCompress, true),
12771324
zstdDecompress: createConvenienceMethod(ZstdDecompress, false),
12781325
zstdDecompressSync: createConvenienceMethod(ZstdDecompress, true),

‎test/parallel/test-zlib-convenience-async-round-trip.js‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
'use strict';
2+
// The asynchronous convenience methods know the whole input up front, so they
3+
// process it with the finish flush in a single write. Check that a call whose
4+
// output fits in one output buffer takes exactly one threadpool round trip.
5+
6+
const common = require('../common');
7+
const assert = require('assert');
8+
const async_hooks = require('async_hooks');
9+
const zlib = require('zlib');
10+
11+
const input = Buffer.from('hello world '.repeat(64));
12+
// [method, argument, function that turns the result back into `input`]
13+
const cases = [
14+
['deflate', input, zlib.inflateSync],
15+
['inflate', zlib.deflateSync(input)],
16+
['gzip', input, zlib.gunzipSync],
17+
['gunzip', zlib.gzipSync(input)],
18+
['unzip', zlib.gzipSync(input)],
19+
['deflateRaw', input, zlib.inflateRawSync],
20+
['inflateRaw', zlib.deflateRawSync(input)],
21+
['brotliCompress', input, zlib.brotliDecompressSync],
22+
['brotliDecompress', zlib.brotliCompressSync(input)],
23+
['zstdDecompress', zlib.zstdCompressSync(input)],
24+
];
25+
26+
// Counts the callbacks into JavaScript (`before` hooks) of each ZLIB resource
27+
// created while `fn` runs.
28+
async function countRoundTrips(fn) {
29+
const callbacks = new Map();
30+
const hook = async_hooks.createHook({
31+
init(asyncId, type) {
32+
if (type === 'ZLIB') callbacks.set(asyncId, 0);
33+
},
34+
before(asyncId) {
35+
if (callbacks.has(asyncId))
36+
callbacks.set(asyncId, callbacks.get(asyncId) + 1);
37+
},
38+
}).enable();
39+
try {
40+
await fn();
41+
} finally {
42+
hook.disable();
43+
}
44+
return [...callbacks.values()];
45+
}
46+
47+
function callAsync(method, buffer, options = {}) {
48+
return new Promise((resolve, reject) => {
49+
zlib[method](buffer, options, common.mustCall((err, result) => {
50+
if (err) reject(err);
51+
else resolve(result);
52+
}));
53+
});
54+
}
55+
56+
(async () => {
57+
for (const [method, buffer, undo = (result) => result] of cases) {
58+
let result;
59+
const roundTrips = await countRoundTrips(async () => {
60+
result = await callAsync(method, buffer);
61+
});
62+
assert.deepStrictEqual(roundTrips, [1], method);
63+
assert.deepStrictEqual(undo(result), input, method);
64+
}
65+
66+
// Errors are also reported after a single round trip.
67+
let error;
68+
const roundTrips = await countRoundTrips(async () => {
69+
error = await callAsync('inflate', Buffer.from('not zlib data'))
70+
.catch((err) => err);
71+
});
72+
assert.deepStrictEqual(roundTrips, [1]);
73+
assert.strictEqual(error.code, 'Z_DATA_ERROR');
74+
})().then(common.mustCall());

0 commit comments

Comments
 (0)