Skip to content

zlib: skip the stream in sync convenience calls without options - #66864

Open
anonrig wants to merge 1 commit into
nodejs:mainfrom
anonrig:zlib-sync-direct
Open

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

Conversation

@anonrig

@anonrig anonrig commented Oct 19, 2026

Copy link
Copy Markdown
Member

zlib.inflateSync(buffer) and the other sync convenience methods build a Transform stream for every call, even though they only use its native handle. Without options, the stream is never exposed to the caller. They also allocate a fresh 16 KiB output chunk per call.

When no options are passed, this creates the native handle with the default settings directly and skips the stream. It also decompresses into a scratch buffer that's kept between calls and copies the result out. Calls with options, including info, keep the existing path.

Results and errors are unchanged. The new test compares every method against the same call with {} (which still goes through the stream): every input type, outputs that fit in one chunk, fill it exactly or span several, plus invalid and truncated input. It also checks that results never share memory, and that a sync call made from an async hook during another call still works. It passes on main too. One difference: a result that fits in one chunk is now its own allocation instead of a slice of a 16 KiB chunk, so small results don't keep that chunk alive anymore.

This is independent of #66854 and #66863. It only touches the sync path.

Benchmarks

benchmark/compare.js, default inputLen, n scaled down, against main. My machine was loaded (load average 15–30), so the intervals are wide:

benchmark run 1 (40 runs) run 2 (20 runs)
inflate.js method='inflateSync' +57.39% *** ±12.66% +71.62% *** ±27.39%
deflate.js method='deflateSync' +12.40% ** ±8.85% +12.49% ±17.03%

deflateSync gains less because most of its time goes to setting up and tearing down zlib's deflate state. The async deflate, inflate and createDeflate configurations, which this doesn't touch, came out within noise (−0.09% to +4.18%). I didn't get a clean pipe.js or creation.js run: pipe.js duration=1 failed on both main and this branch because the machine was too loaded to produce any output within a second.

`zlib.inflateSync(buffer)` and the other synchronous convenience
methods constructed a Transform stream for every call, even though
only its native handle is used and, without options, the stream is
never exposed. They also allocated a fresh output chunk for every
call.

When no options are passed, drive a native handle created with the
default settings directly, and decompress into a scratch buffer kept
between calls, copying the result out. Calls with options, including
`info`, keep the existing path. Results and errors are unchanged.

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 review from H4ad and mcollina October 19, 2026 20:43
@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 (af015be).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #66864    +/-   ##
========================================
  Coverage   90.50%   90.50%            
========================================
  Files         797      797            
  Lines      278195   278363   +168     
  Branches    53453    53509    +56     
========================================
+ Hits       251777   251931   +154     
- Misses      16795    16808    +13     
- Partials     9623     9624     +1     
Files with missing lines Coverage Δ
lib/zlib.js 98.32% <100.00%> (+0.20%) ⬆️

... and 32 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.

lgtm

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

Labels

needs-ci PRs that need a full CI run. 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.

3 participants