Skip to content

zlib: finish async convenience calls in one write - #66854

Closed
anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:zlib-perf
Closed

anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:zlib-perf

Conversation

@anonrig

@anonrig anonrig commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

The async convenience methods (zlib.deflate(), zlib.inflate(), zlib.gunzip(), ...) write the input to a Transform and then end it. That's two threadpool round trips per call, one for the data and one for the final flush, plus the stream machinery on top. In a CPU profile of benchmark/zlib/inflate.js method=inflate, the main thread sits idle about 71% of the time waiting on the threadpool.

We already have the whole input up front, so this hands it to the handle in a single write with the finish flush, the same way the *Sync methods do, and collects the output in the write callback. A call whose output fits in one buffer now takes one round trip. The work still runs on the threadpool.

These stay on the stream path:

  • info: true, since the engine is handed back to the caller.
  • A non-default flush, which can change the output bytes.
  • zstdCompress(). A single ZSTD_e_end write makes frames 3–18 bytes larger than ZSTD_e_continue followed by ZSTD_e_end, so I left it alone.

The output doesn't change. For deflate, gzip, raw deflate and brotli, the old two-write sequence already produced the same bytes as the sync methods. I compared all the async methods plus gzipSync()/deflateSync() on four inputs (source text, text plus random bytes, a short string, empty) against a build of the base commit: 28/28 outputs are byte-identical, and each build decompresses the other's output.

There are two observable differences. The success callback now runs straight from the write completion, so async_hooks sees it in the ZLIB resource instead of a TickObject, which is how fs callbacks behave too. AsyncLocalStorage context is unchanged, and the new test checks it. Errors still arrive through the 'error' event like before. Also, this inside a non-arrow callback is still the internal engine, but it's never ended now, so writableEnded, writableFinished and readableEnded are false (it's still destroyed, with bytesWritten set), and info: true, the documented way to get the engine, keeps the old stream path.

Two new tests:

  • test-zlib-convenience-async.js covers the contract: every input type, interop with the sync methods, invalid and truncated input, maxOutputLength, info, rejectGarbageAfterEnd, dictionaries, finishFlush, and that the callback never runs synchronously. It passes on the base commit too.
  • test-zlib-convenience-async-round-trip.js uses async_hooks to check that a small call takes exactly one round trip. It fails on the base commit, which takes two.

Benchmarks

benchmark/compare.js, 40 runs, n=100000, against a build of the base commit with the same configure flags. The default method/inputLen are kept and only n is scaled down.

benchmark run 1 (loaded machine) run 2 (quiet machine) A/A, quiet
inflate.js method='inflate' +98.11% *** +94.91% *** ±0.59% +0.10%
deflate.js method='deflate' +62.96% *** +66.27% *** ±2.34% +0.34%

--max-regression 5 over deflate.js, inflate.js, pipe.js and creation.js (20 runs) passes. The sync methods, createDeflate writes, pipes and stream creation all stay within about 2%.

compare.js, run 2 (40 runs)
                                                         confidence   improvement   accuracy (*)    (**)   (***)
zlib/inflate.js n=100000 inputLen=1024 method='inflate'  ***            +94.91 %   ±0.59%  ±0.78%  ±1.02%

                                                         confidence   improvement   accuracy (*)    (**)   (***)
zlib/deflate.js n=100000 inputLen=1024 method='deflate'  ***            +66.27 %   ±2.34%  ±3.11%  ±4.06%
compare.js --max-regression 5 (20 runs)
                                                               confidence   improvement   accuracy (*)    (**)   (***)
zlib/deflate.js n=100000 inputLen=1024 method='createDeflate'                  +0.84 %   ±2.03%  ±2.73%  ±3.58%
zlib/deflate.js n=100000 inputLen=1024 method='deflate'        ***            +75.51 %   ±1.87%  ±2.51%  ±3.31%
zlib/deflate.js n=100000 inputLen=1024 method='deflateSync'                    +0.12 %   ±1.06%  ±1.42%  ±1.86%

                                                             confidence   improvement   accuracy (*)    (**)   (***)
zlib/inflate.js n=100000 inputLen=1024 method='inflate'      ***            +95.85 %   ±3.53%  ±4.79%  ±6.43%
zlib/inflate.js n=100000 inputLen=1024 method='inflateSync'                  -1.56 %   ±3.28%  ±4.41%  ±5.84%

                                                                        confidence   improvement   accuracy (*)    (**)   (***)
zlib/pipe.js algorithm='gzip' type='string' duration=1 inputLen=1024                    +1.74 %   ±2.32%  ±3.13%  ±4.18%
zlib/pipe.js algorithm='brotli' type='string' duration=1 inputLen=1024                  +0.12 %   ±3.43%  ±4.59%  ±6.04%
zlib/pipe.js algorithm='zstd' type='string' duration=1 inputLen=1024                    -0.15 %   ±1.40%  ±1.88%  ±2.47%
zlib/pipe.js algorithm='gzip' type='buffer' duration=1 inputLen=1024                    -0.90 %   ±1.75%  ±2.35%  ±3.10%
zlib/pipe.js algorithm='brotli' type='buffer' duration=1 inputLen=1024                  +0.74 %   ±1.50%  ±2.06%  ±2.79%
zlib/pipe.js algorithm='zstd' type='buffer' duration=1 inputLen=1024      *             -1.54 %   ±1.16%  ±1.55%  ±2.05%

                                                                   confidence   improvement   accuracy (*)    (**)   (***)
zlib/creation.js n=500000 options='true' type='Deflate'                            +0.49 %   ±1.14%  ±1.53%  ±2.01%
zlib/creation.js n=500000 options='false' type='Deflate'                           -0.43 %   ±0.72%  ±0.96%  ±1.26%
zlib/creation.js n=500000 options='true' type='DeflateRaw'           *             -2.01 %   ±1.55%  ±2.10%  ±2.81%
zlib/creation.js n=500000 options='false' type='DeflateRaw'                        +0.81 %   ±1.64%  ±2.23%  ±2.98%
zlib/creation.js n=500000 options='true' type='Inflate'                            -0.19 %   ±1.01%  ±1.36%  ±1.80%
zlib/creation.js n=500000 options='false' type='Inflate'                           +1.17 %   ±1.68%  ±2.27%  ±3.02%
zlib/creation.js n=500000 options='true' type='InflateRaw'                         -1.01 %   ±2.40%  ±3.23%  ±4.25%
zlib/creation.js n=500000 options='false' type='InflateRaw'                        +0.72 %   ±1.35%  ±1.82%  ±2.41%
zlib/creation.js n=500000 options='true' type='Gzip'                               +0.91 %   ±1.44%  ±1.93%  ±2.56%
zlib/creation.js n=500000 options='false' type='Gzip'                              +0.34 %   ±1.60%  ±2.16%  ±2.87%
zlib/creation.js n=500000 options='true' type='Gunzip'                             -0.17 %   ±1.15%  ±1.55%  ±2.04%
zlib/creation.js n=500000 options='false' type='Gunzip'                            +0.61 %   ±1.74%  ±2.33%  ±3.06%
zlib/creation.js n=500000 options='true' type='Unzip'                              -1.05 %   ±1.72%  ±2.31%  ±3.06%
zlib/creation.js n=500000 options='false' type='Unzip'                             +0.09 %   ±0.97%  ±1.31%  ±1.74%
zlib/creation.js n=500000 options='true' type='BrotliCompress'                     -2.17 %   ±3.48%  ±4.74%  ±6.39%
zlib/creation.js n=500000 options='false' type='BrotliCompress'                    -1.24 %   ±2.90%  ±3.92%  ±5.25%
zlib/creation.js n=500000 options='true' type='BrotliDecompress'                   -0.96 %   ±2.39%  ±3.21%  ±4.23%
zlib/creation.js n=500000 options='false' type='BrotliDecompress'                  -1.13 %   ±2.70%  ±3.62%  ±4.76%
zlib/creation.js n=500000 options='true' type='ZstdCompress'                       +0.13 %   ±1.42%  ±1.91%  ±2.52%
zlib/creation.js n=500000 options='false' type='ZstdCompress'                      -0.24 %   ±1.38%  ±1.85%  ±2.44%
zlib/creation.js n=500000 options='true' type='ZstdDecompress'                     -0.28 %   ±2.36%  ±3.17%  ±4.21%
zlib/creation.js n=500000 options='false' type='ZstdDecompress'                    +0.13 %   ±1.49%  ±2.00%  ±2.63%

Below, Chrome talks to two local servers, one on the base build and one on this PR. It decodes zlib.gzip()/zlib.deflate()/zlib.brotliCompress() responses, and sends CompressionStream uploads that the servers decode with zlib.gunzip()/zlib.inflate(). The payloads match on both, the round trip count drops from 2 to 1, and sequential zlib.inflate() calls get faster.

Chrome checking both builds: gzip, deflate and brotli responses decode to the same bytes, CompressionStream uploads decode with zlib.gunzip()/zlib.inflate(), one zlib.inflate() takes 2 threadpool round trips on the base build and 1 on this PR, and 20,000 sequential 1 KiB zlib.inflate() calls run at 59,335 vs 111,603 ops/s

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
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib module and its compression dependencies. labels Oct 19, 2026
@anonrig
anonrig requested a review from mcollina October 19, 2026 16:26
@anonrig
anonrig requested a review from ronag October 19, 2026 16:28
@anonrig anonrig added the semver-minor PRs that contain new features and should be released in the next minor version. label Oct 19, 2026
@codecov

codecov Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.50%. Comparing base (2052d0d) to head (a93077c).
⚠️ Report is 142 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #66854   +/-   ##
=======================================
  Coverage   90.50%   90.50%           
=======================================
  Files         797      797           
  Lines      278195   278240   +45     
  Branches    53453    53460    +7     
=======================================
+ Hits       251777   251832   +55     
+ Misses      16795    16786    -9     
+ Partials     9623     9622    -1     
Files with missing lines Coverage Δ
lib/zlib.js 98.19% <100.00%> (+0.07%) ⬆️

... and 44 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change is good. Are the tests actually needed given it’s an optimization?

@anonrig

anonrig commented Oct 20, 2026

Copy link
Copy Markdown
Member Author

change is good. Are the tests actually needed given it’s an optimization?

It doesn't hurt to cover this edge case. @mcollina

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 22, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 22, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@anonrig anonrig added the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 28, 2026
@nodejs-github-bot nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Oct 28, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Commit Queue failed

   ⚠  Could not retrieve the email or name of the PR's author from user's GitHub profile!
   ✖  Failed to apply patches

The pull request was removed from the Commit Queue and labeled commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. . After resolving the failure, remove that label and add commit-queue PRs queued for automated landing through the Commit Queue. to retry.

Full Commit Queue output
�[36m⠋�[39m Loading data for nodejs/node/pull/66854
�[36m⠋�[39m Loading data for nodejs/node/pull/66854
�[36m⠋�[39m Getting collaborator contacts from README of nodejs/node
�[36m⠋�[39m Getting PR from nodejs/node/pull/66854
�[36m⠋�[39m Getting reviews from nodejs/node/pull/66854
�[36m⠋�[39m Getting comments from nodejs/node/pull/66854
�[36m⠋�[39m Getting commits from nodejs/node/pull/66854
✔  Done loading data for nodejs/node/pull/66854
----------------------------------- PR info ------------------------------------
Title      zlib: finish async convenience calls in one write (#66854)
   ⚠  Could not retrieve the email or name of the PR's author from user's GitHub profile!
Branch     anonrig:zlib-perf -> nodejs:main
Labels     zlib, semver-minor, needs-ci, commit-queue
Commits    1
 - zlib: finish async convenience calls in one write
Committers 1
 - Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/66854
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/66854
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 19 Oct 2026 16:23:55 GMT
   ✔  Approvals: 1
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/66854#pullrequestreview-5607904214
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-10-22T01:06:43Z: https://ci.nodejs.org/job/node-test-pull-request/78558/
�[36m⠙�[39m Querying data for job/node-test-pull-request/78558/
�[36m⠙�[39m Querying data for job/node-test-pull-request/78558/
�[36m⠙�[39m Querying API for job/node-test-pull-request/78558/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
�[36m⠹�[39m Bringing origin/main up to date...
�[36m⠹�[39m Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
�[36m⠸�[39m Downloading patch for 66854
�[36m⠸�[39m Downloading patch for 66854
From https://github.com/nodejs/node
 * branch                  refs/pull/66854/merge -> FETCH_HEAD
✔  Fetched commits as 2a604374b753..a93077c50e85
--------------------------------------------------------------------------------
Auto-merging lib/zlib.js
CONFLICT (content): Merge conflict in lib/zlib.js
Auto-merging test/parallel/test-zlib-convenience-async-round-trip.js
CONFLICT (add/add): Merge conflict in test/parallel/test-zlib-convenience-async-round-trip.js
error: could not apply a93077c50e... zlib: finish async convenience calls in one write
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
   ✖  Failed to apply patches

View workflow run

@anonrig

anonrig commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

This landed in cbe5585 together with #66863, so I'm closing it.

@anonrig anonrig closed this Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version. zlib Issues and PRs related to the zlib module and its compression dependencies.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants