[10.x] deps: V8: backport 4 CPU profiler commits from upstream #22028

psmarshall · 2018-07-30T12:03:04Z

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

[cpu-profiler] Add a new profiling mode with a more detailed call tree.
https://chromium.googlesource.com/v8/v8.git/+/ecae80cdb350dde1e654c531b56f5b6c44dc8c77

[cpu-profiler] Reuse free slots in code_entries_
https://chromium.googlesource.com/v8/v8.git/+/3e1126bf15e62c433c4e9cb21316d182f691c63a

[cpu-profiler] Only store deopt inline frames for functions that need it
https://chromium.googlesource.com/v8/v8.git/+/0bfcbdd4726920755e51dab28c18ab93e050819b

[cpu-profiler] Use instruction start as the key for the CodeMap
https://chromium.googlesource.com/v8/v8.git/+/ba752ea4c50713dff1e94f45a79db3ba968a8d66

psmarshall · 2018-07-30T12:05:09Z

I'd like to land this with the V8 update (to 6.8) on 10.x

ofrobots · 2018-08-01T18:50:03Z

/cc @nodejs/v8-update

EDIT:
CI: https://ci.nodejs.org/job/node-test-pull-request/16130/
V8-CI: https://ci.nodejs.org/view/All/job/node-test-commit-v8-linux/

ofrobots · 2018-08-01T18:52:34Z

v8_embedder_string in common.gypi should be bumped (by 1).

psmarshall · 2018-08-03T11:21:29Z

Thanks, I've bumped v8_embedder_string

psmarshall · 2018-08-16T09:37:13Z

@ofrobots I've rebased now that V8 6.8 has landed on v10.x-staging, so we should be OK to land this so that it can be in the 10.10 release

BridgeAR

RSLGTM

bmeurer

LGTM (rubber-stamped)

targos · 2018-08-29T12:08:27Z

Any reason not to land this on the master branch?

psmarshall · 2018-08-29T14:07:34Z

@targos I don't see why not. I'll retarget it to master

[cpu-profiler] Add a new profiling mode with a more detailed call tree. https://chromium.googlesource.com/v8/v8.git/+/ecae80cdb350dde1e654c531b56f5b6c44dc8c77 [cpu-profiler] Reuse free slots in code_entries_ https://chromium.googlesource.com/v8/v8.git/+/3e1126bf15e62c433c4e9cb21316d182f691c63a [cpu-profiler] Only store deopt inline frames for functions that need it https://chromium.googlesource.com/v8/v8.git/+/0bfcbdd4726920755e51dab28c18ab93e050819b [cpu-profiler] Use instruction start as the key for the CodeMap https://chromium.googlesource.com/v8/v8.git/+/ba752ea4c50713dff1e94f45a79db3ba968a8d66

targos · 2018-08-29T14:35:36Z

Thanks! CI: https://ci.nodejs.org/job/node-test-pull-request/16858/

targos · 2018-09-03T17:50:06Z

Landed in 8ac662e

[cpu-profiler] Add a new profiling mode with a more detailed call tree. https://chromium.googlesource.com/v8/v8.git/+/ecae80cdb350dde1e654c531b56f5b6c44dc8c77 [cpu-profiler] Reuse free slots in code_entries_ https://chromium.googlesource.com/v8/v8.git/+/3e1126bf15e62c433c4e9cb21316d182f691c63a [cpu-profiler] Only store deopt inline frames for functions that need it https://chromium.googlesource.com/v8/v8.git/+/0bfcbdd4726920755e51dab28c18ab93e050819b [cpu-profiler] Use instruction start as the key for the CodeMap https://chromium.googlesource.com/v8/v8.git/+/ba752ea4c50713dff1e94f45a79db3ba968a8d66 PR-URL: #22028 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Benedikt Meurer <[email protected]>

targos · 2018-09-03T18:40:34Z

It turns out this change breaks V8 tests:

https://ci.nodejs.org/view/All/job/node-test-commit-v8-linux/nodes=ubuntu1604-intel-64,v8test=v8test/1651/console

14:15:07 ../../test/cctest/test-log.cc:741:59: error: non-virtual member function marked 'override' hides virtual member function
14:15:07     void CodeMoveEvent(i::AbstractCode* from, Address to) override {}
14:15:07                                                           ^
14:15:07 ../../src/code-events.h:86:16: note: hidden overloaded virtual function 'v8::internal::CodeEventListener::CodeMoveEvent' declared here: type mismatch at 2nd parameter ('v8::internal::AbstractCode *' vs 'v8::internal::Address' (aka 'unsigned long'))
14:15:07   virtual void CodeMoveEvent(AbstractCode* from, AbstractCode* to) = 0;
14:15:07                ^
14:15:07 ../../test/cctest/test-log.cc:750:5: error: variable type 'class (anonymous class at ../../test/cctest/test-log.cc:739:3)' is an abstract class
14:15:07   } code_event_logger;
14:15:07     ^
14:15:07 ../../src/code-events.h:86:16: note: unimplemented pure virtual method 'CodeMoveEvent' in ''
14:15:07   virtual void CodeMoveEvent(AbstractCode* from, AbstractCode* to) = 0;
14:15:07                ^
14:15:07 2 errors generated.

Edit: fix at #22677

Fixes a regression introduced in a V8 backport PR. A small change in cctest/test-log.cc was forgotten. Refs: nodejs#22028 Refs: v8/v8@ba752ea

[cpu-profiler] Add a new profiling mode with a more detailed call tree. https://chromium.googlesource.com/v8/v8.git/+/ecae80cdb350dde1e654c531b56f5b6c44dc8c77 [cpu-profiler] Reuse free slots in code_entries_ https://chromium.googlesource.com/v8/v8.git/+/3e1126bf15e62c433c4e9cb21316d182f691c63a [cpu-profiler] Only store deopt inline frames for functions that need it https://chromium.googlesource.com/v8/v8.git/+/0bfcbdd4726920755e51dab28c18ab93e050819b [cpu-profiler] Use instruction start as the key for the CodeMap https://chromium.googlesource.com/v8/v8.git/+/ba752ea4c50713dff1e94f45a79db3ba968a8d66 PR-URL: #22028 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Benedikt Meurer <[email protected]>

Fixes a regression introduced in a V8 backport PR. A small change in cctest/test-log.cc was forgotten. Refs: nodejs#22028 Refs: v8/v8@ba752ea PR-URL: nodejs#22677 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>

psmarshall · 2018-09-04T13:17:42Z

Thanks for fixing that. I think I somehow missed the changes to test-log.cc when I was fixing conflicts - the fix to the test should have been included in this one:

[cpu-profiler] Use instruction start as the key for the CodeMap
https://chromium.googlesource.com/v8/v8.git/+/ba752ea4c50713dff1e94f45a79db3ba968a8d66

[cpu-profiler] Add a new profiling mode with a more detailed call tree. https://chromium.googlesource.com/v8/v8.git/+/ecae80cdb350dde1e654c531b56f5b6c44dc8c77 [cpu-profiler] Reuse free slots in code_entries_ https://chromium.googlesource.com/v8/v8.git/+/3e1126bf15e62c433c4e9cb21316d182f691c63a [cpu-profiler] Only store deopt inline frames for functions that need it https://chromium.googlesource.com/v8/v8.git/+/0bfcbdd4726920755e51dab28c18ab93e050819b [cpu-profiler] Use instruction start as the key for the CodeMap https://chromium.googlesource.com/v8/v8.git/+/ba752ea4c50713dff1e94f45a79db3ba968a8d66 PR-URL: #22028 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Benedikt Meurer <[email protected]>

Fixes a regression introduced in a V8 backport PR. A small change in cctest/test-log.cc was forgotten. Refs: #22028 Refs: v8/v8@ba752ea PR-URL: #22677 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>

[cpu-profiler] Add a new profiling mode with a more detailed call tree. https://chromium.googlesource.com/v8/v8.git/+/ecae80cdb350dde1e654c531b56f5b6c44dc8c77 [cpu-profiler] Reuse free slots in code_entries_ https://chromium.googlesource.com/v8/v8.git/+/3e1126bf15e62c433c4e9cb21316d182f691c63a [cpu-profiler] Only store deopt inline frames for functions that need it https://chromium.googlesource.com/v8/v8.git/+/0bfcbdd4726920755e51dab28c18ab93e050819b [cpu-profiler] Use instruction start as the key for the CodeMap https://chromium.googlesource.com/v8/v8.git/+/ba752ea4c50713dff1e94f45a79db3ba968a8d66 PR-URL: #22028 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Benedikt Meurer <[email protected]>

Fixes a regression introduced in a V8 backport PR. A small change in cctest/test-log.cc was forgotten. Refs: #22028 Refs: v8/v8@ba752ea PR-URL: #22677 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>

nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Jul 30, 2018

psmarshall force-pushed the backport-cpu-profiler-to-v10.x branch from c5be937 to 406d2f6 Compare August 3, 2018 11:21

psmarshall changed the base branch from master to v10.x-staging August 3, 2018 11:30

rvagg force-pushed the v10.x-staging branch from 82830a8 to b1adb44 Compare August 16, 2018 01:42

psmarshall force-pushed the backport-cpu-profiler-to-v10.x branch from 406d2f6 to 4445b31 Compare August 16, 2018 09:35

BridgeAR approved these changes Aug 16, 2018

View reviewed changes

psmarshall mentioned this pull request Aug 22, 2018

[8.x] deps: V8: backport 20 CPU profiler commits from upstream #21558

Closed

2 tasks

psmarshall force-pushed the backport-cpu-profiler-to-v10.x branch from 4445b31 to fe2ada8 Compare August 29, 2018 09:04

bmeurer approved these changes Aug 29, 2018

View reviewed changes

psmarshall changed the base branch from v10.x-staging to master August 29, 2018 14:14

psmarshall force-pushed the backport-cpu-profiler-to-v10.x branch from fe2ada8 to 95072d7 Compare August 29, 2018 14:15

targos closed this Sep 3, 2018

targos added a commit to targos/node that referenced this pull request Sep 3, 2018

deps: fix V8 test regression

d8b470a

Fixes a regression introduced in a V8 backport PR. A small change in cctest/test-log.cc was forgotten. Refs: nodejs#22028 Refs: v8/v8@ba752ea

targos mentioned this pull request Sep 3, 2018

deps: fix V8 test regression #22677

Closed

targos added dont-land-on-v6.x labels Sep 4, 2018

targos mentioned this pull request Sep 5, 2018

Release proposal: v10.10.0 #22716

Merged

This was referenced Sep 6, 2018

2018-09-06 Version 10.10.0 (Current) @targos beevelop/docker-android-nodejs#236

Closed

2018-09-06 Version 10.10.0 (Current) @targos beevelop/docker-nodejs#221

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[10.x] deps: V8: backport 4 CPU profiler commits from upstream #22028

[10.x] deps: V8: backport 4 CPU profiler commits from upstream #22028

psmarshall commented Jul 30, 2018

psmarshall commented Jul 30, 2018

ofrobots commented Aug 1, 2018 •

edited

Loading

ofrobots commented Aug 1, 2018

psmarshall commented Aug 3, 2018

psmarshall commented Aug 16, 2018

BridgeAR left a comment

bmeurer left a comment

targos commented Aug 29, 2018

psmarshall commented Aug 29, 2018

targos commented Aug 29, 2018

targos commented Sep 3, 2018

targos commented Sep 3, 2018 •

edited

Loading

psmarshall commented Sep 4, 2018

[10.x] deps: V8: backport 4 CPU profiler commits from upstream #22028

[10.x] deps: V8: backport 4 CPU profiler commits from upstream #22028

Conversation

psmarshall commented Jul 30, 2018

Checklist

psmarshall commented Jul 30, 2018

ofrobots commented Aug 1, 2018 • edited Loading

ofrobots commented Aug 1, 2018

psmarshall commented Aug 3, 2018

psmarshall commented Aug 16, 2018

BridgeAR left a comment

Choose a reason for hiding this comment

bmeurer left a comment

Choose a reason for hiding this comment

targos commented Aug 29, 2018

psmarshall commented Aug 29, 2018

targos commented Aug 29, 2018

targos commented Sep 3, 2018

targos commented Sep 3, 2018 • edited Loading

psmarshall commented Sep 4, 2018

ofrobots commented Aug 1, 2018 •

edited

Loading

targos commented Sep 3, 2018 •

edited

Loading