build: rework gyp files for zlib#45589

lpinca · 2022-11-22T19:58:31Z

This is #33044 rebased and updated. It allows us to stop floating 26991f7.

Fixes: #32856

nodejs-github-bot · 2022-11-22T19:58:36Z

Review requested:

@nodejs/gyp
@nodejs/tsc

nodejs-github-bot · 2022-11-22T20:04:40Z

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

mscdex · 2022-11-23T01:34:23Z

Something seems not right. This seems to consistently cause a ~23-26% performance regression with zlib/createInflate.js. Example zlib results:

zlib/createInflate.js n=100 chunkLen=1024 inputLen=16777216 *** -26.42 % ±6.37% ±8.48% ±11.04% zlib/creation.js n=500000 options='false' type='BrotliCompress' 1.05 % ±4.04% ±5.37% ±7.00% zlib/creation.js n=500000 options='false' type='BrotliDecompress' 1.33 % ±4.81% ±6.39% ±8.32% zlib/creation.js n=500000 options='false' type='Deflate' -1.38 % ±3.66% ±4.88% ±6.38% zlib/creation.js n=500000 options='false' type='DeflateRaw' -0.66 % ±3.63% ±4.83% ±6.28% zlib/creation.js n=500000 options='false' type='Gunzip' 0.59 % ±3.51% ±4.67% ±6.08% zlib/creation.js n=500000 options='false' type='Gzip' 0.74 % ±4.61% ±6.14% ±8.00% zlib/creation.js n=500000 options='false' type='Inflate' -0.76 % ±3.93% ±5.23% ±6.81% zlib/creation.js n=500000 options='false' type='InflateRaw' -2.86 % ±3.99% ±5.31% ±6.91% zlib/creation.js n=500000 options='false' type='Unzip' -0.45 % ±4.21% ±5.61% ±7.30% zlib/creation.js n=500000 options='true' type='BrotliCompress' ** -5.64 % ±4.13% ±5.50% ±7.15% zlib/creation.js n=500000 options='true' type='BrotliDecompress' 1.55 % ±3.93% ±5.22% ±6.80% zlib/creation.js n=500000 options='true' type='Deflate' 1.30 % ±4.24% ±5.64% ±7.34% zlib/creation.js n=500000 options='true' type='DeflateRaw' 2.37 % ±3.84% ±5.11% ±6.67% zlib/creation.js n=500000 options='true' type='Gunzip' -0.43 % ±4.58% ±6.10% ±7.93% zlib/creation.js n=500000 options='true' type='Gzip' -1.79 % ±3.94% ±5.26% ±6.87% zlib/creation.js n=500000 options='true' type='Inflate' -0.54 % ±5.50% ±7.32% ±9.52% zlib/creation.js n=500000 options='true' type='InflateRaw' 0.02 % ±3.85% ±5.12% ±6.67% zlib/creation.js n=500000 options='true' type='Unzip' 0.66 % ±3.86% ±5.14% ±6.71% zlib/deflate.js n=400000 inputLen=1024 method='createDeflate' 5.66 % ±10.05% ±13.37% ±17.40% zlib/deflate.js n=400000 inputLen=1024 method='deflate' -1.48 % ±5.39% ±7.17% ±9.34% zlib/deflate.js n=400000 inputLen=1024 method='deflateSync' 9.48 % ±19.18% ±25.51% ±33.21% zlib/inflate.js n=400000 inputLen=1024 method='inflate' -1.64 % ±3.74% ±4.98% ±6.48% zlib/inflate.js n=400000 inputLen=1024 method='inflateSync' ** -8.61 % ±5.63% ±7.49% ±9.75% zlib/pipe.js algorithm='brotli' type='buffer' duration=5 inputLen=1024 0.89 % ±2.82% ±3.76% ±4.92% zlib/pipe.js algorithm='brotli' type='string' duration=5 inputLen=1024 -0.05 % ±1.97% ±2.63% ±3.44% zlib/pipe.js algorithm='gzip' type='buffer' duration=5 inputLen=1024 *** 10.93 % ±6.13% ±8.21% ±10.79% zlib/pipe.js algorithm='gzip' type='string' duration=5 inputLen=1024 4.82 % ±7.09% ±9.43% ±12.28%

richardlau · 2022-11-23T02:06:13Z

Something seems not right. This seems to consistently cause a ~23-26% performance regression with zlib/createInflate.js. Example zlib results:

Yeah, that's pretty much why #33044 got stuck.

lpinca · 2022-11-23T06:56:06Z

A possible alternative is to propose dccdc51 upstream but I'm not sure if it will be accepted.

lpinca · 2022-11-23T18:54:08Z

I see no performance regressions after the last commit

https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1235/console

 confidence improvement accuracy (*) (**) (***) zlib/createInflate.js n=100 chunkLen=1024 inputLen=16777216 2.39 % ±6.36% ±8.47% ±11.02% zlib/creation.js n=500000 options='false' type='BrotliCompress' -2.78 % ±4.04% ±5.38% ±7.00% zlib/creation.js n=500000 options='false' type='BrotliDecompress' 2.62 % ±3.99% ±5.31% ±6.92% zlib/creation.js n=500000 options='false' type='Deflate' 1.67 % ±3.58% ±4.77% ±6.22% zlib/creation.js n=500000 options='false' type='DeflateRaw' -0.37 % ±3.74% ±4.98% ±6.49% zlib/creation.js n=500000 options='false' type='Gunzip' 2.78 % ±4.33% ±5.76% ±7.50% zlib/creation.js n=500000 options='false' type='Gzip' 0.68 % ±3.56% ±4.74% ±6.17% zlib/creation.js n=500000 options='false' type='Inflate' 0.87 % ±4.33% ±5.77% ±7.51% zlib/creation.js n=500000 options='false' type='InflateRaw' 2.31 % ±3.68% ±4.89% ±6.37% zlib/creation.js n=500000 options='false' type='Unzip' -1.62 % ±3.81% ±5.09% ±6.65% zlib/creation.js n=500000 options='true' type='BrotliCompress' 1.83 % ±5.21% ±6.95% ±9.07% zlib/creation.js n=500000 options='true' type='BrotliDecompress' -2.27 % ±4.28% ±5.70% ±7.42% zlib/creation.js n=500000 options='true' type='Deflate' 0.90 % ±3.88% ±5.16% ±6.72% zlib/creation.js n=500000 options='true' type='DeflateRaw' -1.35 % ±3.78% ±5.03% ±6.55% zlib/creation.js n=500000 options='true' type='Gunzip' 1.41 % ±3.81% ±5.07% ±6.61% zlib/creation.js n=500000 options='true' type='Gzip' -0.01 % ±3.35% ±4.46% ±5.81% zlib/creation.js n=500000 options='true' type='Inflate' -1.99 % ±3.64% ±4.84% ±6.31% zlib/creation.js n=500000 options='true' type='InflateRaw' 2.47 % ±4.07% ±5.43% ±7.07% zlib/creation.js n=500000 options='true' type='Unzip' 1.47 % ±3.65% ±4.86% ±6.33% zlib/deflate.js n=400000 inputLen=1024 method='createDeflate' -0.54 % ±11.15% ±14.84% ±19.33% zlib/deflate.js n=400000 inputLen=1024 method='deflate' 2.14 % ±6.05% ±8.06% ±10.50% zlib/deflate.js n=400000 inputLen=1024 method='deflateSync' 3.71 % ±18.09% ±24.07% ±31.33% zlib/inflate.js n=400000 inputLen=1024 method='inflate' -1.45 % ±3.46% ±4.60% ±5.99% zlib/inflate.js n=400000 inputLen=1024 method='inflateSync' -0.71 % ±5.50% ±7.32% ±9.53% zlib/pipe.js algorithm='brotli' type='buffer' duration=5 inputLen=1024 -0.81 % ±2.82% ±3.75% ±4.89% zlib/pipe.js algorithm='brotli' type='string' duration=5 inputLen=1024 * 3.19 % ±2.65% ±3.53% ±4.61% zlib/pipe.js algorithm='gzip' type='buffer' duration=5 inputLen=1024 -3.50 % ±5.48% ±7.34% ±9.65% zlib/pipe.js algorithm='gzip' type='string' duration=5 inputLen=1024 1.89 % ±5.89% ±7.84% ±10.21%

