build: rework gyp files for zlib by lpinca · Pull Request #45589 · nodejs/node

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

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

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: #45589 Fixes: #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: #45589 Fixes: #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: #45589 Fixes: #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: #45589 Fixes: #32856 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

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

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

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

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

This reverts commit 26991f7. PR-URL: #45589 Fixes: #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: #45589 Fixes: #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-actions Bot 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 22f9b74 Compare November 22, 2022 20:51