deps: update V8 to 9.1 #38273

targos · 2021-04-17T13:44:41Z

No description provided.

nodejs-github-bot · 2021-04-17T13:46:08Z

CI: https://ci.nodejs.org/job/node-test-pull-request/37536/
V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/3934/

targos · 2021-04-18T07:58:27Z

V8 tests don't pass on PPCLE: https://ci.nodejs.org/job/node-test-commit-v8-linux/3934/

/cc @nodejs/platform-ppc

targos · 2021-04-18T07:59:15Z

test/v8-updates/test-linux-perf fails all the time now. See nodejs/node-v8#198

targos · 2021-04-18T08:00:32Z

There's a problem with --abort-on-uncaught-exception on Windows. See nodejs/node-v8#197

richardlau · 2021-04-18T13:13:10Z

V8 tests don't pass on PPCLE: https://ci.nodejs.org/job/node-test-commit-v8-linux/3934/

/cc @nodejs/platform-ppc

Maybe https://chromium-review.googlesource.com/c/v8/v8/+/2693056? cc @miladfarca

miladfarca · 2021-04-18T16:02:34Z

@richardlau that's right, above CL needs to be cherrypicked. V8 has started to get rid of its Simd lowering pipeline, meaning platforms without the support will not be able to run the tests. Are these machines on power 8 or 9?

richardlau · 2021-04-18T18:15:59Z

Are these machines on power 8 or 9?

@miladfarca Looks like Power 8:

$ cat /proc/cpuinfo 
processor	: 0
cpu		: POWER8 (architected), altivec supported
clock		: 3425.000000MHz
revision	: 2.1 (pvr 004b 0201)

processor	: 1
cpu		: POWER8 (architected), altivec supported
clock		: 3425.000000MHz
revision	: 2.1 (pvr 004b 0201)

processor	: 2
cpu		: POWER8 (architected), altivec supported
clock		: 3425.000000MHz
revision	: 2.1 (pvr 004b 0201)

processor	: 3
cpu		: POWER8 (architected), altivec supported
clock		: 3425.000000MHz
revision	: 2.1 (pvr 004b 0201)

timebase	: 512000000
platform	: pSeries
model		: IBM pSeries (emulated by qemu)
machine		: CHRP IBM pSeries (emulated by qemu)
$

targos · 2021-04-18T18:43:51Z

above CL needs to be cherrypicked.

Is it going to happen upstream or do I need to cherry-pick it here?

miladfarca · 2021-04-18T20:25:35Z

@targos its better to cherry pick it here as we might need to make changes to this upstream.

miladfarca · 2021-04-18T20:26:15Z

@richardlau thanks for confirming.

targos · 2021-04-19T08:14:41Z

Ok, I backported the commit. I saw that the skip was reverted on V8 recently. Does it mean that we're going to have to keep the backport in node forever?

miladfarca · 2021-04-19T12:59:52Z

@targos We have recently turned this feature on for Power 9 machines (and up) and reverted the mentioned CL as our internal test bots are mostly p9. We will either need to turn on the feature for p8 as well or skip the tests on p8 and below.

targos · 2021-04-24T08:43:23Z

The behavior change for --abort-on-uncaught-exception on Windows was probably introduced by v8/v8@26d85ac.
Unfortunately, it references a private Google Doc for detailed explanation.

In our tests, we verify that the exit code is 0xC0000005 (access violation), but now it is 0x80000003 (exception breakpoint).
Can we just change our tests? /cc @nodejs/platform-windows

node/test/common/index.js

Lines 482 to 488 in 720fdf2

    
           // On Windows, 'aborts' are of 2 types, depending on the context: 
        
           // (i) Forced access violation, if --abort-on-uncaught-exception is on 
        
           // which corresponds to exit code 3221225477 (0xC0000005) 
        
           // (ii) Otherwise, _exit(134) which is called in place of abort() due to 
        
           // raising SIGABRT exiting with ambiguous exit code '3' by default 
        
           if (isWindows) 
        
             expectedExitCodes = [0xC0000005, 134];

targos · 2021-04-24T09:36:39Z

About the Linux perf issue, INTERPRETED_FUNCTION_TAG was removed in v8/v8@9a31804

Here are the logs of the test with v16.0.0 vs this PR:

/cc @nodejs/diagnostics

miladfarca · 2021-04-30T17:41:17Z

@targos is it possible to cherry pick this CL as well: https://chromium-review.googlesource.com/c/v8/v8/+/2855041
It adds P10 to the list of supported processors on ppc.
I will also need to also backport it to LTS branches (12, 14 and 16).

miladfarca · 2021-05-03T13:09:03Z

@targos I have crated these 3 PRs to backport above request to node 16, 14 and 12:
#38489
#38508
#38509

Please do let me know if I need to backport the same CL to V8 stable and beta branches, or if cherry picking it here would be enough?

targos · 2021-05-03T16:50:29Z

@miladfarca backporting to V8 beta would be better but if it doesn't happen, I'll add it here.

miladfarca · 2021-05-03T16:59:08Z

@targos no issues I will backport it to V8 beta.

targos · 2021-05-17T17:31:12Z

@miladfarca did it happen?

miladfarca · 2021-05-17T17:33:08Z

@targos yes it was backported to V8 9.1 here:
https://chromium-review.googlesource.com/c/v8/v8/+/2876988

Refs: v8/v8@40e499c PR-URL: #38273 Backport-PR-URL: #38991 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Mary Marchini <oss@mmarchini.me>

Refs: v8/v8@9a31804 PR-URL: #38273 Backport-PR-URL: #38991 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Mary Marchini <oss@mmarchini.me>

V8 9.1 changed the way it aborts on uncaught exceptions on Windows. It now uses the code 0x80000003 (exception breakpoint) instead of 0xC0000005 (access violation). Refs: v8/v8@26d85ac PR-URL: #38273 Backport-PR-URL: #38991 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Mary Marchini <oss@mmarchini.me>

Notable changes: * deps: * update V8 to 9.1.269.36 (Michaël Zasso) #38273 * dns: * allow `--dns-result-order` to change default dns verbatim (Ouyang Yadong) #38099

Notable changes: * deps: * upgrade npm to 7.18.1 (npm-robot) #39065 * update V8 to 9.1.269.36 (Michaël Zasso) #38273 * dns: * allow `--dns-result-order` to change default dns verbatim (Ouyang Yadong) #38099

Notable changes: * async_hooks: * stabilize part of AsyncLocalStorage (Vladimir de Turckheim) #37675 * deps: * upgrade npm to 7.18.1 (npm-robot) #39065 * update V8 to 9.1.269.36 (Michaël Zasso) #38273 * dns: * allow `--dns-result-order` to change default dns verbatim (Ouyang Yadong) #38099

PR-URL: #39031 Notable changes: * async_hooks: * stabilize part of AsyncLocalStorage (Vladimir de Turckheim) #37675 * deps: * upgrade npm to 7.18.1 (npm-robot) #39065 * update V8 to 9.1.269.36 (Michaël Zasso) #38273 * dns: * allow `--dns-result-order` to change default dns verbatim (Ouyang Yadong) #38099

Notable changes: * async_hooks: * stabilize part of AsyncLocalStorage (Vladimir de Turckheim) #37675 * deps: * upgrade npm to 7.18.1 (npm-robot) #39065 * update V8 to 9.1.269.36 (Michaël Zasso) #38273 * dns: * allow `--dns-result-order` to change default dns verbatim (Ouyang Yadong) #38099 PR-URL: #39031

nodejs/node#38273

Since the said issue is resolved and shipped in 16.4, Suggesting users to downgrade is a bad idea, instead they should be recommended to upgrade to the latest release to resolve this issue. Reference: nodejs/node#38273

PR #38000 added indirection so that we could stop finalization in cases where it had been scheduled in a second pass callback but we were doing it in advance in environment teardown. Unforunately we missed that the code which tries to clear the second pass parameter checked if the pointer to the parameter (_secondPassParameter) was nullptr and that when the second pass callback was scheduled we set _secondPassParameter to nullptr in order to avoid it being deleted outside of the second pass callback. The net result was that we would not clear the _secondPassParameter contents and failed to avoid the Finalization in the second pass callback. This PR adds an additional boolean for deciding if the secondPassParameter should be deleted outside of the second pass callback instead of setting secondPassParameter to nullptr thus avoiding the conflict between the 2 ways it was being used. See the discussion starting at: #38273 (comment) for how this was discovered on OSX while trying to upgrade to a new V8 version. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #38899 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Refs: nodejs#38273 (comment) PR-URL: nodejs#38811 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Beth Griggs <bgriggs@redhat.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR #38000 added indirection so that we could stop finalization in cases where it had been scheduled in a second pass callback but we were doing it in advance in environment teardown. Unforunately we missed that the code which tries to clear the second pass parameter checked if the pointer to the parameter (_secondPassParameter) was nullptr and that when the second pass callback was scheduled we set _secondPassParameter to nullptr in order to avoid it being deleted outside of the second pass callback. The net result was that we would not clear the _secondPassParameter contents and failed to avoid the Finalization in the second pass callback. This PR adds an additional boolean for deciding if the secondPassParameter should be deleted outside of the second pass callback instead of setting secondPassParameter to nullptr thus avoiding the conflict between the 2 ways it was being used. See the discussion starting at: #38273 (comment) for how this was discovered on OSX while trying to upgrade to a new V8 version. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #38899 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Refs: nodejs#38273 (comment) PR-URL: nodejs#38811 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Beth Griggs <bgriggs@redhat.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Rich Trott <rtrott@gmail.com>

Refs: #38273 (comment) PR-URL: #38811 Backport-PR-URL: #39446 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Beth Griggs <bgriggs@redhat.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR nodejs#38000 added indirection so that we could stop finalization in cases where it had been scheduled in a second pass callback but we were doing it in advance in environment teardown. Unforunately we missed that the code which tries to clear the second pass parameter checked if the pointer to the parameter (_secondPassParameter) was nullptr and that when the second pass callback was scheduled we set _secondPassParameter to nullptr in order to avoid it being deleted outside of the second pass callback. The net result was that we would not clear the _secondPassParameter contents and failed to avoid the Finalization in the second pass callback. This PR adds an additional boolean for deciding if the secondPassParameter should be deleted outside of the second pass callback instead of setting secondPassParameter to nullptr thus avoiding the conflict between the 2 ways it was being used. See the discussion starting at: nodejs#38273 (comment) for how this was discovered on OSX while trying to upgrade to a new V8 version. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: nodejs#38899 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Refs: nodejs#38273 (comment) PR-URL: nodejs#38811 Backport-PR-URL: nodejs#39446 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Beth Griggs <bgriggs@redhat.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Rich Trott <rtrott@gmail.com>

3228689373 · 2021-12-06T06:54:18Z

I meet this too:

^M^M[00:00|% 0|+ 0|- 0]: release test-linux-perf^M ^M=== release test-linux-perf ===
Path: v8-updates/test-linux-perf
node:assert:400
throw err;
^

AssertionError [ERR_ASSERTION]: Couldn't find interpreted functionOne()
Perf output:

node 9465 690874.279801: 10101010 cpu-clock:
7fff99e77d07 __do_page_fault ([kernel.kallsyms])
7fff99e77fee do_page_fault ([kernel.kallsyms])
7fff9a803635 page_fault ([kernel.kallsyms])
eb21b5 v8::internal::Deserializerv8::internal::Isolate::ReadObject (/mnt/sdb/NVNODE/node4/node-master/out/Release/node)

node-master# ./node
Welcome to Node.js v18.0.0-pre.
Type ".help" for more information.

PR nodejs#38000 added indirection so that we could stop finalization in cases where it had been scheduled in a second pass callback but we were doing it in advance in environment teardown. Unforunately we missed that the code which tries to clear the second pass parameter checked if the pointer to the parameter (_secondPassParameter) was nullptr and that when the second pass callback was scheduled we set _secondPassParameter to nullptr in order to avoid it being deleted outside of the second pass callback. The net result was that we would not clear the _secondPassParameter contents and failed to avoid the Finalization in the second pass callback. This PR adds an additional boolean for deciding if the secondPassParameter should be deleted outside of the second pass callback instead of setting secondPassParameter to nullptr thus avoiding the conflict between the 2 ways it was being used. See the discussion starting at: nodejs#38273 (comment) for how this was discovered on OSX while trying to upgrade to a new V8 version. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: nodejs#38899 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

PR #38000 added indirection so that we could stop finalization in cases where it had been scheduled in a second pass callback but we were doing it in advance in environment teardown. Unforunately we missed that the code which tries to clear the second pass parameter checked if the pointer to the parameter (_secondPassParameter) was nullptr and that when the second pass callback was scheduled we set _secondPassParameter to nullptr in order to avoid it being deleted outside of the second pass callback. The net result was that we would not clear the _secondPassParameter contents and failed to avoid the Finalization in the second pass callback. This PR adds an additional boolean for deciding if the secondPassParameter should be deleted outside of the second pass callback instead of setting secondPassParameter to nullptr thus avoiding the conflict between the 2 ways it was being used. See the discussion starting at: #38273 (comment) for how this was discovered on OSX while trying to upgrade to a new V8 version. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #38899 Backport-PR-URL: #42512 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

targos added the semver-major PRs that contain breaking changes and should be released in the next major version. label Apr 17, 2021

nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Apr 17, 2021

targos added the wip Issues and PRs that are still a work in progress. label Apr 17, 2021

targos force-pushed the v8-91 branch from 6cc41b1 to 78ca4bf Compare April 19, 2021 08:13

targos mentioned this pull request Apr 24, 2021

Abort on uncaught exception broken on Windows? nodejs/node-v8#197

Closed

targos mentioned this pull request Apr 24, 2021

test-linux-perf failing consistently nodejs/node-v8#198

Closed

ronag mentioned this pull request Apr 24, 2021

perf: enable wasm simd nodejs/undici#735

Merged

miladfarca mentioned this pull request May 3, 2021

[v16.x] deps: V8: Add Power10 to the supported list and enable related features #38489

Closed

danielleadams added a commit that referenced this pull request Jun 17, 2021

2021-06-17, Version 16.4.0 (Current)

6f58f67

Notable changes: * deps: * update V8 to 9.1.269.36 (Michaël Zasso) #38273 * dns: * allow `--dns-result-order` to change default dns verbatim (Ouyang Yadong) #38099

codebytere added a commit to electron/electron that referenced this pull request Jun 24, 2021

deps: update V8 to 9.1

9081deb

nodejs/node#38273

kleisauke mentioned this pull request Jun 24, 2021

Shipping progress WebAssembly/simd#480

Open

codebytere added a commit to electron/electron that referenced this pull request Jun 24, 2021

deps: update V8 to 9.1

a44775b

nodejs/node#38273

Ashesh3 mentioned this pull request Jun 27, 2021

Node 16.x breakpoint warning resolved in 16.4 microsoft/vscode-js-debug#1044

Merged

pluris mentioned this pull request Jul 25, 2023

test: modify unmatched message in assert(v8-updates) #48911

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deps: update V8 to 9.1 #38273

deps: update V8 to 9.1 #38273

targos commented Apr 17, 2021

nodejs-github-bot commented Apr 17, 2021 •

edited by targos

Loading

targos commented Apr 18, 2021

targos commented Apr 18, 2021

targos commented Apr 18, 2021

richardlau commented Apr 18, 2021 •

edited

Loading

miladfarca commented Apr 18, 2021

richardlau commented Apr 18, 2021

targos commented Apr 18, 2021

miladfarca commented Apr 18, 2021

miladfarca commented Apr 18, 2021

targos commented Apr 19, 2021

miladfarca commented Apr 19, 2021

targos commented Apr 24, 2021

targos commented Apr 24, 2021

miladfarca commented Apr 30, 2021

miladfarca commented May 3, 2021

targos commented May 3, 2021

miladfarca commented May 3, 2021 •

edited

Loading

targos commented May 17, 2021

miladfarca commented May 17, 2021

3228689373 commented Dec 6, 2021

deps: update V8 to 9.1 #38273

deps: update V8 to 9.1 #38273

Conversation

targos commented Apr 17, 2021

nodejs-github-bot commented Apr 17, 2021 • edited by targos Loading

targos commented Apr 18, 2021

targos commented Apr 18, 2021

targos commented Apr 18, 2021

richardlau commented Apr 18, 2021 • edited Loading

miladfarca commented Apr 18, 2021

richardlau commented Apr 18, 2021

targos commented Apr 18, 2021

miladfarca commented Apr 18, 2021

miladfarca commented Apr 18, 2021

targos commented Apr 19, 2021

miladfarca commented Apr 19, 2021

targos commented Apr 24, 2021

targos commented Apr 24, 2021

miladfarca commented Apr 30, 2021

miladfarca commented May 3, 2021

targos commented May 3, 2021

miladfarca commented May 3, 2021 • edited Loading

targos commented May 17, 2021

miladfarca commented May 17, 2021

3228689373 commented Dec 6, 2021

nodejs-github-bot commented Apr 17, 2021 •

edited by targos

Loading

richardlau commented Apr 18, 2021 •

edited

Loading

miladfarca commented May 3, 2021 •

edited

Loading