FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[v24.x] deps: V8: cherry-pick 06bf293610ef, 146962dda8d2 and e0fb10b5148c by targos · Pull Request #60713 · nodejs/node · GitHub

/ node Public

[v24.x] deps: V8: cherry-pick 06bf293610ef, 146962dda8d2 and e0fb10b5148c - #60713

Closed
targos wants to merge 1 commit into
nodejs:v24.x-stagingfrom
targos:node-array-size
ConversationCommits (1)ChecksFiles changed
Closed

[v24.x] deps: V8: cherry-pick 06bf293610ef, 146962dda8d2 and e0fb10b5148c#60713
targos wants to merge 1 commit into
nodejs:v24.x-stagingfrom
targos:node-array-size

Conversation

targos commented Nov 14, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Cherry-pick 06bf293610ef.
Original commit message:

[tagged] Make FreeSpace a HeapObjectLayout

Bug: 42202654
Change-Id: I2c5d1a69d9bf0272b631e3fa7964026f3ccded11
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6596552
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#100564}

Refs: v8/v8@06bf293

Cherry-pick 146962dda8d2.
Original commit message:

[heap] Store FreeSpace size in multiples of tagged words

Since FreeSpace has to be aligned to Tagged words, we can support larger
free spaces by storing the size in words rather than bytes.

Bug: 417413670
Change-Id: I19ef4921e00a5ec23d39ff4aa5b379b36fc86e0a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6596680
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#100590}

Refs: v8/v8@146962d

Cherry-pick e0fb10b5148c.
Original commit message:

[array] Increase the maximum size of FixedArrays

Use the newly increased maximum FreeSpace size to allow a larger upper
bound for FixedArray/FixedDoubleArray size.

Bug: 417413670
Change-Id: I655c98bb68dfe033ae62f2b16441c62bc4403058
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6597277
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#100593}

Refs: v8/v8@e0fb10b

nodejs-github-bot commented Nov 14, 2025

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/v8-update

nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch. v8 engine Issues and PRs related to the V8 dependency. labels Nov 14, 2025
targos changed the title deps: V8: cherry-pick e0fb10b5148c [v24.x] deps: V8: cherry-pick e0fb10b5148c Nov 14, 2025
targos mentioned this pull request Nov 14, 2025
Closed

LeszekSwirski commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

You'll want to cherry-pick https://chromium-review.googlesource.com/c/v8/v8/+/6596680 as well, which is necessary to enable this.

deps: V8: cherry-pick 06bf293610ef, 146962dda8d2 and e0fb10b5148c
8ceaaf4 
Cherry-pick 06bf293610ef.
Original commit message:

    [tagged] Make FreeSpace a HeapObjectLayout

    Bug: 42202654
    Change-Id: I2c5d1a69d9bf0272b631e3fa7964026f3ccded11
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6596552
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Auto-Submit: Leszek Swirski <leszeks@chromium.org>
    Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#100564}

Refs: v8/v8@06bf293

Cherry-pick 146962dda8d2.
Original commit message:

    [heap] Store FreeSpace size in multiples of tagged words

    Since FreeSpace has to be aligned to Tagged words, we can support larger
    free spaces by storing the size in words rather than bytes.

    Bug: 417413670
    Change-Id: I19ef4921e00a5ec23d39ff4aa5b379b36fc86e0a
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6596680
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Auto-Submit: Leszek Swirski <leszeks@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#100590}

Refs: v8/v8@146962d

Cherry-pick e0fb10b5148c.
Original commit message:

    [array] Increase the maximum size of FixedArrays

    Use the newly increased maximum FreeSpace size to allow a larger upper
    bound for FixedArray/FixedDoubleArray size.

    Bug: 417413670
    Change-Id: I655c98bb68dfe033ae62f2b16441c62bc4403058
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6597277
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Igor Sheludko <ishell@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#100593}

Refs: v8/v8@e0fb10b

targos commented Nov 14, 2025

Copy link
Copy Markdown
Member Author

OK, thanks @LeszekSwirski. I also had to cherry-pick https://chromium-review.googlesource.com/c/v8/v8/+/6596552 so the other patch could be applied.

