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.

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

please allow the use of the simdjson library provided by the distribution #52914

Closed
williamh opened this issue May 9, 2024 · 6 comments · Fixed by #52924
Closed

please allow the use of the simdjson library provided by the distribution #52914

williamh opened this issue May 9, 2024 · 6 comments · Fixed by #52924
Labels
build Issues and PRs related to build files or the CI. good first issue Issues that are suitable for first-time contributors.

Comments

@williamh
Copy link

williamh commented May 9, 2024

Hi, I am the primary maintainer of nodejs on Gentoo linux.
nodejs 22.1.0 bundles an old version of simdjson which causes it to not build with gcc-14.x.
see this bug for more information.
Can you please allow nodejs to build using the distribution provided libsimdjson maybe by allowing a configure toggle?
Thanks much for your time.

@benjamingr
Copy link
Member

@anonrig @lemire

@lemire
Copy link
Member

lemire commented May 9, 2024

nodejs 22.1.0 bundles an old version of simdjson which causes it to not build with gcc-14.x.

I think that Node.js 22.1.0 released with recent dependencies. I don't think that's a fault in Node.js.

A timeline might be useful (reverse chronological order).

  • We are May 9 2024. You can grab gentoo and build Node 22.1.0 with the bleeding edge compiler GCC 14 opted in. It always worked with default compiler (GCC 13).
  • Gentoo included GCC 14 in its tree on May 7, as an opt in. Trivially, gentoo could check that this would break some builds, but my understanding is that gentoo did this opt-in move to allow testing. That is, nobody should be using GCC 14 for production systems. This release of GCC 14 triggered an issue with gentoo users who had opted in for GCC 14 and wanted to build Node.js from source. This affected not just the latest release of Node.js, but also previous releases: the key ingredient being the breaking change introduced by GCC 14. A patch was applied on May 7 (same day) to the gentoo tree, fixing the issue.
  • The first GCC 14 public release (14.1) is dated from May 6 2024. It introduced a breaking change which affected the simdjson library with respect to releases prior to that of April 5 2024.
  • Node 22.1.0 was released May 2nd 2024. GCC 14 was not part of its CI testing at the time. It did not include the very latest versions from simdjson (although it did include a recent release, it missed the latest patch by a week or so).
  • On April 5, an issue was identified with a breaking change introduced by GCC 14 by RedHat: macros like __AVX2__ used to be constant within the compilation of a given unit, but was changed so that they reflect the current set of the enabled/disabled ISAs in the region. Once the issue had been identified and characterized, patched releases were produced from simdjson and simdutf (the same day). I believe that gentoo immediately adopted the newly patched version (same day).

I submit to you that the system works as it should. Currently, Node.js is still at simdjson 3.8, but 3.9 will eventually be folded in and be part of future releases.

I do not object to the unbundling.

@williamh
Copy link
Author

williamh commented May 9, 2024

I understand there are situations where bundling the library is helpful, but bundling has significant drawbacks as well, so Gentoo prefers using unbundled libraries when possible.
This is our documentation on the subject.
Please let me know what you think.

@lemire
Copy link
Member

lemire commented May 9, 2024

I understand there are situations where bundling the library is helpful, but bundling has significant drawbacks as well, so Gentoo prefers using unbundled libraries when possible.

The issue, as you raised it, as to do with unbundling simdjson specifically. It is also the specific subject of the gentoo bug. I submit to you that the argument should apply to ada, simdutf, ngtcp2, and possibly other bundled dependencies.

Correct?

@lemire lemire added build Issues and PRs related to build files or the CI. good first issue Issues that are suitable for first-time contributors. labels May 9, 2024
@lemire
Copy link
Member

lemire commented May 9, 2024

Currently, Node.js can be built with...

                --shared-brotli
		--shared-cares
		--shared-libuv
		--shared-nghttp2
		--shared-zlib

The flag...

               --shared-ngtcp2

is present but unused by Gentoo.

I think it might be possible to add...

                --shared-ada
		--shared-simdutf
		--shared-simdjson

It appears that both ada and simdutf are missing in gentoo, but ngtcp2 and simdjson are present:

I might be missing some dependencies.

@williamh
Copy link
Author

williamh commented May 9, 2024

You are correct; I will fix those dependencies.
Thanks for pointing that out.

jirutka added a commit to jirutka/node that referenced this issue Nov 16, 2024
The --shared-ada flag was introduced in nodejs#52924, but the implementation
was incomplete.

Resolves nodejs#52914
jirutka added a commit to jirutka/node that referenced this issue Nov 16, 2024
The --shared-simdjson flag was introduced in nodejs#52924, but the
implementation was incomplete.

Resolves nodejs#52914
jirutka added a commit to jirutka/node that referenced this issue Nov 16, 2024
The --shared-simdutf flag was introduced in nodejs#52924, but the
implementation was incomplete.

Resolves nodejs#52914
jirutka added a commit to jirutka/node that referenced this issue Nov 16, 2024
The --shared-ada flag was introduced in nodejs#52924, but the implementation
was incomplete.

Resolves nodejs#52914
jirutka added a commit to jirutka/node that referenced this issue Nov 16, 2024
The --shared-simdjson flag was introduced in nodejs#52924, but the
implementation was incomplete.

Resolves nodejs#52914
jirutka added a commit to jirutka/node that referenced this issue Nov 16, 2024
The --shared-simdutf flag was introduced in nodejs#52924, but the
implementation was incomplete.

Resolves nodejs#52914
nodejs-github-bot pushed a commit that referenced this issue Nov 18, 2024
The --shared-ada flag was introduced in #52924, but the implementation
was incomplete.

Resolves #52914

PR-URL: #55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
nodejs-github-bot pushed a commit that referenced this issue Nov 18, 2024
The --shared-simdjson flag was introduced in #52924, but the
implementation was incomplete.

Resolves #52914

PR-URL: #55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
nodejs-github-bot pushed a commit that referenced this issue Nov 18, 2024
The --shared-simdutf flag was introduced in #52924, but the
implementation was incomplete.

Resolves #52914

PR-URL: #55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
tpoisseau pushed a commit to tpoisseau/node that referenced this issue Nov 21, 2024
The --shared-ada flag was introduced in nodejs#52924, but the implementation
was incomplete.

Resolves nodejs#52914

PR-URL: nodejs#55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
tpoisseau pushed a commit to tpoisseau/node that referenced this issue Nov 21, 2024
The --shared-simdjson flag was introduced in nodejs#52924, but the
implementation was incomplete.

Resolves nodejs#52914

PR-URL: nodejs#55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
tpoisseau pushed a commit to tpoisseau/node that referenced this issue Nov 21, 2024
The --shared-simdutf flag was introduced in nodejs#52924, but the
implementation was incomplete.

Resolves nodejs#52914

PR-URL: nodejs#55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Nov 26, 2024
The --shared-ada flag was introduced in nodejs#52924, but the implementation
was incomplete.

Resolves nodejs#52914

PR-URL: nodejs#55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Nov 26, 2024
The --shared-simdjson flag was introduced in nodejs#52924, but the
implementation was incomplete.

Resolves nodejs#52914

PR-URL: nodejs#55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Nov 26, 2024
The --shared-simdutf flag was introduced in nodejs#52924, but the
implementation was incomplete.

Resolves nodejs#52914

PR-URL: nodejs#55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this issue Nov 26, 2024
The --shared-ada flag was introduced in #52924, but the implementation
was incomplete.

Resolves #52914

PR-URL: #55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this issue Nov 26, 2024
The --shared-simdjson flag was introduced in #52924, but the
implementation was incomplete.

Resolves #52914

PR-URL: #55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this issue Nov 26, 2024
The --shared-simdutf flag was introduced in #52924, but the
implementation was incomplete.

Resolves #52914

PR-URL: #55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this issue Dec 10, 2024
The --shared-ada flag was introduced in #52924, but the implementation
was incomplete.

Resolves #52914

PR-URL: #55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this issue Dec 10, 2024
The --shared-simdjson flag was introduced in #52924, but the
implementation was incomplete.

Resolves #52914

PR-URL: #55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
aduh95 pushed a commit that referenced this issue Dec 10, 2024
The --shared-simdutf flag was introduced in #52924, but the
implementation was incomplete.

Resolves #52914

PR-URL: #55886
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants