DevTools: extract CPU profile independent part of CPUProfileNode.

DevTools: extract CPU profile independent part of CPUProfileNode.

This is a prerequisite to make the profile node class reused for sampling heap profiler.

BUG=586613
Committed: https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
Cr-Commit-Position: refs/heads/master@{#387249}

Committed: https://crrev.com/4feec764268174177b6d0ec3db7d7b81120a3ad6
Cr-Commit-Position: refs/heads/master@{#388359}

[alph, 2016-04-08 20:56:58]

alph@chromium.org changed reviewers:
+ caseq@chromium.org

[caseq, 2016-04-13 17:21:13]

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js
(right):

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:23:
this.deoptReason = sourceNode.deoptReason;
Why do we have to do this? Having to copy all fields of all nodes is quite
unfortunate. Can these be getters?

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:65: var
idToNode = {};
use Map()?

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:67:
while (stack.length) {
can this be combined with tree traversal we perform in processSubtree?

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:78: node
= node.parent;
so we actually care to know nearest non-native parent for each node id? I think
you can easily factor this into the  loop you introduced above in a more
efficient manner.

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:143:
function treeHitCount(node)
nit: computeHitCountForSubtree()

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:150: //
The stack contains pairs: (parentNode, sourceNode.children)
Please make it an array of objects, this way it's a bit confusing.

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js
(right):

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:7: *
@implements {RuntimeAgent.CallFrame}
WAT?

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:37: *
@param {!WebInspector.ProfileNode} head
nit: s/head/root/ everywhere?

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:51:
_assignInfo: function()
_updateDepthAndParent?

[alph, 2016-04-13 19:05:08]

ptal

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js
(right):

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:23:
this.deoptReason = sourceNode.deoptReason;
On 2016/04/13 17:21:12, caseq wrote:
> Why do we have to do this? Having to copy all fields of all nodes is quite
> unfortunate. Can these be getters?

This is done just once for the tree. I want to have fast access though.

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:65: var
idToNode = {};
On 2016/04/13 17:21:12, caseq wrote:
> use Map()?

Done.

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:67:
while (stack.length) {
On 2016/04/13 17:21:12, caseq wrote:
> can this be combined with tree traversal we perform in processSubtree?

Yes, building the tree along with filtering should be faster and simpler, but
I'd like to have a separate patch for that.

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:78: node
= node.parent;
On 2016/04/13 17:21:12, caseq wrote:
> so we actually care to know nearest non-native parent for each node id? I
think
> you can easily factor this into the  loop you introduced above in a more
> efficient manner.

I'm going to get rid of this function later. Added a TODO.

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:143:
function treeHitCount(node)
On 2016/04/13 17:21:12, caseq wrote:
> nit: computeHitCountForSubtree()

Done.

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:150: //
The stack contains pairs: (parentNode, sourceNode.children)
On 2016/04/13 17:21:12, caseq wrote:
> Please make it an array of objects, this way it's a bit confusing.

Done.

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js
(right):

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:7: *
@implements {RuntimeAgent.CallFrame}
On 2016/04/13 17:21:12, caseq wrote:
> WAT?

Fixed. Encapsulated it instead and added getters to retain compatibility.

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:37: *
@param {!WebInspector.ProfileNode} head
On 2016/04/13 17:21:12, caseq wrote:
> nit: s/head/root/ everywhere?

Renamed local variables. Left the rest for now to keep the patch small.

https://codereview.chromium.org/1873973002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:51:
_assignInfo: function()
On 2016/04/13 17:21:12, caseq wrote:
> _updateDepthAndParent?

Done.

[caseq, 2016-04-13 22:19:37]

lgtm

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js
(right):

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:61: /**
@type {!Map<number,!ProfilerAgent.CPUProfileNode>} */
nit: space after comma

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:144:
this.totalHitCount = computeHitCountForSubtree(root);
annotate this.totalHitCount in constructor?

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:147: var
targetNodeStack = [ resultRoot ];
[resultRoot]

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:148: var
sourceNodeStack = [ root ];
[root]

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js
(right):

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:36: get
functionName() {
{ => next line

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:43: get
scriptId() {
ditto

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:50: get
url() {
ditto

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:57: get
lineNumber() {
ditto

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:64: get
columnNumber() {
ditto

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ProfileTreeModel.js:91: var
nodesToTraverse = [ root ];
[root]

[alph, 2016-04-13 22:36:47]

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js
(right):

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:61: /**
@type {!Map<number,!ProfilerAgent.CPUProfileNode>} */
On 2016/04/13 22:19:36, caseq wrote:
> nit: space after comma

Done.

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:144:
this.totalHitCount = computeHitCountForSubtree(root);
On 2016/04/13 22:19:36, caseq wrote:
> annotate this.totalHitCount in constructor?

Done.

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:147: var
targetNodeStack = [ resultRoot ];
On 2016/04/13 22:19:36, caseq wrote:
> [resultRoot]

Done.

https://codereview.chromium.org/1873973002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/CPUProfileDataModel.js:148: var
sourceNodeStack = [ root ];
On 2016/04/13 22:19:36, caseq wrote:
> [root]

Done.

[alph, 2016-04-13 22:36:54]

The CQ bit was checked by alph@chromium.org

[alph, 2016-04-13 22:36:55]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org
Link to the patchset: https://codereview.chromium.org/1873973002/#ps40001
(title: "4 landing")

[commit-bot: I haz the power, 2016-04-13 22:37:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1873973002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1873973002/40001

[commit-bot: I haz the power, 2016-04-14 02:59:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-14 02:59:24]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[alph, 2016-04-14 05:46:39]

The CQ bit was checked by alph@chromium.org

[commit-bot: I haz the power, 2016-04-14 05:46:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1873973002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1873973002/40001

[commit-bot: I haz the power, 2016-04-14 06:58:24]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-14 06:59:55]

Description was changed from

==========
DevTools: extract CPU profile independent part of CPUProfileNode.

This is a prerequisite to make the profile node class reused for sampling heap
profiler.

BUG=586613
==========

to

==========
DevTools: extract CPU profile independent part of CPUProfileNode.

This is a prerequisite to make the profile node class reused for sampling heap
profiler.

BUG=586613

Committed: https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
Cr-Commit-Position: refs/heads/master@{#387249}
==========

[commit-bot: I haz the power, 2016-04-14 06:59:56]

Patchset 3 (id:??) landed as
https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
Cr-Commit-Position: refs/heads/master@{#387249}

[vabr (Chromium), 2016-04-14 09:07:27]

On 2016/04/14 06:59:56, commit-bot: I haz the power wrote:
> Patchset 3 (id:??) landed as
> https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
> Cr-Commit-Position: refs/heads/master@{#387249}

Could this have broken
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%2...
?

[alph, 2016-04-14 17:24:45]

On 2016/04/14 09:07:27, vabr (Chromium) wrote:
> On 2016/04/14 06:59:56, commit-bot: I haz the power wrote:
> > Patchset 3 (id:??) landed as
> > https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
> > Cr-Commit-Position: refs/heads/master@{#387249}
> 
> Could this have broken
>
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%2...
> ?

This is very unlikely. The modified code is not executed in tests outside
inspector/profielr

[caseq, 2016-04-15 19:07:50]

On 2016/04/14 17:24:45, alph wrote:
> On 2016/04/14 09:07:27, vabr (Chromium) wrote:
> > On 2016/04/14 06:59:56, commit-bot: I haz the power wrote:
> > > Patchset 3 (id:??) landed as
> > > https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
> > > Cr-Commit-Position: refs/heads/master@{#387249}
> > 
> > Could this have broken
> >
>
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%2...
> > ?
> 
> This is very unlikely. The modified code is not executed in tests outside
> inspector/profielr

vabr, I think that's the culprit.
alph, the test vabr points to is a timeline test.

More of similar failures here:
https://bugs.chromium.org/p/chromium/issues/detail?id=603767

[caseq, 2016-04-15 19:09:25]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1889963003/ by caseq@chromium.org.

The reason for reverting is: This broke at least two timeline tests with Cannot
read property 'parent' of null

BUG=603767.

[alph, 2016-04-19 22:09:41]

Description was changed from

==========
DevTools: extract CPU profile independent part of CPUProfileNode.

This is a prerequisite to make the profile node class reused for sampling heap
profiler.

BUG=586613

Committed: https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
Cr-Commit-Position: refs/heads/master@{#387249}
==========

to

==========
DevTools: extract CPU profile independent part of CPUProfileNode.

This is a prerequisite to make the profile node class reused for sampling heap
profiler.

BUG=586613

Committed: https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
Cr-Commit-Position: refs/heads/master@{#387249}
==========

[alph, 2016-04-19 22:18:33]

The CQ bit was checked by alph@chromium.org

[alph, 2016-04-19 22:18:34]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org
Link to the patchset: https://codereview.chromium.org/1873973002/#ps60001
(title: "fix tests.")

[commit-bot: I haz the power, 2016-04-19 22:18:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1873973002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1873973002/60001

[commit-bot: I haz the power, 2016-04-19 23:43:18]

Description was changed from

==========
DevTools: extract CPU profile independent part of CPUProfileNode.

This is a prerequisite to make the profile node class reused for sampling heap
profiler.

BUG=586613

Committed: https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
Cr-Commit-Position: refs/heads/master@{#387249}
==========

to

==========
DevTools: extract CPU profile independent part of CPUProfileNode.

This is a prerequisite to make the profile node class reused for sampling heap
profiler.

BUG=586613

Committed: https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
Cr-Commit-Position: refs/heads/master@{#387249}
==========

[commit-bot: I haz the power, 2016-04-19 23:43:19]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-22 19:18:12]

Description was changed from

==========
DevTools: extract CPU profile independent part of CPUProfileNode.

This is a prerequisite to make the profile node class reused for sampling heap
profiler.

BUG=586613

Committed: https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
Cr-Commit-Position: refs/heads/master@{#387249}
==========

to

==========
DevTools: extract CPU profile independent part of CPUProfileNode.

This is a prerequisite to make the profile node class reused for sampling heap
profiler.

BUG=586613

Committed: https://crrev.com/1c33b9ee4940826b3a972c093cbc3f8e1f1b3f50
Cr-Commit-Position: refs/heads/master@{#387249}

Committed: https://crrev.com/4feec764268174177b6d0ec3db7d7b81120a3ad6
Cr-Commit-Position: refs/heads/master@{#388359}
==========

[commit-bot: I haz the power, 2016-04-22 19:18:13]

Patchset 4 (id:??) landed as
https://crrev.com/4feec764268174177b6d0ec3db7d7b81120a3ad6
Cr-Commit-Position: refs/heads/master@{#388359}