Skip to content

Conversation

@targos
Copy link
Member

@targos targos commented Nov 14, 2025

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
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/v8-update
@nodejs-github-bot 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 targos changed the title deps: V8: cherry-pick e0fb10b5148c [v24.x] deps: V8: cherry-pick e0fb10b5148c Nov 14, 2025
@LeszekSwirski
Copy link
Contributor

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

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
Copy link
Member Author

targos commented Nov 14, 2025

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 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
Copy link
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
Copy link
Member Author

targos commented Nov 14, 2025

CI looks good (tests also passed on my machine)

@aduh95
Copy link
Contributor

aduh95 commented Nov 14, 2025

Does this need any dont-land-… labels?

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

targos commented Nov 14, 2025

Added the labels

@targos targos added dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in 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 Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. labels Nov 14, 2025
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 14, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 14, 2025
@targos
Copy link
Member Author

targos commented Nov 14, 2025

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
Copy link
Contributor

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

@richardlau
Copy link
Member

richardlau commented Nov 14, 2025

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
Copy link
Member Author

targos commented Nov 14, 2025

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
Copy link
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:

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
Copy link
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.

targos added a commit that referenced this pull request Nov 25, 2025
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
Copy link
Member Author

targos commented Nov 25, 2025

Landed in 04e360f

@targos targos closed this Nov 25, 2025
@targos targos deleted the node-array-size branch November 25, 2025 08:35
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. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in 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. 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.

6 participants