★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/14004Nouvelle recherche | Portail wanayoo
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

Jump to bottom

deps: update V8 to 6.0 #14004

Closed
wants to merge 8 commits into from
Closed

deps: update V8 to 6.0 #14004

wants to merge 8 commits into from
+61,304 −40,860

Conversation

targos
Copy link
Member

@targos targos commented Jun 30, 2017 •
edited by refack

As discussed in nodejs/CTC#146, I am opening this PR early so we can have test builds of Node with V8 6.0 as soon as possible.
The update is ready but blocked on five issues:

/cc @nodejs/v8 @nodejs/ctc

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

V8

@targos targos mentioned this pull request Jun 30, 2017
Closed
@targos
Copy link
Member Author

@targos targos commented Jun 30, 2017

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

@targos targos mentioned this pull request Jun 30, 2017
Closed
@targos
Copy link
Member Author

@targos targos commented Jun 30, 2017

I forgot there are two other issues (initially reported here: nodejs/node-v8#7):

And finally a V8 cctest fails: https://ci.nodejs.org/job/node-test-commit-v8-linux/763/

@misterdjules
Copy link
Contributor

@misterdjules misterdjules commented Jun 30, 2017

@targos When do you need those SmartOS build errors fixed? In other words, is it urgent?

@joaocgreis
Copy link
Member

@joaocgreis joaocgreis commented Jun 30, 2017

@targos https://chromium-review.googlesource.com/c/556599/ landed upstream, it would be better to replace my floating patch (deps,win: increase msvs_shard in gyp for V8) with a backport of that commit.

@targos
Copy link
Member Author

@targos targos commented Jul 3, 2017

@joaocgreis Thank you. I cherry-picked that commit.

@targos
Copy link
Member Author

@targos targos commented Jul 3, 2017

@misterdjules No, this is not urgent, and we have other issues to fix in the meantime

@bmeurer
Copy link
Member

@bmeurer bmeurer commented Jul 6, 2017

cc @natorion

@natorion
Copy link

@natorion natorion commented Jul 11, 2017

Some comments/questions:

Node crashes when running jest. See: #13804, https://bugs.chromium.org/p/v8/issues/detail?id=6490 and https://github.com/targos/jest-crash.
==> scheduled to be merged upstream to 6.0

async-hooks/test-zlib.zlib-binding.deflate can crash (seems to happen only with the 32-bit build): https://ci.nodejs.org/job/node-test-commit-linux/10874/
==> Is there already a V8 issue? That sounds like something that might be fixed upstream. Sadly, I can't access ci.nodejs.org

V8 cctest failure: https://ci.nodejs.org/job/node-test-commit-v8-linux/763/
==> Same as above, it is hard to judge what needs to be done.

Compilation errors on SmartOS and FreeBSD =>
Are you going to float those patches? Are you expecting that they are going to be upstreamed? Please keep in mind that we will promote 6.0 to V8 stable next week. That means, that merges will have a much higher burden to overcome. Depending on the actual patch it might not be an issue though because of SmartOS and FreeBSD.

@Trott
Copy link
Member

@Trott Trott commented Jul 11, 2017

Sadly, I can't access ci.nodejs.org

@natorion It should be available again in a few hours, if all goes well.

Access is extremely restricted right now because it's being used for the scheduled security release of Node.js. Any changes to that schedule are likely to be posted to #14153. If necessary to know more, @MylesBorins can fill you in.

@gibfahn
Copy link
Member

@gibfahn gibfahn commented Jul 11, 2017

@natorion It should be available again in a few hours, if all goes well.

It's back btw.

@targos
Copy link
Member Author

@targos targos commented Jul 11, 2017

New CI run: https://ci.nodejs.org/job/node-test-pull-request/9075/

@natorion
Copy link

@natorion natorion commented Jul 12, 2017

Thanks, seems like https://ci.nodejs.org/job/node-test-pull-request/9075/ was aborted and the other links produce 404s.

@MylesBorins
Copy link
Member

@MylesBorins MylesBorins commented Jul 12, 2017

@natorion it would appear that the test runner was hanging on centos 5

https://ci.nodejs.org/job/node-test-commit-linux/11105/nodes=centos5-64/console

@targos
Copy link
Member Author

@targos targos commented Jul 12, 2017

I updated the branch to V8 6.0.286.34 to include the fix for https://bugs.chromium.org/p/v8/issues/detail?id=6490.

New CI: https://ci.nodejs.org/job/node-test-commit/11071/
V8 tests: https://ci.nodejs.org/job/node-test-commit-v8-linux/776/

@MylesBorins
Copy link
Member

@MylesBorins MylesBorins commented Jul 12, 2017

@targos @addaleax any interest in floating the bsd / smart os packages upstream?

@addaleax
Copy link
Member

@addaleax addaleax commented Jul 12, 2017

Sorry, I didn’t quite follow here – which are the patches you are referring to?

@MylesBorins
Copy link
Member

@MylesBorins MylesBorins commented Jul 12, 2017

@addaleax patches that fix the compilation errors, Im not sure we have them yet...

was trying to summarize @natorion's questions in #14004 (comment)

@targos
Copy link
Member Author

@targos targos commented Jul 12, 2017

We don't have those patches yet and I don't know if anyone is working on it.

@addaleax
Copy link
Member

@addaleax addaleax commented Jul 12, 2017

async-hooks/test-zlib.zlib-binding.deflate can crash (seems to happen only with the 32-bit build): https://ci.nodejs.org/job/node-test-commit-linux/10874/

Do you have a CI log that contains that failure? Could it be the same as #14161?

@targos
Copy link
Member Author

@targos targos commented Jul 12, 2017

Do you have a CI log that contains that failure?

No, the logs have been deleted. It does not seem to be reproducible anymore. I'll mark this point as done for now.

@MylesBorins MylesBorins closed this Aug 1, 2017
MylesBorins added a commit that referenced this issue Aug 1, 2017
@MylesBorins
deps: update V8 to 6.0.286.52
0a66b22 
PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins added a commit that referenced this issue Aug 1, 2017
@MylesBorins
src: update NODE_MODULE_VERSION to 57
db476fc 
Major V8 updates are usually API/ABI incompatible with previous
versions. This commit adapts NODE_MODULE_VERSION for V8 6.0.

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

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins added a commit that referenced this issue Aug 1, 2017
@mhdawson @MylesBorins
deps: limit regress/regress-crbug-514081 v8 test
ebd13c3 
regress/regress-crbug-514081 allocates a 2G block of memory
and if there  are multiple variants running at the
same time this can lead to crashes, OOM kills or
the OS failing to allocate memory.  This patch
limits us to running a single variant of the test

Fixes: #6340
Refs: #6678

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins added a commit that referenced this issue Aug 1, 2017
@bnoordhuis @MylesBorins
v8: fix stack overflow in recursive method
28aa7d0 
HGlobalValueNumberingPhase::CollectSideEffectsOnPathsToDominatedBlock()
used to self-recurse before this commit, causing stack overflows on
systems with small stack sizes.  Make it non-recursive by storing
intermediate results in a heap-allocated list.

Fixes: #11991
Refs: #12460

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins added a commit that referenced this issue Aug 1, 2017
@bzoz @MylesBorins
deps: fix addons compilation with VS2013
ed30842 
VS2013 does not support defaulting move constructor and assignment
operator. This adds explicit definitions of those methods for two
classes.
This fix is required because we still support building addons with
VS2013 and the incompatibility is in v8.h.

Fixes: nodejs/node-v8#4
Refs: #13263

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins added a commit that referenced this issue Aug 1, 2017
@targos @MylesBorins
deps: backport c0f1ff2 from upstream V8
5651a6f 
Original commit message:

    Fix GCC 7 build errors

    BUG=chromium:691681
    R=franzih@chromium.org

    Change-Id: Id7e5698487f16dc217a804f6d3f24da7213c72b9
    Reviewed-on: https://chromium-review.googlesource.com/530227
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#46045}

Refs: #13517
Fixes: #10388
Refs: #12392

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins added a commit that referenced this issue Aug 1, 2017
@targos @MylesBorins
src: fix new V8 compiler warnings
a3c5ccd 
PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins added a commit that referenced this issue Aug 1, 2017
@hashseed @MylesBorins
deps: backport rehash strings after deserialization
98ddab4 
Original commit messages:
v8/v8@a2ab135
  [snapshot] Rehash strings after deserialization.

  See https://goo.gl/6aN8xA

  Bug: v8:6593
  Change-Id: Ic8b0b57195d01d41591397d5d45de3f0f3ebc3d9
  Reviewed-on: https://chromium-review.googlesource.com/574527
  Reviewed-by: Camillo Bruni <cbruni@chromium.org>
  Reviewed-by: Jakob Gruber <jgruber@chromium.org>
  Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
  Commit-Queue: Yang Guo <yangguo@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#46732}

v8/v8@182caaf
  Do not track transitions for built-in objects.

  Objects created during bootstrapping do not need
  a transition tree except for elements kind transitions.

  Bug: v8:6596
  Change-Id: I237b8b2792f201336e1c9731c815095dd06bc182
  Reviewed-on: https://chromium-review.googlesource.com/571750
  Reviewed-by: Igor Sheludko <ishell@chromium.org>
  Commit-Queue: Yang Guo <yangguo@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#46693}

Fixes: #14171
Refs: #14345

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Aug 1, 2017
Closed
@MylesBorins MylesBorins mentioned this pull request Aug 1, 2017
Closed
addaleax added a commit that referenced this issue Aug 2, 2017
@mhdawson @addaleax
deps: limit regress/regress-crbug-514081 v8 test
160e2f0 
regress/regress-crbug-514081 allocates a 2G block of memory
and if there  are multiple variants running at the
same time this can lead to crashes, OOM kills or
the OS failing to allocate memory.  This patch
limits us to running a single variant of the test

Fixes: #6340
Refs: #6678

Backport-PR-URL: #14574
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport-Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax added a commit that referenced this issue Aug 2, 2017
@bnoordhuis @addaleax
v8: fix stack overflow in recursive method
357873d 
HGlobalValueNumberingPhase::CollectSideEffectsOnPathsToDominatedBlock()
used to self-recurse before this commit, causing stack overflows on
systems with small stack sizes.  Make it non-recursive by storing
intermediate results in a heap-allocated list.

Fixes: #11991
Refs: #12460

Backport-PR-URL: #14574
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport-Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax added a commit that referenced this issue Aug 2, 2017
@bzoz @addaleax
deps: fix addons compilation with VS2013
efd297a 
VS2013 does not support defaulting move constructor and assignment
operator. This adds explicit definitions of those methods for two
classes.
This fix is required because we still support building addons with
VS2013 and the incompatibility is in v8.h.

Fixes: nodejs/node-v8#4
Refs: #13263

Backport-PR-URL: #14574
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport-Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax added a commit that referenced this issue Aug 2, 2017
@targos @addaleax
deps: backport c0f1ff2 from upstream V8
2dbf95d 
Original commit message:

    Fix GCC 7 build errors

    BUG=chromium:691681
    R=franzih@chromium.org

    Change-Id: Id7e5698487f16dc217a804f6d3f24da7213c72b9
    Reviewed-on: https://chromium-review.googlesource.com/530227
    Commit-Queue: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#46045}

Refs: #13517
Fixes: #10388
Refs: #12392

Backport-PR-URL: #14574
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport-Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax added a commit that referenced this issue Aug 2, 2017
@targos @addaleax
src: fix new V8 compiler warnings
997204a 
Backport-PR-URL: #14574
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport-Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax added a commit that referenced this issue Aug 2, 2017
@hashseed @addaleax
deps: backport rehash strings after deserialization
e2b306c 
Original commit messages:
v8/v8@a2ab135
  [snapshot] Rehash strings after deserialization.

  See https://goo.gl/6aN8xA

  Bug: v8:6593
  Change-Id: Ic8b0b57195d01d41591397d5d45de3f0f3ebc3d9
  Reviewed-on: https://chromium-review.googlesource.com/574527
  Reviewed-by: Camillo Bruni <cbruni@chromium.org>
  Reviewed-by: Jakob Gruber <jgruber@chromium.org>
  Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
  Commit-Queue: Yang Guo <yangguo@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#46732}

v8/v8@182caaf
  Do not track transitions for built-in objects.

  Objects created during bootstrapping do not need
  a transition tree except for elements kind transitions.

  Bug: v8:6596
  Change-Id: I237b8b2792f201336e1c9731c815095dd06bc182
  Reviewed-on: https://chromium-review.googlesource.com/571750
  Reviewed-by: Igor Sheludko <ishell@chromium.org>
  Commit-Queue: Yang Guo <yangguo@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#46693}

Fixes: #14171
Refs: #14345

Backport-PR-URL: #14574
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport-Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: #14004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleax addaleax mentioned this pull request Aug 2, 2017
Merged
@targos targos deleted the v8-6.0 branch Aug 10, 2017
MylesBorins added a commit to MylesBorins/node that referenced this issue Oct 2, 2017
@targos @MylesBorins
build: run test-hash-seed at the end of test-v8
4cb743d 
The v8 and test-hash-seed targets cannot be run in parallel because they
need different copies of the deps/v8 directory.

Ref: nodejs#14004 (comment)
PR-URL: nodejs#14219
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins added a commit that referenced this issue Oct 18, 2017
@targos @MylesBorins
build: run test-hash-seed at the end of test-v8
2ae2a96 
The v8 and test-hash-seed targets cannot be run in parallel because they
need different copies of the deps/v8 directory.

Ref: #14004 (comment)
Backport-PR-URL: #15562
PR-URL: #14219
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins added a commit that referenced this issue Oct 25, 2017
@targos @MylesBorins
build: run test-hash-seed at the end of test-v8
a7f7a87 
The v8 and test-hash-seed targets cannot be run in parallel because they
need different copies of the deps/v8 directory.

Ref: #14004 (comment)
Backport-PR-URL: #15562
PR-URL: #14219
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@jasnell jasnell mentioned this pull request Oct 30, 2017
Merged
@remojansen remojansen mentioned this pull request Dec 15, 2017
Closed
@nfalco79 nfalco79 mentioned this pull request Feb 14, 2018
Closed
@refack refack mentioned this pull request Nov 23, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina

@refack refack

@fhinkel fhinkel

@jasnell jasnell

@addaleax addaleax

@cjihrig cjihrig

Assignees
No one assigned
Labels
semver-major v8 engine
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet