Repository navigation
Conversation
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
The asynchronous convenience methods hand the whole input to zlib in one write, but still take one threadpool round trip per `chunkSize` (16 KiB by default) of output. Decompressing 4 MiB took 257 round trips. Only the concatenated result of these calls is observable, so double the output buffer whenever it fills up, up to 1 MiB. 4 MiB of output now takes 10 round trips. The buffer never grows past what is needed to tell that `maxOutputLength` was exceeded, and results that fit in the first buffer are unchanged. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> Assisted-by: Grok
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #66863 +/- ##
==========================================
- Coverage 90.50% 90.50% -0.01%
==========================================
Files 797 797
Lines 278195 278282 +87
Branches 53453 53472 +19
==========================================
+ Hits 251777 251854 +77
- Misses 16795 16806 +11
+ Partials 9623 9622 -1
🚀 New features to boost your workflow:
|
mcollina
left a comment
There was a problem hiding this comment.
The change is ok. Are the tests actually needed?
Commit Queue failedThis pull request has multiple commits, but no landing policy was selected. Add
commit-queue-squash
The pull request was removed from the Commit Queue and labeled
commit-queue-failed
Full Commit Queue output |
This builds on #66854, so only the last commit (753dd48) is new here.
After #66854 the async convenience methods hand the whole input to zlib in one write, but they still take one threadpool round trip per
chunkSize(16 KiB by default) of output. Azlib.gunzip()that returns 4 MiB takes 257 round trips.Only the concatenated result of these calls is visible to the caller, so this doubles the output buffer whenever it fills up, up to 1 MiB. 4 MiB of output now takes 10 round trips. The buffer never grows past what's needed to tell that
maxOutputLengthwas exceeded, so it doesn't decompress any further past the limit than before. Results that fit in the first buffer take the same path as before, and the stream classes aren't touched. The extra memory while a call runs is the unused tail of the last buffer, so under 1 MiB.Tests:
test-zlib-convenience-async-large.jsruns 2.25 MiB through every async method. It checks the result matches the sync methods byte for byte, and trieschunkSizeof 64 and larger than the input. It also checksmaxOutputLengthone byte short, at 100000, at 1 and at the exact size. It passes on main and on zlib: finish async convenience calls in one write #66854 too.test-zlib-convenience-async-round-trip.jsgets a 4 MiB case pinned at 10 round trips. It's 257 on zlib: finish async convenience calls in one write #66854.Benchmarks
benchmark/compare.js,inflate.js method='inflate'withinputLenraised to get large outputs andnscaled down. My machine was heavily loaded (load average 70–100 from other builds), so the intervals are wide, but the effect is far outside them:inputLenA/A (#66854 against itself, 1 MiB, 40 runs) is +2.97% ±20.84%.
The default 1 KiB configurations don't reach the new code, because their output fits in the first buffer. The
--max-regression 5gate overdeflate.js,inflate.js,pipe.jsandcreation.jspassed but came back inconclusive under that load (intervals around ±40%). Outside one-shot calls, the only change is an extra condition when a stream's output buffer fills up.compare.js output
Below, Chrome talks to three local servers, one each on main, #66854 and this PR, all with the same 4 MiB payload. It decodes a
zlib.gzip()response and uploads aCompressionStream('gzip')body that the server decodes withzlib.gunzip(). The bytes match on all three. Onezlib.gunzip()takes 258, 257 and 10 round trips. The per-call time is the median of five rounds, interleaved so the load hit each build alike.