-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
[v20.x] backport V8 changes related to compile cache #56711
Open
joyeecheung
wants to merge
221
commits into
nodejs:v20.x-staging
Choose a base branch
from
joyeecheung:backport-cache-fixes
base: v20.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.
Open
[v20.x] backport V8 changes related to compile cache #56711
joyeecheung
wants to merge
221
commits into
nodejs:v20.x-staging
from
joyeecheung:backport-cache-fixes
Conversation
This file contains 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
PR-URL: nodejs#54619 Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#55255 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
Fixes: nodejs#55208 PR-URL: nodejs#55249 Reviewed-By: Yagiz Nizipli <[email protected]>
The actual implementation returns `outgoingMessage` itself, but not exactly `http.ServerResponse`. Refs: https://github.com/nodejs/node/blob/20d8b85d3493bec944de541a896e0165dd356345/lib/_http_outgoing.js#L712-L751 PR-URL: nodejs#55290 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Qingyu Deng <[email protected]>
PR-URL: nodejs#55334 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Bumps [step-security/harden-runner](https://github.com/step-security/harden-runner) from 2.9.1 to 2.10.1. - [Release notes](https://github.com/step-security/harden-runner/releases) - [Commits](step-security/harden-runner@5c7944e...91182cc) --- updated-dependencies: - dependency-name: step-security/harden-runner dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> PR-URL: nodejs#55220 Reviewed-By: Luigi Pinca <[email protected]>
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.26.6 to 3.26.10. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@4dd1613...e2b3eaf) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> PR-URL: nodejs#55221 Reviewed-By: Luigi Pinca <[email protected]>
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 4.5.0 to 4.6.0. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@e28ff12...b9fd7d1) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> PR-URL: nodejs#55222 Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#55361 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#55273 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
fix make errors that occur in coverage-clean case and coverage-test in Makefile PR-URL: nodejs#55287 Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
update test_util.cc for code coverage src/util-inl.h:PopFront() PR-URL: nodejs#55291 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#55312 Fixes: nodejs#55311 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#55116 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Claudio Wunder <[email protected]>
PR-URL: nodejs#55295 Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#55369 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Fixes: nodejs#23191 PR-URL: nodejs#55207 Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#55375 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Claudio Wunder <[email protected]>
PR-URL: nodejs#55379 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
As the documentation states, the `context.importAssertion` should be still supported and emit a warning. This is true for the `load` hook, but not correct for context of the `resolve` hook. This commit fixes the inconsistency. PR-URL: nodejs#55365 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
This adds support for nodetimers.promises.scheduler.wait on Mocktimers Refs: nodejs#55244 PR-URL: nodejs#55244 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#55187 Reviewed-By: Matteo Collina <[email protected]>
- The `Path` class does not support concatenation with the `+` operator, so use the `/` operator instead. - When concatenating paths, if the operand is an absolute path the previous path is ignored, so change `/include` to `include`. PR-URL: nodejs#55387 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Fixes: nodejs#55391 PR-URL: nodejs#55392 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Tim Perry <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]>
`test/parallel/test-dns-default-verbatim-false.js` is a duplicate of `test/parallel/test-dns-default-order-ipv4.js` and `test/parallel/test-dns-default-verbatim-true.js` is a duplicate of `test/parallel/test-dns-default-order-verbatim.js`. PR-URL: nodejs#55393 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]>
The current colour seems something went wrong when in fact it's just someone asking for a review. PR-URL: nodejs#55423 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
Refs: nodejs/Release#1042 PR-URL: nodejs#55399 Refs: nodejs/Release#1036 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Refs: nodejs/Release#1040 PR-URL: nodejs#55399 Refs: nodejs/Release#1036 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Refs: nodejs/Release#1041 PR-URL: nodejs#55399 Refs: nodejs/Release#1036 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#55158 Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
PR-URL: nodejs#56251 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
PR-URL: nodejs#56255 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#56205 Reviewed-By: Moshe Atlow <[email protected]>
PR-URL: nodejs#56266 Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Signed-off-by: 吴小白 <[email protected]> PR-URL: nodejs#56271 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
Attributes are being highlighted as #f00 on a background of #f2f2f2. That's a color contrast of 3.98:1, failing to meet the 4.5:1 requirement of WCAG 2.1 AA. This changes the attribute color to #d00, which has a color contrast of 5.09:1 meeting the 4.5:1 requirement. PR-URL: nodejs#56272 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Claudio Wunder <[email protected]>
marco-ippolito
force-pushed
the
v20.x-staging
branch
from
January 23, 2025 12:58
595e71b
to
d55d368
Compare
joyeecheung
force-pushed
the
backport-cache-fixes
branch
from
January 23, 2025 14:50
c832e66
to
0398e4b
Compare
github-actions
bot
removed
the
request-ci
Add this label to start a Jenkins CI on a PR.
label
Jan 23, 2025
Original commit message: [import-attributes] Deprecate 'assert' for removal in 12.6 See https://groups.google.com/a/chromium.org/g/blink-dev/c/ZHvzLaJZRvo/m/FgNDBjrtBQAJ Bug: v8:10958 Change-Id: I4d21c9f7aad1024b198b4a1cdfb4792a011da464 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5055681 Reviewed-by: Rezvan Mahdavi Hezaveh <[email protected]> Auto-Submit: Shu-yu Guo <[email protected]> Commit-Queue: Shu-yu Guo <[email protected]> Cr-Commit-Position: refs/heads/main@{#92044} Refs: v8/v8@ae5a4db Co-authored-by: Antoine du Hamel <[email protected]> PR-URL: nodejs#55961 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
Original commit message: [import-attributes] Deprecate 'assert' for dynamic import as well Bug: v8:10958 Change-Id: I7847bdb5d2c79f057f4e1df99f8f5889788f09cb Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5249778 Commit-Queue: Shu-yu Guo <[email protected]> Reviewed-by: Leszek Swirski <[email protected]> Cr-Commit-Position: refs/heads/main@{#92123} Refs: v8/v8@26fd1df PR-URL: nodejs#55961 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
The two proposals reached stage 4 at the October 2024 meeting. PR-URL: nodejs#55333 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Backport-PR-URL: nodejs#55961
PR-URL: nodejs#55855 Refs: nodejs#55333 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Backport-PR-URL: nodejs#55961
PR-URL: nodejs#56706 Backport-PR-URL: nodejs#56721 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#56707 Backport-PR-URL: nodejs#56724 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
marco-ippolito
force-pushed
the
v20.x-staging
branch
from
January 23, 2025 15:37
416c658
to
cc7d1a7
Compare
joyeecheung
force-pushed
the
backport-cache-fixes
branch
from
January 23, 2025 16:08
0398e4b
to
c83cf72
Compare
Original commit message: Reland "[cache] Don't compare host defined options if the script was deserialized" This is a reland of commit b9cfb8b7cfdbf195c3baf87735865948dfa5907e Original change's description: > [cache] Don't compare host defined options if the script was deserialized > > We don't serialize host defined options (see > CodeSerializer::SerializeObjectImpl()). > > Change-Id: Icab9fe910a5ec93b5eacc4869aba75034ad8b447 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4805329 > Reviewed-by: Camillo Bruni <[email protected]> > Reviewed-by: Toon Verwaest <[email protected]> > Commit-Queue: Tao Pan <[email protected]> > Cr-Commit-Position: refs/heads/main@{#90698} Change-Id: I7ea5e9355056104ebd25b385aba63c1233d42260 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4998581 Reviewed-by: Camillo Bruni <[email protected]> Reviewed-by: Toon Verwaest <[email protected]> Commit-Queue: Tao Pan <[email protected]> Cr-Commit-Position: refs/heads/main@{#90711} Refs: v8/v8@7b677a5
Original commit message: [compiler] support isolate compilation cache in CompileFunction() Previously there was no isolate compilation cache support for scripts compiled Script::CompileFunction() with wrapped arguments. This patch adds support for that. Refs: nodejs#35375 Change-Id: Id1849961ecd1282eb2dac95829157d167a3aa9a1 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4962094 Reviewed-by: Camillo Bruni <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Cr-Commit-Position: refs/heads/main@{#91681} Refs: v8/v8@f4b3f6e
Original commit message: [compiler] reset script details in functions deserialized from code cache During the serialization of the code cache, V8 would wipe out the host-defined options, so after a script id deserialized from the code cache, the host-defined options need to be reset on the script using what's provided by the embedder when doing the deserializing compilation, otherwise the HostImportModuleDynamically callbacks can't get the data it needs to implement dynamic import(). Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Cr-Commit-Position: refs/heads/main@{#93323} Refs: v8/v8@cd10ad7 Co-authored-by: Joyee Cheung <[email protected]>
Original commit message: [snapshot] Check if a cached data has wrapped arguments Fixes that ScriptCompiler::CreateCodeCacheForFunction aborts on a deserialized shared function info from a cached data accepted with ScriptCompiler::CompileFunction. If the wrapped argument list does not match, the cached data should be rejected. Refs: nodejs#56366 Change-Id: I3f0376216791d866fac8eed1ce88dfa05e919b48 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6140933 Commit-Queue: Chengzhong Wu <[email protected]> Reviewed-by: Leszek Swirski <[email protected]> Cr-Commit-Position: refs/heads/main@{#97942} Refs: v8/v8@96ee9bb Co-authored-by: Joyee Cheung <[email protected]>
joyeecheung
force-pushed
the
backport-cache-fixes
branch
from
January 23, 2025 17:08
c83cf72
to
df1513e
Compare
marco-ippolito
force-pushed
the
v20.x-staging
branch
from
January 24, 2025 21:19
cc7d1a7
to
f78508c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This backports the following bug fixes in V8 that are related to compile cache.
https://chromium-review.googlesource.com/c/v8/v8/+/4998581
https://chromium-review.googlesource.com/c/v8/v8/+/4962094
https://chromium-review.googlesource.com/c/v8/v8/+/5401780
https://chromium-review.googlesource.com/c/v8/v8/+/6140933
The motivation of backporting these is that they would help backporting compile cache to v20.x, which would in turn help backporting require(esm) because the two are a bit intertwined in the module compilation/format detection routine, so there would be more conflicts if we backport
require(esm)
without backporting the compile cache.One of them fixes in-isolate compilation cache hit for
comepileFunction()
and another fixesimport()
when code cache is used, which may be meaningful in themselves for use cases like Jest (e.g. jestjs/jest#15461).Refs: #52697