GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — deps: update zlib to upstream fc5cfd78a3 by targos · Pull Request #41745 · nodejs/node

Conversation

@targos

@targos targos commented Jan 29, 2022

Copy link
Copy Markdown
Member

Fixes: #41744

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
targos and others added 3 commits January 29, 2022 13:13
@targos
deps: update zlib to upstream fc5cfd78a3
6170ee4 
Updated as described in doc/contributing/maintaining-zlib.md.
@addaleax @targos
deps: fix zlib compilation for CPUs without SIMD features
56e6a51 
Fix the compile flags so that zlib can run on CPUs that do
not have SSSE3/SSE4.2/etc. Do not compile zlib with flags that
indicate that those features are available, and instead enable
them selectively for functions that use them.

There are probably better way to do this, e.g. through gyp file
modifications as suggested in the issue. However, this patch
should do just fine until that happens.

Fixes: nodejs#32553

PR-URL: nodejs#32627
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@RaisinTen @targos
build: fix zlib inlining for IA-32
ec258fe 
PR-URL: nodejs#35679
Fixes: nodejs#35629
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@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 Jan 29, 2022
@targos targos mentioned this pull request Jan 29, 2022
deps: update zlib to upstream 8603eee #34183
Closed
@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jan 29, 2022
@targos targos mentioned this pull request Jan 29, 2022
bundled zlib is missing an upstream UB fix #41744
Closed
@targos

targos commented Jan 29, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Windows error: D:\a\node\node\deps\zlib\cpu_features.c(52,1): fatal error C1189: #error: cpu_features.c CPU feature detection in not defined for your platform [D:\a\node\node\deps\zlib\zlib.vcxproj]
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Jan 29, 2022
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@targos
fixup! deps: update zlib to upstream fc5cfd78a3
634ad79
@nodejs-github-bot

nodejs-github-bot commented Jan 29, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/42213/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos

targos commented Jan 29, 2022

Copy link
Copy Markdown
Member Author

Error remains on ARM64 Windows but that should be fixable in a similar way.
RaisinTen and benjamingr reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@RaisinTen

RaisinTen commented Jan 29, 2022

Copy link
Copy Markdown
Member

Don't we need #33044 to land first?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Jan 30, 2022
CI Reliability 2022-01-30 nodejs/reliability#187
Open
25 tasks
@targos

targos commented Jan 30, 2022

Copy link
Copy Markdown
Member Author

Don't we need #33044 to land first?

Why? I'd like that PR to land but it is not being worked on currently.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@RaisinTen

RaisinTen commented Jan 30, 2022

Copy link
Copy Markdown
Member

If the gyp files are not in sync with the upstream gn files, the compiler won't be passed the expected options which might lead to issues like incorrect code generation - #39313 (comment) or slowdowns - #33044 (comment).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos
build: default ARM FPU to "neon" on ARM64 Windows
5824778
@nodejs-github-bot

nodejs-github-bot commented Jan 30, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/42231/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos
fixup! build: default ARM FPU to "neon" on ARM64 Windows
a83aa1c
@targos

targos commented Jan 30, 2022

Copy link
Copy Markdown
Member Author

I think we'll need help from @nodejs/platform-windows-arm
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
benjamingr
benjamingr approved these changes Jan 30, 2022
View reviewed changes

@benjamingr benjamingr left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
cjihrig
cjihrig approved these changes Jan 30, 2022
View reviewed changes
This was referenced Jan 31, 2022
CI Reliability 2022-01-31 nodejs/reliability#188
Open
CI Reliability 2022-02-01 nodejs/reliability#189
Open
CI Reliability 2022-02-02 nodejs/reliability#190
Open
CI Reliability 2022-02-03 nodejs/reliability#191
Open
@RaisinTen

RaisinTen commented Feb 3, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Zlib benchmark CI: https://ci.nodejs.org/job/benchmark-node-micro-benchmarks/1093/

                                                                       confidence improvement accuracy (*)    (**)   (***)
