zlib: pledge input size in async zstdCompress()

[Cherry]

We precompress our site's static assets at build time, and compression-webpack-plugin calls zlib.zstdCompress() once per file. On the same files, that was 3-5x slower than zstdCompressSync().

zstdCompressSync() hands zstd the whole input in one ZSTD_e_end call, so zstd infers the size. zstdCompress() writes the input and ends the frame in separate calls, so zstd never learns the size and sizes its tables for an unbounded stream.

This defaults pledgedSrcSize to the input's byte length in zstdCompress(). Async output is now byte-identical to zstdCompressSync():

level	before	after
3	60ms	60ms
9	300ms	113ms
12	4521ms	194ms
19	9696ms	1474ms

That's 162 JS/CSS/SVG files from our production build (up to ~370 KB each), on Linux x64. Windows shows the same pattern. Single large inputs (1-64 MB) are unchanged apart from the few header bytes that record the size.

An explicit pledgedSrcSize is used as-is. Strings with a custom defaultEncoding aren't pledged, since their byte length depends on that encoding.

[codecov]

Codecov Report

❌ Patch coverage is 86.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.36%. Comparing base (a2a064c) to head (b22c547).
⚠️ Report is 15 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/zlib.js	86.66%	4 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #66358   +/-   ##
=======================================
  Coverage   90.36%   90.36%           
=======================================
  Files         792      792           
  Lines      275386   275462   +76     
  Branches    52775    52791   +16     
=======================================
+ Hits       248843   248918   +75     
+ Misses      16979    16953   -26     
- Partials     9564     9591   +27     

Files with missing lines	Coverage Δ
lib/zlib.js	98.12% <86.66%> (-0.31%)	⬇️

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

[H4ad]

I only found one benchmark that covers zstd (https://github.com/Cherry/node/blob/b4a8dbbc5a6194bd36229ffaa3bf83dcdf14f2e0/benchmark/zlib/pipe.js), might be insteresting to add another one to cover this case (at least level 12, which is fast enough to not be too long for a benchmark)

[Cherry]

Added the benchmark - here's some results on my local machine for reference:

Node	level	input	zstdCompress	pledged	speedup	zstdCompressSync
v24.21.0	3	16 KiB	1,257	4,717	3.8x	12,087
v24.21.0	3	256 KiB	1,826	2,356	1.3x	3,772
v24.21.0	12	16 KiB	104	3,394	32.6x	5,796
v24.21.0	12	256 KiB	107	895	8.4x	829
v26.10.0	3	16 KiB	3,002	5,311	1.8x	14,086
v26.10.0	3	256 KiB	2,442	2,488	1.0x	3,897
v26.10.0	12	16 KiB	125	3,523	28.1x	6,791
v26.10.0	12	256 KiB	120	921	7.7x	1,068

[H4ad]

nit: you could check for utf-8 and utf8 as well here.

And I wonder if the utf8 is really the only one, wonder if latin1/ascii and others might be applied here, except for hex/base64/base64url as denoted by

node/doc/api/buffer.md

Lines 990 to 993 in 75e4bbe

	For `'base64'`, `'base64url'`, and `'hex'`, this function assumes valid input.
	For strings that contain non-base64/hex-encoded data (e.g. whitespace), the
	return value might be greater than the length of a `Buffer` created from the
	string.

[Cherry]

Good point on utf8/utf-8. I was a bit wary of a list, though. A wrong pledge isn't just slower, zstd errors out with "Src size is incorrect". Buffer.byteLength can overcount malformed hex/base64 ('ab cd' in hex gives 2 when the actual length is 1), so we'd have to get the list exactly right.

Another option: convert the string up front with Buffer.from(buffer, opts.defaultEncoding) and pledge its real length. The stream does that conversion anyway since decodeStrings is on, so I don't think it should cost anything (thought not sure?), and it'd work for every encoding. The downside is a slightly bigger diff, since the hook would need to hand back the buffer too.

Happy to go either way. What do you think?

[H4ad]

Let's handle only check for utf8/utf-u and then we can have a follow-up PR with a extended benchmark for other encodings and we can try handle more encodings by:

• add to the bypass if the encoding is safe 1:1
• or try approach of using Buffer., in this case, we will be able to measure the performance diff between current approach and the new one

The PR is good enough now, so I would avoid over-doing it for now and have a follow-up for it to ensure correctness

[Cherry]

Thanks, makes sense, done. utf8/utf-8 now get the pledge too. Other encodings are left for a follow-up with a benchmark, as you suggested - I think there's probably some other low hanging fruit here given there's still pretty large differences in the async and sync implementations.

[H4ad]

cc @nodejs/zlib

[github-actions]

Benchmark GHA (zlib / zstd-): https://github.com/nodejs/node/actions/runs/36421711232

Results

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

Benchmark results:

                                                                               confidence improvement accuracy (*)    (**)    (***)
zlib/zstd-compress.js n=100 inputLen=16384 level=12 method='zstdCompress'             ***    871.14 %      ±60.04% ±79.30% ±102.07%
zlib/zstd-compress.js n=100 inputLen=16384 level=12 method='zstdCompressSync'                 -4.36 %       ±7.22%  ±9.52%  ±12.22%
zlib/zstd-compress.js n=100 inputLen=16384 level=3 method='zstdCompress'                       4.19 %       ±9.19% ±12.11%  ±15.54%
zlib/zstd-compress.js n=100 inputLen=16384 level=3 method='zstdCompressSync'                   0.56 %      ±10.23% ±13.48%  ±17.30%
zlib/zstd-compress.js n=100 inputLen=262144 level=12 method='zstdCompress'            ***    548.69 %      ±40.54% ±53.51%  ±68.79%
zlib/zstd-compress.js n=100 inputLen=262144 level=12 method='zstdCompressSync'                 1.28 %       ±7.94% ±10.47%  ±13.43%
zlib/zstd-compress.js n=100 inputLen=262144 level=3 method='zstdCompress'                      0.44 %       ±9.22% ±12.15%  ±15.59%
zlib/zstd-compress.js n=100 inputLen=262144 level=3 method='zstdCompressSync'                  5.12 %       ±9.17% ±12.09%  ±15.52%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 8 comparisons, you can thus
expect the following amount of false-positive results:
  0.40 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.08 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/78055/