https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1236/console

 confidence improvement accuracy (*) (**) (***) zlib/createInflate.js n=100 chunkLen=1024 inputLen=16777216 2.11 % ±7.19% ±9.57% ±12.46% zlib/creation.js n=500000 options='false' type='BrotliCompress' * 4.72 % ±4.46% ±5.93% ±7.73% zlib/creation.js n=500000 options='false' type='BrotliDecompress' * 4.47 % ±4.46% ±5.93% ±7.72% zlib/creation.js n=500000 options='false' type='Deflate' 0.03 % ±3.62% ±4.82% ±6.28% zlib/creation.js n=500000 options='false' type='DeflateRaw' 0.49 % ±4.94% ±6.58% ±8.56% zlib/creation.js n=500000 options='false' type='Gunzip' -1.79 % ±3.66% ±4.88% ±6.35% zlib/creation.js n=500000 options='false' type='Gzip' 1.15 % ±3.55% ±4.72% ±6.15% zlib/creation.js n=500000 options='false' type='Inflate' 2.92 % ±3.40% ±4.53% ±5.90% zlib/creation.js n=500000 options='false' type='InflateRaw' -0.07 % ±3.30% ±4.39% ±5.73% zlib/creation.js n=500000 options='false' type='Unzip' -3.96 % ±4.35% ±5.80% ±7.58% zlib/creation.js n=500000 options='true' type='BrotliCompress' 0.78 % ±3.94% ±5.24% ±6.84% zlib/creation.js n=500000 options='true' type='BrotliDecompress' 1.69 % ±4.07% ±5.42% ±7.06% zlib/creation.js n=500000 options='true' type='Deflate' -3.07 % ±4.42% ±5.88% ±7.65% zlib/creation.js n=500000 options='true' type='DeflateRaw' 1.68 % ±3.75% ±5.00% ±6.53% zlib/creation.js n=500000 options='true' type='Gunzip' -1.02 % ±4.31% ±5.74% ±7.47% zlib/creation.js n=500000 options='true' type='Gzip' 2.04 % ±3.27% ±4.35% ±5.66% zlib/creation.js n=500000 options='true' type='Inflate' 0.89 % ±3.73% ±4.96% ±6.46% zlib/creation.js n=500000 options='true' type='InflateRaw' 0.66 % ±4.42% ±5.88% ±7.66% zlib/creation.js n=500000 options='true' type='Unzip' -2.21 % ±3.43% ±4.57% ±5.94% zlib/deflate.js n=400000 inputLen=1024 method='createDeflate' 8.23 % ±10.88% ±14.48% ±18.86% zlib/deflate.js n=400000 inputLen=1024 method='deflate' -3.03 % ±6.48% ±8.62% ±11.22% zlib/deflate.js n=400000 inputLen=1024 method='deflateSync' 11.92 % ±19.99% ±26.60% ±34.62% zlib/inflate.js n=400000 inputLen=1024 method='inflate' * -3.83 % ±3.43% ±4.56% ±5.95% zlib/inflate.js n=400000 inputLen=1024 method='inflateSync' * 6.14 % ±6.08% ±8.09% ±10.54% zlib/pipe.js algorithm='brotli' type='buffer' duration=5 inputLen=1024 0.73 % ±2.52% ±3.35% ±4.37% zlib/pipe.js algorithm='brotli' type='string' duration=5 inputLen=1024 -1.47 % ±2.28% ±3.04% ±3.97% zlib/pipe.js algorithm='gzip' type='buffer' duration=5 inputLen=1024 -1.18 % ±4.99% ±6.64% ±8.65% zlib/pipe.js algorithm='gzip' type='string' duration=5 inputLen=1024 3.73 % ±4.00% ±5.32% ±6.92%

This reverts commit 26991f7.

Restructure the zlib.gyp file based on the upstream gn file, breaking out the files with optimizations that need additional compiler flags. Use a copy of the GN-scraper.py script to reduce the amount of hand editing when the zlib dependency is updated.

nodejs-github-bot · 2022-11-25T18:54:35Z

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

nodejs-github-bot · 2022-12-04T08:14:12Z

Landed in 81fa992...7811d2d

This reverts commit 26991f7. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

Restructure the zlib.gyp file based on the upstream gn file, breaking out the files with optimizations that need additional compiler flags. Use a copy of the GN-scraper.py script to reduce the amount of hand editing when the zlib dependency is updated. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

Neustradamus · 2022-12-04T14:47:33Z

To follow this PR

This reverts commit 26991f7. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

Restructure the zlib.gyp file based on the upstream gn file, breaking out the files with optimizations that need additional compiler flags. Use a copy of the GN-scraper.py script to reduce the amount of hand editing when the zlib dependency is updated. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

This reverts commit 26991f7. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

Restructure the zlib.gyp file based on the upstream gn file, breaking out the files with optimizations that need additional compiler flags. Use a copy of the GN-scraper.py script to reduce the amount of hand editing when the zlib dependency is updated. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

This reverts commit 26991f7. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

Restructure the zlib.gyp file based on the upstream gn file, breaking out the files with optimizations that need additional compiler flags. Use a copy of the GN-scraper.py script to reduce the amount of hand editing when the zlib dependency is updated. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

This reverts commit 26991f7. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

Restructure the zlib.gyp file based on the upstream gn file, breaking out the files with optimizations that need additional compiler flags. Use a copy of the GN-scraper.py script to reduce the amount of hand editing when the zlib dependency is updated. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

This reverts commit 26991f7. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

Restructure the zlib.gyp file based on the upstream gn file, breaking out the files with optimizations that need additional compiler flags. Use a copy of the GN-scraper.py script to reduce the amount of hand editing when the zlib dependency is updated. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

This reverts commit 26991f7. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

Restructure the zlib.gyp file based on the upstream gn file, breaking out the files with optimizations that need additional compiler flags. Use a copy of the GN-scraper.py script to reduce the amount of hand editing when the zlib dependency is updated. PR-URL: #45589Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

nappy · 2023-02-20T16:40:16Z

On Android arm64 builds of 18.14.2 the cpu-features.h header/library referenced by cpu_features.c cannot be found. I suspect due to these changes,

nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem. labels Nov 22, 2022

lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 22, 2022

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 22, 2022

lpinca force-pushed the rework/zlib.gyp branch from d315a06 to 22f9b74Compare November 22, 2022 20:51