zlib/createInflate.js n=100 chunkLen=1024 inputLen=16777216                            0.35 %       ±7.56% ±10.05% ±13.09%
zlib/creation.js n=500000 options='false' type='BrotliCompress'                       -1.65 %       ±4.25%  ±5.65%  ±7.36%
zlib/creation.js n=500000 options='false' type='BrotliDecompress'                     -3.81 %       ±4.78%  ±6.35%  ±8.27%
zlib/creation.js n=500000 options='false' type='Deflate'                               0.25 %       ±4.22%  ±5.62%  ±7.31%
zlib/creation.js n=500000 options='false' type='DeflateRaw'                           -2.12 %       ±3.54%  ±4.71%  ±6.13%
zlib/creation.js n=500000 options='false' type='Gunzip'                                1.68 %       ±4.18%  ±5.57%  ±7.27%
zlib/creation.js n=500000 options='false' type='Gzip'                                  2.04 %       ±4.08%  ±5.43%  ±7.07%
zlib/creation.js n=500000 options='false' type='Inflate'                              -0.78 %       ±3.50%  ±4.66%  ±6.07%
zlib/creation.js n=500000 options='false' type='InflateRaw'                           -2.67 %       ±2.78%  ±3.70%  ±4.82%
zlib/creation.js n=500000 options='false' type='Unzip'                                -2.04 %       ±3.31%  ±4.41%  ±5.74%
zlib/creation.js n=500000 options='true' type='BrotliCompress'                         0.84 %       ±4.88%  ±6.49%  ±8.44%
zlib/creation.js n=500000 options='true' type='BrotliDecompress'                       0.72 %       ±5.75%  ±7.65%  ±9.97%
zlib/creation.js n=500000 options='true' type='Deflate'                               -3.15 %       ±3.92%  ±5.22%  ±6.80%
zlib/creation.js n=500000 options='true' type='DeflateRaw'                            -3.66 %       ±4.16%  ±5.54%  ±7.23%
zlib/creation.js n=500000 options='true' type='Gunzip'                                -0.02 %       ±3.74%  ±4.98%  ±6.49%
zlib/creation.js n=500000 options='true' type='Gzip'                                  -2.11 %       ±4.29%  ±5.71%  ±7.43%
zlib/creation.js n=500000 options='true' type='Inflate'                                1.08 %       ±3.96%  ±5.27%  ±6.86%
zlib/creation.js n=500000 options='true' type='InflateRaw'                             2.51 %       ±4.65%  ±6.19%  ±8.07%
zlib/creation.js n=500000 options='true' type='Unzip'                          **     -5.13 %       ±3.42%  ±4.55%  ±5.93%
zlib/deflate.js n=400000 inputLen=1024 method='createDeflate'                         -1.70 %       ±9.96% ±13.25% ±17.25%
zlib/deflate.js n=400000 inputLen=1024 method='deflate'                       ***    -10.67 %       ±5.75%  ±7.65%  ±9.97%
zlib/deflate.js n=400000 inputLen=1024 method='deflateSync'                   ***    -23.99 %      ±10.14% ±13.49% ±17.56%
zlib/inflate.js n=400000 inputLen=1024 method='inflate'                               -1.77 %       ±3.60%  ±4.80%  ±6.26%
zlib/inflate.js n=400000 inputLen=1024 method='inflateSync'                           -3.80 %       ±4.35%  ±5.79%  ±7.55%
zlib/pipe.js algorithm='brotli' type='buffer' duration=5 inputLen=1024                 2.06 %       ±2.10%  ±2.79%  ±3.65%
zlib/pipe.js algorithm='brotli' type='string' duration=5 inputLen=1024                -0.82 %       ±2.79%  ±3.72%  ±4.84%
zlib/pipe.js algorithm='gzip' type='buffer' duration=5 inputLen=1024                  -2.51 %       ±4.97%  ±6.62%  ±8.63%
zlib/pipe.js algorithm='gzip' type='string' duration=5 inputLen=1024            *     -7.94 %       ±6.27%  ±8.38% ±10.97%
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
RaisinTen
RaisinTen requested changes Feb 4, 2022
View reviewed changes

@RaisinTen RaisinTen left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some performance regressions which need to be fixed before landing this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@targos

targos commented Feb 4, 2022

Copy link
Copy Markdown
Member Author

Well, I tried :(
I'm closing the PR because I personally don't know how to go further with it, but keeping the branch in case someone wants to use it as a base.
RaisinTen reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos targos closed this Feb 4, 2022
@RaisinTen RaisinTen mentioned this pull request Mar 13, 2022
test,crypto: add and update empty passphrase regression tests #42319
Merged
@aduh95 aduh95 mentioned this pull request Jul 19, 2022
lib: refactor PriorityQueue to use private field #43889
Merged
@targos targos mentioned this pull request Aug 17, 2022
Is maintaining-zlib document up-to-date? #44254
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RaisinTen RaisinTen RaisinTen requested changes
@benjamingr benjamingr benjamingr approved these changes
@cjihrig cjihrig cjihrig approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bundled zlib is missing an upstream UB fix

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@targos @nodejs-github-bot @RaisinTen @benjamingr @cjihrig @addaleax
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.