Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

zlib: disable CRC32 SIMD optimization #49511

Merged
merged 1 commit into from
Sep 10, 2023
Merged

zlib: disable CRC32 SIMD optimization #49511

merged 1 commit into from
Sep 10, 2023

Conversation

lpinca
Copy link
Member

@lpinca lpinca commented Sep 6, 2023

It seems that the optimization causes memory corruption. Disable it until the issue is fixed upstream.

Fixes: #45268

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot 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 Sep 6, 2023
@lpinca lpinca force-pushed the disable/zlib-optimization branch 2 times, most recently from d39b222 to a7bb3ee Compare September 6, 2023 10:35
@lpinca
zlib: disable CRC32 SIMD optimization
550a760 
It seems that the optimization causes memory corruption. Disable it
until the issue is fixed upstream.

Fixes: nodejs#45268
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 6, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 6, 2023
@nodejs-github-bot
Copy link
Collaborator

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

@lpinca lpinca mentioned this pull request Sep 7, 2023
Closed
@nodejs-github-bot
Copy link
Collaborator

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

This was referenced Sep 8, 2023
Open
Open
@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Sep 10, 2023
Open
19 tasks
@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 10, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 10, 2023
@nodejs-github-bot nodejs-github-bot merged commit ae115d6 into nodejs:main Sep 10, 2023
53 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in ae115d6

@lpinca lpinca deleted the disable/zlib-optimization branch September 10, 2023 05:59
@github-actions github-actions bot mentioned this pull request Sep 11, 2023
Open
18 tasks
@lpinca lpinca added needs-benchmark-ci PR that need a benchmark CI run. and removed needs-benchmark-ci PR that need a benchmark CI run. labels Sep 11, 2023
@lpinca
Copy link
Member Author

lpinca commented Sep 11, 2023 •
edited
Loading

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

                                                                       confidence improvement accuracy (*)   (**)  (***)
zlib/createInflate.js n=100 chunkLen=1024 inputLen=16777216                           -0.11 %       ±1.63% ±2.18% ±2.85%
zlib/creation.js n=500000 options='false' type='BrotliCompress'                       -0.41 %       ±1.24% ±1.65% ±2.15%
zlib/creation.js n=500000 options='false' type='BrotliDecompress'                      0.26 %       ±1.13% ±1.50% ±1.96%
zlib/creation.js n=500000 options='false' type='Deflate'                               0.75 %       ±1.34% ±1.78% ±2.32%
zlib/creation.js n=500000 options='false' type='DeflateRaw'                           -0.42 %       ±1.59% ±2.12% ±2.75%
zlib/creation.js n=500000 options='false' type='Gunzip'                               -0.01 %       ±1.27% ±1.69% ±2.20%
zlib/creation.js n=500000 options='false' type='Gzip'                                  0.16 %       ±1.59% ±2.12% ±2.76%
zlib/creation.js n=500000 options='false' type='Inflate'                              -1.09 %       ±1.37% ±1.82% ±2.37%
zlib/creation.js n=500000 options='false' type='InflateRaw'                            0.27 %       ±1.18% ±1.57% ±2.05%
zlib/creation.js n=500000 options='false' type='Unzip'                                 0.16 %       ±1.49% ±1.99% ±2.59%
zlib/creation.js n=500000 options='true' type='BrotliCompress'                         0.32 %       ±1.33% ±1.78% ±2.32%
zlib/creation.js n=500000 options='true' type='BrotliDecompress'                       0.22 %       ±1.32% ±1.76% ±2.32%
zlib/creation.js n=500000 options='true' type='Deflate'                               -0.66 %       ±1.91% ±2.54% ±3.30%
zlib/creation.js n=500000 options='true' type='DeflateRaw'                             0.50 %       ±1.35% ±1.80% ±2.34%
zlib/creation.js n=500000 options='true' type='Gunzip'                                 0.07 %       ±1.37% ±1.83% ±2.38%
zlib/creation.js n=500000 options='true' type='Gzip'                                  -0.35 %       ±1.39% ±1.86% ±2.42%
zlib/creation.js n=500000 options='true' type='Inflate'                               -0.71 %       ±1.46% ±1.94% ±2.52%
zlib/creation.js n=500000 options='true' type='InflateRaw'                             0.62 %       ±1.45% ±1.93% ±2.52%
zlib/creation.js n=500000 options='true' type='Unzip'                                  1.08 %       ±1.30% ±1.73% ±2.25%
zlib/deflate.js n=400000 inputLen=1024 method='createDeflate'                 ***     13.00 %       ±2.29% ±3.06% ±4.00%
zlib/deflate.js n=400000 inputLen=1024 method='deflate'                         *      2.06 %       ±1.60% ±2.13% ±2.79%
zlib/deflate.js n=400000 inputLen=1024 method='deflateSync'                   ***      4.17 %       ±1.22% ±1.63% ±2.12%
zlib/inflate.js n=400000 inputLen=1024 method='inflate'                               -1.60 %       ±2.60% ±3.49% ±4.58%
zlib/inflate.js n=400000 inputLen=1024 method='inflateSync'                           -0.05 %       ±1.54% ±2.05% ±2.67%
zlib/pipe.js algorithm='brotli' type='buffer' duration=5 inputLen=1024                -1.22 %       ±2.21% ±2.94% ±3.85%
zlib/pipe.js algorithm='brotli' type='string' duration=5 inputLen=1024                -0.45 %       ±2.28% ±3.04% ±3.96%
zlib/pipe.js algorithm='gzip' type='buffer' duration=5 inputLen=1024          ***     23.98 %       ±2.70% ±3.61% ±4.73%
zlib/pipe.js algorithm='gzip' type='string' duration=5 inputLen=1024          ***     35.61 %       ±3.38% ±4.51% ±5.92%

@lpinca
Copy link
Member Author

lpinca commented Sep 12, 2023

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

                                                                       confidence improvement accuracy (*)   (**)   (***)
zlib/createInflate.js n=100 chunkLen=1024 inputLen=16777216                     *     -2.28 %       ±2.16% ±2.88%  ±3.75%
zlib/creation.js n=500000 options='false' type='BrotliCompress'                        0.10 %       ±1.40% ±1.87%  ±2.43%
zlib/creation.js n=500000 options='false' type='BrotliDecompress'                     -0.15 %       ±1.08% ±1.44%  ±1.87%
zlib/creation.js n=500000 options='false' type='Deflate'                               0.78 %       ±1.54% ±2.05%  ±2.68%
zlib/creation.js n=500000 options='false' type='DeflateRaw'                           -0.23 %       ±1.34% ±1.79%  ±2.33%
zlib/creation.js n=500000 options='false' type='Gunzip'                               -1.30 %       ±1.78% ±2.38%  ±3.11%
zlib/creation.js n=500000 options='false' type='Gzip'                                 -0.98 %       ±1.17% ±1.55%  ±2.02%
zlib/creation.js n=500000 options='false' type='Inflate'                               0.15 %       ±1.46% ±1.94%  ±2.53%
zlib/creation.js n=500000 options='false' type='InflateRaw'                           -0.42 %       ±2.90% ±3.88%  ±5.07%
zlib/creation.js n=500000 options='false' type='Unzip'                          *     -1.84 %       ±1.64% ±2.19%  ±2.87%
zlib/creation.js n=500000 options='true' type='BrotliCompress'                         1.16 %       ±2.33% ±3.11%  ±4.07%
zlib/creation.js n=500000 options='true' type='BrotliDecompress'                *      1.24 %       ±1.07% ±1.43%  ±1.86%
zlib/creation.js n=500000 options='true' type='Deflate'                                2.11 %       ±2.90% ±3.88%  ±5.09%
zlib/creation.js n=500000 options='true' type='DeflateRaw'                             0.26 %       ±1.39% ±1.85%  ±2.41%
zlib/creation.js n=500000 options='true' type='Gunzip'                                 0.58 %       ±1.36% ±1.82%  ±2.36%
zlib/creation.js n=500000 options='true' type='Gzip'                                   0.29 %       ±1.22% ±1.63%  ±2.12%
zlib/creation.js n=500000 options='true' type='Inflate'                         *     -3.10 %       ±2.78% ±3.72%  ±4.91%
zlib/creation.js n=500000 options='true' type='InflateRaw'                            -0.47 %       ±1.23% ±1.64%  ±2.14%
zlib/creation.js n=500000 options='true' type='Unzip'                                  0.35 %       ±1.21% ±1.60%  ±2.09%
zlib/deflate.js n=400000 inputLen=1024 method='createDeflate'                 ***      9.51 %       ±3.55% ±4.77%  ±6.29%
zlib/deflate.js n=400000 inputLen=1024 method='deflate'                                3.20 %       ±3.89% ±5.22%  ±6.90%
zlib/deflate.js n=400000 inputLen=1024 method='deflateSync'                   ***      3.17 %       ±1.31% ±1.74%  ±2.27%
zlib/inflate.js n=400000 inputLen=1024 method='inflate'                               -1.55 %       ±1.84% ±2.45%  ±3.19%
zlib/inflate.js n=400000 inputLen=1024 method='inflateSync'                            1.38 %       ±1.56% ±2.07%  ±2.70%
zlib/pipe.js algorithm='brotli' type='buffer' duration=5 inputLen=1024                 0.10 %       ±2.01% ±2.68%  ±3.51%
zlib/pipe.js algorithm='brotli' type='string' duration=5 inputLen=1024                 0.36 %       ±2.77% ±3.70%  ±4.84%
zlib/pipe.js algorithm='gzip' type='buffer' duration=5 inputLen=1024          ***     26.79 %       ±3.21% ±4.27%  ±5.56%
zlib/pipe.js algorithm='gzip' type='string' duration=5 inputLen=1024          ***     33.82 %       ±6.22% ±8.28% ±10.79%

ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@lpinca @ruyadorno
zlib: disable CRC32 SIMD optimization
c23c60f 
It seems that the optimization causes memory corruption. Disable it
until the issue is fixed upstream.

Fixes: #45268
PR-URL: #49511
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
This was referenced Sep 28, 2023
Closed
Merged
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@lpinca @alexfernandez
zlib: disable CRC32 SIMD optimization
514da58 
It seems that the optimization causes memory corruption. Disable it
until the issue is fixed upstream.

Fixes: nodejs#45268
PR-URL: nodejs#49511
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@anonrig anonrig anonrig approved these changes

Assignees
No one assigned
Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Memory corruption and crash when streaming to zlib
4 participants
@lpinca @nodejs-github-bot @bnoordhuis @anonrig