targos changed the title [v24.x] deps: V8: cherry-pick e0fb10b5148c [v24.x] deps: V8: cherry-pick 06bf293610ef, 146962dda8d2 and e0fb10b5148c Nov 14, 2025

LeszekSwirski commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Makes sense, doesn't hurt to merge that one too (if it causes issues, the other patch can also be adjusted to the older FreeSpace object definition)

targos commented Nov 14, 2025

Copy link
Copy Markdown
Member Author

CI looks good (tests also passed on my machine)

aduh95 commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Does this need any dont-land-… labels?

targos added v20.x v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. labels Nov 14, 2025

targos commented Nov 14, 2025

Copy link
Copy Markdown
Member Author

Added the labels

targos added dont-land-on-v20.x dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. and removed v20.x v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. labels Nov 14, 2025
richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 14, 2025
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 14, 2025

nodejs-github-bot commented Nov 14, 2025

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Nov 14, 2025

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6859/

nodejs-github-bot commented Nov 14, 2025

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6859/

nodejs-github-bot commented Nov 14, 2025

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6859/

targos commented Nov 14, 2025

Copy link
Copy Markdown
Member Author

Error with s390:

14:40:32 ccache g++ -MD -MF obj/v8_libbase/bignum.o.d -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_OZONE=1 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_NONE -D_GLIBCXX_ASSERTIONS=1 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64 -DV8_INTL_SUPPORT -DV8_USE_EXTERNAL_STARTUP_DATA -DV8_ATOMIC_OBJECT_FIELD_WRITES -DV8_ENABLE_LAZY_SOURCE_POSITIONS -DV8_WIN64_UNWINDING_INFO -DV8_ENABLE_REGEXP_INTERPRETER_THREADED_DISPATCH -DV8_ENABLE_SPARKPLUG -DV8_ENABLE_MAGLEV -DV8_ENABLE_TURBOFAN -DV8_ENABLE_WEBASSEMBLY -DV8_ENABLE_CONTINUATION_PRESERVED_EMBEDDER_DATA -DV8_ALLOCATION_FOLDING -DV8_ALLOCATION_SITE_TRACKING -DV8_ADVANCED_BIGINT_ALGORITHMS -DV8_USE_ZLIB -DV8_ENABLE_MAGLEV_GRAPH_PRINTER -DV8_ENABLE_EXTENSIBLE_RO_SNAPSHOT -DV8_ENABLE_BLACK_ALLOCATED_PAGES -DV8_ENABLE_LEAPTIERING -DV8_WASM_RANDOM_FUZZERS -DV8_ARRAY_BUFFER_INTERNAL_FIELD_COUNT=0 -DV8_ARRAY_BUFFER_VIEW_INTERNAL_FIELD_COUNT=0 -DV8_PROMISE_INTERNAL_FIELD_COUNT=0 -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -DV8_HAVE_TARGET_OS -DV8_TARGET_OS_LINUX -DCPPGC_ENABLE_LARGER_CAGE -DCPPGC_SLIM_WRITE_BARRIER -DV8_TARGET_ARCH_S390X -DABSL_ALLOCATOR_NOTHROW=1 -I../.. -Igen -I../../include -I../../third_party/abseil-cpp -Wall -Wno-unused-local-typedefs -Wno-maybe-uninitialized -Wno-deprecated-declarations -Wno-comments -Wno-packed-not-aligned -Wno-missing-field-initializers -Wno-unused-parameter -Wno-psabi -fno-strict-overflow -fno-ident -fno-strict-aliasing -fstack-protector -funwind-tables -fPIC -pipe -pthread -m64 -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -fno-omit-frame-pointer -g0 -ffp-contract=off -march=z196 -Wno-invalid-offsetof -Wno-strict-overflow -Wno-return-type -Wno-int-in-bool-context -Wno-deprecated -Wno-stringop-overflow -Wno-stringop-overread -Wno-restrict -Wno-array-bounds -Wno-nonnull -Wno-dangling-pointer -O3 -fdata-sections -ffunction-sections -fno-math-errno -fvisibility=default -Wno-narrowing -Wno-class-memaccess -Wno-invalid-offsetof -std=gnu++2a -fno-exceptions -fno-rtti  -c ../../src/base/numbers/bignum.cc -o obj/v8_libbase/bignum.o
14:40:32 In file included from ../../src/base/numbers/bignum.h:8,
14:40:32                  from ../../src/base/numbers/bignum.cc:5:
14:40:32 ../../src/base/vector.h: In static member function ‘static v8::base::OwnedVector<T> v8::base::OwnedVector<T>::NewForOverwrite(size_t)’:
14:40:32 ../../src/base/vector.h:296:32: error: ‘make_unique_for_overwrite’ is not a member of ‘std’
14:40:32   296 |     return OwnedVector<T>(std::make_unique_for_overwrite<T[]>(size), size);
14:40:32       |                                ^~~~~~~~~~~~~~~~~~~~~~~~~
14:40:32 ../../src/base/vector.h:296:26: error: expected primary-expression before ‘(’ token
14:40:32   296 |     return OwnedVector<T>(std::make_unique_for_overwrite<T[]>(size), size);
14:40:32       |                          ^
14:40:32 ../../src/base/vector.h:296:32: error: ‘make_unique_for_overwrite’ is not a member of ‘std’
14:40:32   296 |     return OwnedVector<T>(std::make_unique_for_overwrite<T[]>(size), size);
14:40:32       |                                ^~~~~~~~~~~~~~~~~~~~~~~~~
14:40:32 ../../src/base/vector.h:296:59: error: expected primary-expression before ‘[’ token
14:40:32   296 |     return OwnedVector<T>(std::make_unique_for_overwrite<T[]>(size), size);
14:40:32       |                                                           ^
14:40:32 ../../src/base/vector.h:296:60: error: expected primary-expression before ‘]’ token
14:40:32   296 |     return OwnedVector<T>(std::make_unique_for_overwrite<T[]>(size), size);
14:40:32       |                                                            ^
14:40:32 At global scope:
14:40:32 cc1plus: note: unrecognized command-line option ‘-Wno-dangling-pointer’ may have been intended to silence earlier diagnostics
14:40:32 cc1plus: note: unrecognized command-line option ‘-Wno-stringop-overread’ may have been intended to silence earlier diagnostics

This doesn't seem directly related to the changes (vector.h is untouched).
@nodejs/platform-s390

miladfarca commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

@targos is this using Clang++ ? We don't support gcc anymore.

richardlau commented Nov 14, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6859/

This looks weird

13:38:23 + export BUILD_TOOLS=/home/iojs/build-tools
13:38:23 + BUILD_TOOLS=/home/iojs/build-tools
13:38:23 + export LD_LIBRARY_PATH=/home/iojs/build-tools:/opt/rh/gcc-toolset-12/root/usr/lib64:/opt/rh/gcc-toolset-12/root/usr/lib
13:38:23 + LD_LIBRARY_PATH=/home/iojs/build-tools:/opt/rh/gcc-toolset-12/root/usr/lib64:/opt/rh/gcc-toolset-12/root/usr/lib
13:38:23 + export PATH=/home/iojs/build-tools:/opt/rh/gcc-toolset-12/root/usr/bin:/home/iojs/build/workspace/node-test-commit-v8-linux/depot_tools:/home/iojs/venv/bin:/home/iojs/nghttp2/src:/home/iojs/wrk:/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
13:38:23 + PATH=/home/iojs/build-tools:/opt/rh/gcc-toolset-12/root/usr/bin:/home/iojs/build/workspace/node-test-commit-v8-linux/depot_tools:/home/iojs/venv/bin:/home/iojs/nghttp2/src:/home/iojs/wrk:/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
13:38:25 + case "$CXX" in
13:38:25 + FORCE_PYTHON2=true
13:38:25 + ./configure --verbose
13:38:25 Node.js configure: Found Python 3.12.11...
13:38:25 Detected C++ compiler (CXX=ccache g++) version: 10.3.1
13:38:25 WARNING: C++ compiler (CXX=ccache g++, 10.3.1) too old, need g++ 12.2.0 or clang++ 8.0.0
13:38:25 Detected C compiler (CC=ccache gcc) version: 10.3.1

i.e. we're supposed to have enabled gcc-toolset-12 but configure says we're using gcc 10.3.1 (which isn't going to work).

targos commented Nov 14, 2025

Copy link
Copy Markdown
Member Author

The equivalent run for #60712 didn't fail: https://ci.nodejs.org/job/node-test-commit-v8-linux/6860/nodes=rhel8-s390x,v8test=v8test/consoleFull
But it was executed on a different host.

richardlau commented Nov 14, 2025

Copy link
Copy Markdown
Member

I've figured out what has happened.

[iojs@test-ibm-rhel8-s390x-4 ~]$ ls -al build-tools/
total 8
drwxr-xr-x  2 iojs iojs 4096 Nov 13 07:11 .
drwx------ 16 iojs iojs 4096 Nov 14 08:38 ..
lrwxrwxrwx  1 iojs iojs   39 Nov 13 07:11 g++ -> /opt/rh/gcc-toolset-10/root/usr/bin/g++
lrwxrwxrwx  1 iojs iojs   39 Nov 13 07:11 gcc -> /opt/rh/gcc-toolset-10/root/usr/bin/gcc
lrwxrwxrwx  1 iojs iojs   20 Apr  2  2025 gn -> /home/iojs/gn/out/gn
[iojs@test-ibm-rhel8-s390x-4 ~]$

This is from tools/make-v8.sh on the v20.x-staging branch:

node/tools/make-v8.sh

Line 41 in e65b930

ln -s "$CXX_PATH" "$BUILD_TOOLS/g++"

What has happened is that I undid a lot of the machine specific stuff in tools/make-v8.sh as part of #59893 so we no longer create the symlinks, but that hasn't been backported to v20.x-staging and I ran a V8 CI for that yesterday to test.

I'll remove the symlink and look at cherry-picking/backporting #59893 to v20.x-staging so that it doesn't happen again.

richardlau commented Nov 14, 2025

Copy link
Copy Markdown
Member

I'll remove the symlink and look at cherry-picking/backporting #59893 to v20.x-staging so that it doesn't happen again.

Cherry-pick is done (found another issue with the V8 CI and v20.x-staging but that won't affect his PR). I've also added explicit removal of the symlinks in the Jenkins job, so even if someone runs the CI on older commit hashes (e.g. without #59893) the job will remove the symlinks prior to running the build.

nodejs-github-bot commented Nov 15, 2025

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Nov 15, 2025

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6866/

nodejs-github-bot commented Nov 16, 2025

Copy link
Copy Markdown
Collaborator

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

targos added a commit that referenced this pull request Nov 25, 2025
deps: V8: cherry-pick 06bf293610ef, 146962dda8d2 and e0fb10b5148c
04e360f 
Cherry-pick 06bf293610ef.
Original commit message:

    [tagged] Make FreeSpace a HeapObjectLayout

    Bug: 42202654
    Change-Id: I2c5d1a69d9bf0272b631e3fa7964026f3ccded11
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6596552
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Auto-Submit: Leszek Swirski <leszeks@chromium.org>
    Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#100564}

Refs: v8/v8@06bf293

Cherry-pick 146962dda8d2.
Original commit message:

    [heap] Store FreeSpace size in multiples of tagged words

    Since FreeSpace has to be aligned to Tagged words, we can support larger
    free spaces by storing the size in words rather than bytes.

    Bug: 417413670
    Change-Id: I19ef4921e00a5ec23d39ff4aa5b379b36fc86e0a
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6596680
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Auto-Submit: Leszek Swirski <leszeks@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#100590}

Refs: v8/v8@146962d

Cherry-pick e0fb10b5148c.
Original commit message:

    [array] Increase the maximum size of FixedArrays

    Use the newly increased maximum FreeSpace size to allow a larger upper
    bound for FixedArray/FixedDoubleArray size.

    Bug: 417413670
    Change-Id: I655c98bb68dfe033ae62f2b16441c62bc4403058
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6597277
    Commit-Queue: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Igor Sheludko <ishell@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#100593}

Refs: v8/v8@e0fb10b
PR-URL: #60713
Reviewed-By: Richard Lau <richard.lau@ibm.com>

targos commented Nov 25, 2025

Copy link
Copy Markdown
Member Author

Landed in 04e360f

targos closed this Nov 25, 2025
targos deleted the node-array-size branch November 25, 2025 08:35
github-actions Bot mentioned this pull request Dec 9, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

richardlau richardlau approved these changes

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL