-
-
Notifications
You must be signed in to change notification settings - Fork 34.4k
[v25.x] v8: changing total_allocated_bytes to avoid ABI changes #60800
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
Open
caiolima
wants to merge
160
commits into
nodejs:v25.x-staging
Choose a base branch
from
caiolima:backport-60573-to-v25
base: v25.x-staging
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
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
Collaborator
|
Review requested:
|
This was referenced Nov 21, 2025
RafaelGSS
approved these changes
Nov 21, 2025
Member
RafaelGSS
left a comment
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.
LGTM.
joyeecheung
approved these changes
Nov 22, 2025
Collaborator
This reverts commit 8a6de38. PR-URL: nodejs#60774 Reviewed-By: Richard Lau <richard.lau@ibm.com> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Contributor
Author
|
Hi all, could someone help me trigger CI again, please? |
Instead of polyfilling it with vm.SourceTextModule, use a built-in source text module loader so that we can also build the code cache for it at build tiem to embed the code cache for them in the binary. Drive-by: instead of inferring how to compile a particular built-in at run time, do the inferring at build time, so the function-based built-ins can be compiled using parameters quickly looked up from a static map, and the builtins that should be compiled as source text modules are known internally based on extension in the source code (at run time, the extensions are all removed). PR-URL: nodejs#60518 Reviewed-By: Aditi Singh <aditisingh1400@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: nodejs#60641 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
This test contains too many independent test cases and as a result, marking it as flaky on all major platforms means actual regressions could be covered up, and it's constantly making the CI orange and requires extra resuming on the flaked platforms which is still not great. Split it into individual files so that the actual flake can be identified out of the monolith. PR-URL: nodejs#60653 Refs: nodejs#54534 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Recent architectures like RISC-V does not support open syscall,
which will cause strace to fail and thus test failure.
AssertionError [ERR_ASSERTION]: strace: invalid system call 'open'
This patch disables tracing open syscall for RISC-V in the test to fix
the test failure.
PR-URL: nodejs#60588
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <richard.lau@ibm.com>
For riscv64, the most commonly supported paging mode is sv39, which allocates 256GiB of virtual address space for the user space. However, due to trap handler security mechanism in V8, creating a wasm memory will cost 8GiB of continuous virtual address space. In a fresh node repl, I could only create 27 WebAssembly.Memory instances. When the virtual address space is more fragmented, it is worse. The wpt tests are randomly failing on riscv64 due to insufficient virtual address space to create wasm memories. This PR fixes it by limiting the concurrency of the WPTRunner to prevent the tests from creating too many wasm memories. PR-URL: nodejs#60591 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#60645 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
PR-URL: nodejs#60644 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
The original benchmark uses a not very realistic fixture (it has a huge try-catch block that would throw on the first line and then export at the end, hardly representative of real-world code). Also, it measures the entire import including evaluation, not just parsing. This updates the name to import-cjs to be more accurate, and use the typescript.js as the fixture which has been reported to be slow to import, leading users to use require() to work around the peformance impact. It splits the measurement into two different types: parsing CJS for the first time (where the overhead of loading the lexer makes a difference) and parsing CJS after the lexer has been loaded. PR-URL: nodejs#60663 Refs: nodejs#59913 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
The synchronous version has been available since 1.4.0. PR-URL: nodejs#60663 Refs: nodejs#59913 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#60675 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
PR-URL: nodejs#60676 Refs: actions/runner-images#13046 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Richard Lau <richard.lau@ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Correct wrong object identifiers listed for slh-dsa algorithms in doc/api/crypto.md. the valid range is from id-slh-dsa-sha2-128s (2.16.840.1.101.3.4.3.20) through id-slh-dsa-shake-256f (2.16.840.1.101.3.4.3.31), as defined in the ietf draft-ietf-lamps-x509-slhdsa. Fixes: nodejs#60680 Refs: https://datatracker.ietf.org/doc/draft-ietf-lamps-x509-slhdsa/ PR-URL: nodejs#60681 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
PR-URL: nodejs#60623 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#60473 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#60586 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#60503 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#60690 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.31.3 to 4.31.6. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.31.6 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> PR-URL: nodejs#60926 Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 6.0.0 to 6.1.0. - [Release notes](https://github.com/actions/setup-python/releases) --- updated-dependencies: - dependency-name: actions/setup-python dependency-version: 6.1.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> PR-URL: nodejs#60927 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
It's a common ecosystem pattern to map a source root directory to `@/` but it requires special tooling support. This turns `#/*` into a more realistic alternative for that pattern. PR-URL: nodejs#60864 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Claudio Wunder <cwunder@gnome.org> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Bumps [mdast-util-to-hast](https://github.com/syntax-tree/mdast-util-to-hast) from 13.2.0 to 13.2.1. - [Release notes](https://github.com/syntax-tree/mdast-util-to-hast/releases) - [Commits](syntax-tree/mdast-util-to-hast@13.2.0...13.2.1) --- updated-dependencies: - dependency-name: mdast-util-to-hast dependency-version: 13.2.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> PR-URL: nodejs#60930 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#60893 Fixes: nodejs#60888 Fixes: nodejs#59515 Fixes: nodejs#56542 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: nodejs#60834 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Refs: nodejs#60746 (review) PR-URL: nodejs#60929 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#60376 Fixes: nodejs#60322 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Adds links to the File modes section from fsPromises.open(), fsPromises.mkdir(), and fs.mkdir() to improve discoverability of file permission documentation. PR-URL: nodejs#60286 Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
PR-URL: nodejs#60911 Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
PR-URL: nodejs#60912 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Collaborator
Original commit message:
[regexp] Fix modifiers for ChoiceNodes
Each alternative might modify flags when their sub-graph is emitted.
We need to restore flags to the value at the beginning of a ChoiceNode
for each alternative.
Drive-by: Move regexp-modifiers test out of harmony/
Fixed: 447583670
Change-Id: I9f41e51f34df7659461da0a4fcd28b7e157f52e1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6995181
Reviewed-by: Jakob Linke <jgruber@chromium.org>
Commit-Queue: Patrick Thier <pthier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#102838}
Refs: v8/v8@72b0e27
Refs: nodejs#60030
PR-URL: nodejs#60706
Backport-PR-URL: nodejs#60706
PR-URL: nodejs#60776 Refs: nodejs#60303 Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Contributor
Author
|
I'm getting permission errors to look what is failing on CI. Probably it already expired. Does any of you have permission to see what's failing? |
5669bd4 to
f6b05d5
Compare
9f2e230 to
65696e4
Compare
Contributor
Failed to start CI⚠ Commits were pushed since the last approving review: ⚠ - tools: fix linter warning in `test-shared.yml` ⚠ - doc: correct concurrency wording in test() documentation ⚠ - build: fix OpenSSL version parsing for OpenSSL < 3 ⚠ - test_runner: simplify code and make it more consistent ⚠ - repl: tab completion targets `` instead of `new ` ⚠ - tools: bump js-yaml from 4.1.0 to 4.1.1 in /tools/lint-md ⚠ - deps: update brotli to 1.2.0 ⚠ - deps: upgrade npm to 11.6.3 ⚠ - tools: dump config.gypi as json ⚠ - lib: prefer `call()` over `apply()` if argument list is not array ⚠ - src: rename config file testRunner to test ⚠ - src: add test flag to config file ⚠ - src: add permission flag to config file ⚠ - doc: fix webstorage config file property ⚠ - src: implicitly enable namespace in config ⚠ - doc: keep sidebar module visible when navigating docs ⚠ - deps: update sqlite to 3.51.0 ⚠ - deps: add temporal_rs 0.1.0 ⚠ - build: add temporal_capi gyp ⚠ - test: add basic temporal presence check ⚠ - build: ignore built-in temporal when building with shared lib ⚠ - url: remove array.reduce usage ⚠ - test_runner: fix line feed escaping in JUnit ⚠ - test: use `RegExp.escape` to improve test reliability ⚠ - doc: add additional codemods for deprecation ⚠ - test: prefer major GC in cppgc-object teardown ⚠ - test: ensure assertions are reached on more tests ⚠ - test: ensure assertions are reached on more tests ⚠ - test: ensure assertions are reached on more tests ⚠ - test: ensure assertions are reached on more tests ⚠ - tools: add temporal updater ⚠ - test: replace deprecated regex test assertions in http trailers test ⚠ - doc: add fullName property to SuiteContext ⚠ - test: add lint rule to forbid use of `assert.ok(/regex/.test(…))` ⚠ - events: repurpose `events.listenerCount()` to accept EventTargets ⚠ - src: mark unused private field as such ⚠ - src: simply uint32 to string as it must not fail ⚠ - lib: add lint rules for reflective function calls ⚠ - test: ensure assertions are reached on all tests ⚠ - test: fix debug test crashes caused by sea tests ⚠ - doc: mark module.register as active development ⚠ - util: safely inspect getter errors whose message throws ⚠ - node-api: fix data race and use-after-free in napi_threadsafe_function ⚠ - src: handle indexed properties in `process.env` ⚠ - deps: upgrade npm to 11.6.4 ⚠ - doc: fix typos in changelogs ⚠ - doc: correct 'event handle' to 'event handler' in Utf8Stream drop event ⚠ - test: skip failing tests when compiled without amaro ⚠ - doc: update debuglog examples to use 'foo-bar' instead of 'foo' ⚠ - src: use static_cast instead of C-style cast ⚠ - test: fix embedtest in debug windows ⚠ - doc: correct spelling in BUILDING.md ⚠ - doc: replace column with columnNumber in example of `util.getCallSites` ⚠ - deps: update corepack to 0.34.5 ⚠ - test: use `assert.match` for non-literal regexp tests ⚠ - doc: show the use of string expressions in the SQLTagStore example ⚠ - lib,test: fix jsdoc comments ⚠ - test: lint more `assert(regexp.test(...))` cases ⚠ - doc: clarify fileURLToPath security considerations ⚠ - util: improve textencoder encodeInto performance ⚠ - src: handle DER decoding errors from system certificates ⚠ - tools: don't fetch V8 deps in the source tree ⚠ - build: run embedtest with node_g when BUILDTYPE=Debug ⚠ - test: update WPT for WebCryptoAPI to 1e4933113d ⚠ - deps: update zlib to 1.3.1-63d7e16 ⚠ - deps: update sqlite to 3.51.1 ⚠ - deps,src: prepare for cpplint update ⚠ - tools: disable some new cpplint rules before update ⚠ - tools: update cpplint to 2.0.2 ⚠ - tools: refloat 10 Node.js patches to cpplint.py ⚠ - src: fix off-thread cert loading in bundled cert mode ⚠ - test: skip SEA inspect test if inspector is not available ⚠ - tools: update ESLint dependencies ⚠ - tools: remove deprecated ESLint plugins ⚠ - tools: replace deprecated eslint-plugin-markdown ⚠ - doc: add missing `zstd` to mjs example of zlib ⚠ - benchmark: fix incorrect base64 input in byteLength benchmark ⚠ - tools: run tests `--without-amaro` on test-shared macOS ⚠ - stream: fix isErrored/isWritable for WritableStreams ⚠ - tools: enforce trailing commas in `test/sequential` ⚠ - tools: enforce trailing commas in `test/es-module` ⚠ - tools: ignore more paths in GHA CI ⚠ - util: assert getCallSites does not invoke Error.prepareStackTrace ⚠ - meta: bump peter-evans/create-pull-request from 7.0.8 to 7.0.9 ⚠ - meta: bump actions/checkout from 5.0.1 to 6.0.0 ⚠ - meta: bump github/codeql-action from 4.31.3 to 4.31.6 ⚠ - meta: bump actions/setup-python from 6.0.0 to 6.1.0 ⚠ - module: allow subpath imports that start with `#/` ⚠ - tools: bump mdast-util-to-hast from 13.2.0 to 13.2.1 in /tools/doc ⚠ - src: implement Windows-1252 encoding support and update related tests ⚠ - http2,zlib: prefer `call()` over `apply()` if argument list is not array ⚠ - test: improve config-file permission test coverage ⚠ - esm: improve error messages for ambiguous module syntax ⚠ - doc: add File modes cross-references in fs methods ⚠ - tools: add some options and comments to `shell.nix` ⚠ - test: skip tests not passing without `NODE_OPTIONS` support ⚠ - deps: V8: cherry-pick 72b0e27bd936 ⚠ - lib: do not provide an empty Proxy from localStorage getter ⚠ - v8: changing total_allocated_bytes to avoid ABI changes ⚠ - increasing node embedder number ✘ Refusing to run CI on potentially unsafe PRhttps://github.com/nodejs/node/actions/runs/21052182155 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
author ready
PRs that have at least one approval, no pending requests for changes, and a CI started.
lib / src
Issues and PRs related to general changes in the lib or src directory.
needs-ci
PRs that need a full CI run.
request-ci-failed
An error occurred while starting CI via request-ci label, and manual interventon is needed.
v25.x
Issues that can be reproduced on v25.x or PRs targeting the v25.x-staging branch.
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.
As noted by @targos in #60429 (comment), the implementation of
total_allocated_byteschanges the ABI. This PR changes original V8 implementation to a version where there's no addition ofHeapStatistics::total_allocated_bytes_to V8'sHeapStatistics, and instead we get such values usingIsolate::GetTotalAllocatedBytes. This also changes code introduced by #60573 to use this new API on eitherv8.getHeapStatistics()andWorker::GetHeapStatistics. The goal here is to maketotal_allocated_bytesback portable to previous versions, but keeping thev8.getHeapStatistics()API stable once users transition from v25 to future versions. It also solves the ABI change introduced by https://github.com/nodejs/node/pull/60429/filesThere's also a fix on documentation text with missing
.and updating the code snippet to includetotal_allocated_byteson output.