[v8.x] deps: update V8 to 5.9#13515
Closed
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or 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
targos added dont-land-on-v4.x semver-minor 
nodejs-github-bot added v8 engine 
MemberAuthor
targos commented Jun 7, 2017
@addaleax thank you for doing the forward compatibility in multiple commits. That helped a lot to prepare this :) |
addaleax approved these changes Jun 7, 2017
Member
addaleax left a comment
addaleax left a comment There was a problem hiding this comment.
Looked through the include/ diffs to Node 8.0.0 and V8 6.0, this should be good to go 👍
Member
gibfahn commented Jun 7, 2017
cc/ @nodejs/lts, note that targos@ec7cbaf updates the |
Member
gibfahn commented Jun 7, 2017
Here's a CI: https://ci.nodejs.org/job/node-test-commit/10418/ I guess this also needs CitGM and V8 tests. |
MemberAuthor
targos commented Jun 7, 2017
There seems to be an issue with #10388 on master (and therefore here too). None of the existing fixes apply cleanly to V8 5.9 and I don't have gcc 7 to investigate. |
MemberAuthor
targos commented Jun 7, 2017
Updated to add the fix from #10388 (comment): 99d154e |
MemberAuthor
targos commented Jun 7, 2017
Contributor
MylesBorins commented Jun 7, 2017
I'll try and get the ABI smoker up and running today so we can test |
Member
Btw, you might also want to include v8/v8@a16c3c910548e – it’s the only breaking change in V8 6.0 for a feature that was just introduced in V8 5.9. I doubt anybody gets hurt when we don’t include it, but I’d guess it applies cleanly. |
MemberAuthor
targos commented Jun 12, 2017
|
MemberAuthor
targos commented Jun 13, 2017
jasnell approved these changes Jun 13, 2017
winksaville commented Jun 13, 2017
I found a performance issue which might be related to node using v8 5.8. What I see is that my application is 6x slower targeting es6 vs es5. For details see this post on the v8 email list. When this is merged I'll test again and see if v8 5.9 resolves the issue as is speculated. |
Member
addaleax commented Jun 16, 2017
@targos Could you rebase this against v8.x-staging? Github doesn’t see a problem but I can’t apply it cleanly. |
Member
addaleax commented Jun 17, 2017
Okay, I’ve rebased this. The previous head was at 532be47fc690d300b5b097d3e529bc5a9b60394d, in case it’s necessary to roll back. CI: https://ci.nodejs.org/job/node-test-commit/10623/ |
Member
addaleax commented Jun 17, 2017
Landed in 369b4e53e8806d38cbe1d8f57abde9fb465427a8...36bab2190a75c4c615e91b0a924fd84f4110ec0f |
PR-URL: nodejs#12875 Reviewed-By: James M Snell <[email protected]>
Original commit message: [PATCH] Merged: Make Object::GetOwnPropertyDescriptor() take a Name, not a String. Revision: b5e610c19208ef854755eec67011ca7aff008bf4 NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true [email protected] Bug: Change-Id: I396b559b28aab6afa138db747711e50cd0da3da7 Reviewed-on: https://chromium-review.googlesource.com/513927 Reviewed-by: Michael Achenbach <[email protected]> Cr-Commit-Position: refs/branch-heads/6.0@{#5} Cr-Branched-From: 97dbf624a5eeffb3a8df36d24cdb2a883137385f-refs/heads/6.0.286@{#1} Cr-Branched-From: 12e6f1cb5cd9616da7b9d4a7655c088778a6d415-refs/heads/master@{nodejs#45439} PR-URL: nodejs#13217 Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Because 5.8 still had other uses of the removed flag, there are some extra changes in Heap::ConfigureHeap and api.cc:SetResourceConstraints. Original commit message: [heap] Remove max_executable_size resource constraint. BUG=chromium:716032 Review-Url: https://codereview.chromium.org/2890603007 Cr-Commit-Position: refs/heads/master@{nodejs#45400} PR-URL: nodejs#13217 Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: [Api] Add an idle time garbage collection callback flag to GCCallbackFlags. BUG=chromium:718484 Review-Url: https://codereview.chromium.org/2867073002 Cr-Commit-Position: refs/heads/master@{nodejs#45167} PR-URL: nodejs#13217 Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: [PATCH] Rename idle garbage collection callback flag. [email protected] Review-Url: https://codereview.chromium.org/2867863002 Cr-Commit-Position: refs/heads/master@{nodejs#45173} PR-URL: nodejs#13217 Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport new virtual methods from 18a26cfe174 ("Add memory protection API to ArrayBuffer::Allocator") PR-URL: nodejs#13217 Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Expose the ValueSerializer data format version as a compile-time constant. BUG=chromium:704293 Review-Url: https://codereview.chromium.org/2804643006 Cr-Commit-Position: refs/heads/master@{nodejs#44945}
Original commit message: [string] Re-enable result caching for String.p.split Runtime::kStringSplit's result caching is only enabled when limit equals kMaxUInt32. BUG=v8:6463 Review-Url: https://codereview.chromium.org/2923183002 Cr-Commit-Position: refs/heads/master@{nodejs#45724} Fixes: nodejs#13445
Adds missing return which fixes debug builds on Windows Fixes: nodejs#13392 Ref: https://codereview.chromium.org/2929993003/ PR-URL: nodejs#13634 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
MemberAuthor
targos commented Jul 21, 2017
@addaleax rebased ;) |
PR-URL: nodejs#13790 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Original commit message: d8: Make in process stack dumping optional Adds a flag (--disable-in-process-stack-traces) to not install signal handlers so that e.g. ASan signal handlers will work. This flag mirrors chromium's one. [email protected] BUG=chromium:716235 Review-Url: https://codereview.chromium.org/2854173002 Cr-Commit-Position: refs/heads/master@{nodejs#45142} PR-URL: nodejs#13985 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Since V8 5.9 V8 installs a default signal handler for some signals when creating a default platform instance that prints a stack trace. However, Node already does the same thing, so it would seem like the two different stack traces would be printed; also, the V8 handler would lead to a `SIGSEGV` under some circumstances, rather than letting the abort continue normally. Resolve this by disabling V8’s signal handler by default. PR-URL: nodejs#13985Fixes: nodejs#13865 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Member
@targos I cherry-picked 3 more commits that should be part of the upgrade if I understand correctly (and previously were on v8.x-staging), please remove if I’m mistaken. :) |
addaleax pushed a commit that referenced this pull request Jul 24, 2017
PR-URL: #13515 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
addaleax pushed a commit that referenced this pull request Jul 24, 2017
Porting #12392 to V8 5.9 Ref: #12392Fixes: #10388 PR-URL: #13515 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
addaleax pushed a commit that referenced this pull request Jul 24, 2017
Original commit message: Expose the ValueSerializer data format version as a compile-time constant. BUG=chromium:704293 Review-Url: https://codereview.chromium.org/2804643006 Cr-Commit-Position: refs/heads/master@{#44945} PR-URL: #13515 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
addaleax pushed a commit that referenced this pull request Jul 24, 2017
Original commit message: [string] Re-enable result caching for String.p.split Runtime::kStringSplit's result caching is only enabled when limit equals kMaxUInt32. BUG=v8:6463 Review-Url: https://codereview.chromium.org/2923183002 Cr-Commit-Position: refs/heads/master@{#45724} Fixes: #13445 PR-URL: #13515 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Closed
Original commit message: Properly handle loads from global interceptor via prototype chain. ... when receiver is in dictionary mode. Bug: v8:6490 Change-Id: Ic5a8d214adcc4efd4cb163cbc6b351c4e6b596af Reviewed-on: https://chromium-review.googlesource.com/559548 Reviewed-by: Camillo Bruni <[email protected]> Commit-Queue: Igor Sheludko <[email protected]> Cr-Commit-Position: refs/heads/master@{nodejs#46428} Ref: https://chromium.googlesource.com/v8/v8.git/+/6cb999b97b7953ebfd4aabf2e1f62bf405f21c69Fixes: nodejs#13804 PR-URL: nodejs#14188 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
Member
addaleax commented Jul 27, 2017
I guess this is just not going to happen. Let’s wait for 6.0! |
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
19 participants
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 is a fresh update of V8 applied to v8.x-staging with:
/cc @nodejs/v8 @jasnell@MylesBorins@addaleax
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
V8