Skip to content
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

deps: update V8 to 6.5 #18453

Closed
wants to merge 11 commits into from
Closed

deps: update V8 to 6.5 #18453

wants to merge 11 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Jan 30, 2018

ETA: Mar 6th, 2018

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

V8

@targos targos added the blocked PRs that are blocked by other issues or PRs. label Jan 30, 2018
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. labels Jan 30, 2018
@targos targos removed the build Issues and PRs related to build files or the CI. label Jan 30, 2018
@targos
Copy link
Member Author

targos commented Jan 30, 2018

@targos
Copy link
Member Author

targos commented Jan 31, 2018

@nodejs/platform-ppc @nodejs/platform-s390
PTAL at the V8 test runs. There are multiple reproducible failures.

@mhdawson
Copy link
Member

@jBarz @john-yan can you please take a look and comment here.

@MylesBorins
Copy link
Contributor

/cc @nodejs/v8

@hashseed
Copy link
Member

hashseed commented Feb 1, 2018

You might want to cherry-pick this patch related to code caching after this lands.

@jBarz
Copy link
Contributor

jBarz commented Feb 2, 2018

fyi, we are investigating the ppc/s390 failures

@targos
Copy link
Member Author

targos commented Feb 12, 2018

Updated to 6.5.254.19
Opened a merge request for the debug build fix: https://bugs.chromium.org/p/v8/issues/detail?id=7443

@targos
Copy link
Member Author

targos commented Feb 12, 2018

@targos
Copy link
Member Author

targos commented Feb 21, 2018

Ping @jBarz any updates on ppc/s390?

@john-yan
Copy link

Hello @targos ,
for bigint/turbo.js, it's a endian issue on both ppc/s390 which has been fixed by https://chromium-review.googlesource.com/c/v8/v8/+/914822
for wasm/shared-memory, wasm/compiled-module-serialization and regress/wasm/regress-808980, we skip them on master branch becaue there are issues related to relocation and constant pool. here: https://chromium-review.googlesource.com/c/v8/v8/+/898040 and https://chromium-review.googlesource.com/906950. But it doesn't seem to be able to get through yet on 6.5 branch. Do you think applying a floating patch on top of it can be a solution?
for the inspector-test, the fix is in https://chromium-review.googlesource.com/c/v8/v8/+/881883. I will contact the owner and back port it to 6.5 asap.

@hashseed
Copy link
Member

hashseed commented Feb 21, 2018

Related, but not necessary, is this change that you might want to cherry-pick.

Background is that we are changing the code caching API to be more powerful. Until V8 6.5, you had to request for cache data to be produced as part of script compilation. Now we have a new API with V8::ScriptCompiler::CreateCodeCache that allows you to generate cache data at any time. What that buys you is that you could generate cache data after you have executed the compiled script once, so that the cache data can contain code for functions that were compiled during execution. The previous way to request cache data is going to go away in V8 6.6.

While the commit I mentioned updates Node.js so that the API exposed by the vm module does not change, I could imagine to extend it to take advantage of this more powerful feature.

@targos
Copy link
Member Author

targos commented Feb 22, 2018

@john-yan thanks. I applied floated the changes for now. Let's see if everything is fixed.

@hashseed thanks. I cherry-picked that commit.

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

@targos
Copy link
Member Author

targos commented Feb 22, 2018

@john-yan there are still two test failures:

  • v8tests.cctest/test-run-wasm-simd/RunWasm_SimdLoadStoreLoad_compiled
  • v8tests.unittests/TyperTest.Monotonicity_Operation_NumberToString

@john-yan
Copy link

Hello @targos , ppcbe linux is not supported anymore. Just notice the unittest error, I will investigate soon.

@ofrobots
Copy link
Contributor

@john-yan can you elaborate? If IBM is not supporting ppcbe-linux platform for Node.js anymore, we would want to make sure we (Node.js) announce dropping of support before Node 10.0 goes out the door. /cc @nodejs/build.

@richardlau
Copy link
Member

@ofrobots ppcbe-linux support was dropped for Node.js 8: #12309

@john-yan
Copy link

@targos For the typer failure, it could also reproduce on other platforms like x64 and arm. Therefore, it may not be a s390x issue. I opened an bug report from here: https://bugs.chromium.org/p/v8/issues/detail?id=7493

@john-yan
Copy link

@targos Updates on the typer error is here: https://bugs.chromium.org/p/v8/issues/detail?id=7493#c5, this patch https://chromium-review.googlesource.com/c/v8/v8/+/880982 will fix the issue. But since it's not critical, they won't probably checkpick to 6.5.

Original commit message:

    [compiler] Fix typing of NumberToString operator.

    It must be monotone.

    R=bmeurer@chromium.org

    Bug: v8:7354
    Change-Id: I08dcd3333518029eef08c074c2b91b5c20ad699e
    Reviewed-on: https://chromium-review.googlesource.com/880982
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Commit-Queue: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#50801}

Refs: v8/v8@8bfbe25
Original commit message:

    Use wider types for max_old_space_size and co.

    Make --max_old_space_size and friends work with values >= 2**31.
    Such values did not work reliably (or sometimes not all) due to
    signed integer overflow in size computations, which is UB.

    Fixes nodejs#18786.

    Bug: chromium:814138
    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182
    Reviewed-on: https://chromium-review.googlesource.com/927063
    Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#51433}

Refs: v8/v8@46c4979
@targos
Copy link
Member Author

targos commented Mar 7, 2018

targos added a commit that referenced this pull request Mar 7, 2018
PR-URL: #18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos added a commit that referenced this pull request Mar 7, 2018
PR-URL: #18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos added a commit that referenced this pull request Mar 7, 2018
Major V8 updates are usually API/ABI incompatible with previous
versions. This commit adapts NODE_MODULE_VERSION for V8 6.5.

Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md

PR-URL: #18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos pushed a commit that referenced this pull request Mar 7, 2018
This commit updates the following postmortem metadata constant:

- v8dbg_bit_field3_dictionary_map_shift
  - This is now v8dbg_bit_field3_is_dictionary_map_shift as of
    v8/v8@7a159da

Refs: nodejs/node-v8#34

Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

PR-URL: #18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos added a commit that referenced this pull request Mar 7, 2018
PR-URL: #18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos pushed a commit that referenced this pull request Mar 7, 2018
Earlier we used to produce code cache along with compile. Now V8 has
added an API to request code cache. Support for producing code cache
along with compile will be removed soon.

PR-URL: #18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos added a commit that referenced this pull request Mar 7, 2018
Original commit message:

    [inspector] Make test byte order independent

    Change-Id: If0fdc76170ad29b4d3dadddbb32bc87c307c04af
    Reviewed-on: https://chromium-review.googlesource.com/881883
    Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
    Commit-Queue: Eugene Ostroukhov <eostroukhov@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#50817}

Refs: v8/v8@04a06c9

PR-URL: #18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos added a commit that referenced this pull request Mar 7, 2018
Original commit message:

    [compiler] Fix typing of NumberToString operator.

    It must be monotone.

    R=bmeurer@chromium.org

    Bug: v8:7354
    Change-Id: I08dcd3333518029eef08c074c2b91b5c20ad699e
    Reviewed-on: https://chromium-review.googlesource.com/880982
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Commit-Queue: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#50801}

Refs: v8/v8@8bfbe25

PR-URL: #18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos added a commit that referenced this pull request Mar 7, 2018
PR-URL: #18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
targos added a commit that referenced this pull request Mar 7, 2018
Original commit message:

    Use wider types for max_old_space_size and co.

    Make --max_old_space_size and friends work with values >= 2**31.
    Such values did not work reliably (or sometimes not all) due to
    signed integer overflow in size computations, which is UB.

    Fixes #18786.

    Bug: chromium:814138
    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182
    Reviewed-on: https://chromium-review.googlesource.com/927063
    Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#51433}

Refs: v8/v8@46c4979

PR-URL: #18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@targos
Copy link
Member Author

targos commented Mar 7, 2018

Landed in 4e86f9b...9759573

@targos targos closed this Mar 7, 2018
@targos targos deleted the v8-6.5 branch April 19, 2018 13:38
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
PR-URL: nodejs#18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
PR-URL: nodejs#18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Major V8 updates are usually API/ABI incompatible with previous
versions. This commit adapts NODE_MODULE_VERSION for V8 6.5.

Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md

PR-URL: nodejs#18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
This commit updates the following postmortem metadata constant:

- v8dbg_bit_field3_dictionary_map_shift
  - This is now v8dbg_bit_field3_is_dictionary_map_shift as of
    v8/v8@7a159da

Refs: nodejs/node-v8#34

Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

PR-URL: nodejs#18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
PR-URL: nodejs#18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Earlier we used to produce code cache along with compile. Now V8 has
added an API to request code cache. Support for producing code cache
along with compile will be removed soon.

PR-URL: nodejs#18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Original commit message:

    [inspector] Make test byte order independent

    Change-Id: If0fdc76170ad29b4d3dadddbb32bc87c307c04af
    Reviewed-on: https://chromium-review.googlesource.com/881883
    Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
    Commit-Queue: Eugene Ostroukhov <eostroukhov@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#50817}

Refs: v8/v8@04a06c9

PR-URL: nodejs#18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Original commit message:

    [compiler] Fix typing of NumberToString operator.

    It must be monotone.

    R=bmeurer@chromium.org

    Bug: v8:7354
    Change-Id: I08dcd3333518029eef08c074c2b91b5c20ad699e
    Reviewed-on: https://chromium-review.googlesource.com/880982
    Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
    Commit-Queue: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#50801}

Refs: v8/v8@8bfbe25

PR-URL: nodejs#18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
PR-URL: nodejs#18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Original commit message:

    Use wider types for max_old_space_size and co.

    Make --max_old_space_size and friends work with values >= 2**31.
    Such values did not work reliably (or sometimes not all) due to
    signed integer overflow in size computations, which is UB.

    Fixes nodejs#18786.

    Bug: chromium:814138
    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182
    Reviewed-on: https://chromium-review.googlesource.com/927063
    Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#51433}

Refs: v8/v8@46c4979

PR-URL: nodejs#18453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major PRs that contain breaking changes and should be released in the next major version. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet