-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
[v12.x] deps: V8: cherry-pick cb1c2b0fbfe7 #31939
[v12.x] deps: V8: cherry-pick cb1c2b0fbfe7 #31939
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
FWIW, this is a known failure (#32076) |
This comment has been minimized.
This comment has been minimized.
928f179
to
c7b94b6
Compare
The CI failures looked strange. Rebased to get any relevant change in. |
CI fails because |
There are a lot of failures like this:
Might be because of the rebase? I'm not sure |
Should be fixed by rebasing: #32282 (comment) |
Ah, @Flarna rebased after the CI. I'll start it again |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I mean, this is probably good to land, the last failing on Jenkins was infra-related, but let's try again just to be sure. @Flarna were you expecting this to be included in the next release? |
@mmarchini I was not targeting a specific release. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
a01fb4a
to
e5dc2fb
Compare
ecb1eb1
to
ce374fe
Compare
Rebased to get rid of merge conflicts. |
63a03d2
to
d577190
Compare
@Flarna apologies but this requires another rebase. I've gone ahead and done the work but was unable to push to your branch. https://github.com/MylesBorins/node/tree/backport-cb1c2b0 |
Original commit message: Remove noscript_shared_function_infos SharedFunctionInfos that do not belong to a script were tracked in noscript_shared_function_infos. However this was only used in object-stats. Remove this since it was actually leaking memory in some use cases. Bug: v8:9674 Change-Id: I9482f7e5dedf975666a70684b3d2ea04c9a23518 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1798423 Reviewed-by: Ulan Degenbaev <[email protected]> Reviewed-by: Michael Stanton <[email protected]> Commit-Queue: Dominik Inführ <[email protected]> Cr-Commit-Position: refs/heads/master@{#63685} Refs: v8/v8@cb1c2b0
@MylesBorins Rebased |
Looks like this might introduce new failures on both regular CI and V8-CI. Running CI against staging to confirm 12.x-staging CI: https://ci.nodejs.org/job/node-test-commit/37126/ |
@MylesBorins those are known, V8 CI on v12 is consistently failing, see comment above: #31939 (comment). (I think I know how to fix it, will try later) |
Original commit message: Remove noscript_shared_function_infos SharedFunctionInfos that do not belong to a script were tracked in noscript_shared_function_infos. However this was only used in object-stats. Remove this since it was actually leaking memory in some use cases. Bug: v8:9674 Change-Id: I9482f7e5dedf975666a70684b3d2ea04c9a23518 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1798423 Reviewed-by: Ulan Degenbaev <[email protected]> Reviewed-by: Michael Stanton <[email protected]> Commit-Queue: Dominik Inführ <[email protected]> Cr-Commit-Position: refs/heads/master@{#63685} PR-URL: #31939 Refs: v8/v8@cb1c2b0 Refs: #31914 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Matheus Marchini <[email protected]>
landed in a83fc49 |
Original commit message:
Refs: v8/v8@cb1c2b0
Refs: #31914
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes