CPU profile displayed in three tables with a tree

CPU profile displayed in three tables with a tree

Table #1:
All functions sortable on inclusive or exclusive.

Table #2:
Callers of focused function.

Table #3:
Callees of focused function.

Tree:
Inclusive or Exclusive rooted profile tree filtered by focused function.

Also includes:

- Kill LinkLocationManager.
- Merge HashLocationManager into LocationManager.
- Kill nav-control element.
- Add goParameter to navigate based on page parameters (e.g. ?isolateId=...).

R=turnidge@google.com

Committed: https://code.google.com/p/dart/source/detail?r=45348

[Cutch, 2015-04-05 22:59:34]

johnmccutchan@google.com changed reviewers:
+ turnidge@google.com

[Cutch, 2015-04-05 23:03:27]

PTAL

[turnidge, 2015-04-07 21:24:31]

Sending partial review now... other ideas discussed offline.

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
File runtime/observatory/lib/src/app/location_manager.dart (right):

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/app/location_manager.dart:9: class FragmentUri {
Do we need to introduce this class?  What if we just stored the fragment as a
Uri?

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
File runtime/observatory/lib/src/cpu_profile/cpu_profile.dart (right):

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/cpu_profile/cpu_profile.dart:57: _markCodeCalls() {
What does "marking" mean here?  Is there another name for this?  Maybe
"SetCallCounts" or maybe "RecordCalls".

BTW, What do you think of this class being named "CallTree" instead of
"CodeCallTree"?

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/cpu_profile/cpu_profile.dart:152: class
_FilteredTreeBuilder {
Can you make some of the functions in this class private so I can tell which
ones are external entry points?  Maybe add some one-line comments on the
functions too.

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/cpu_profile/cpu_profile.dart:412:
_noteCaller(ProfileCode caller, int count) {
Maybe _addCaller and _addCallee?

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/cpu_profile.dart (right):

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/elements/cpu_profile.dart:520: print('$num');
Does this print belong here still?

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/css/shared.css (right):

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/elements/css/shared.css:193: display: flex;
Is this change needed?

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/elements/css/shared.css:224: display: flex;
Here too.

[Cutch, 2015-04-14 21:45:15]

PTAL

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
File runtime/observatory/lib/src/app/location_manager.dart (right):

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/app/location_manager.dart:9: class FragmentUri {
On 2015/04/07 21:24:31, turnidge wrote:
> Do we need to introduce this class?  What if we just stored the fragment as a
> Uri?

I've redone this part of the CL.

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
File runtime/observatory/lib/src/cpu_profile/cpu_profile.dart (right):

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/cpu_profile/cpu_profile.dart:57: _markCodeCalls() {
On 2015/04/07 21:24:31, turnidge wrote:
> What does "marking" mean here?  Is there another name for this?  Maybe
> "SetCallCounts" or maybe "RecordCalls".
> 
> BTW, What do you think of this class being named "CallTree" instead of
> "CodeCallTree"?

Went with _recordCallerAndCallees.

The class is named CodeCallTree because there is also a FunctionCallTree (Code
vs. Function).

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/cpu_profile/cpu_profile.dart:152: class
_FilteredTreeBuilder {
On 2015/04/07 21:24:31, turnidge wrote:
> Can you make some of the functions in this class private so I can tell which
> ones are external entry points?  Maybe add some one-line comments on the
> functions too.

Done.

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/cpu_profile/cpu_profile.dart:412:
_noteCaller(ProfileCode caller, int count) {
On 2015/04/07 21:24:31, turnidge wrote:
> Maybe _addCaller and _addCallee?

Done.

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/cpu_profile.dart (right):

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/elements/cpu_profile.dart:520: print('$num');
On 2015/04/07 21:24:31, turnidge wrote:
> Does this print belong here still?

Done.

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/css/shared.css (right):

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/elements/css/shared.css:193: display: flex;
On 2015/04/07 21:24:31, turnidge wrote:
> Is this change needed?

Done.

https://codereview.chromium.org/1013563002/diff/140001/runtime/observatory/li...
runtime/observatory/lib/src/elements/css/shared.css:224: display: flex;
On 2015/04/07 21:24:31, turnidge wrote:
> Here too.

Done.

[turnidge, 2015-04-16 17:23:45]

lgtm

https://codereview.chromium.org/1013563002/diff/160001/runtime/observatory/li...
File runtime/observatory/lib/src/app/location_manager.dart (right):

https://codereview.chromium.org/1013563002/diff/160001/runtime/observatory/li...
runtime/observatory/lib/src/app/location_manager.dart:16: Uri get uri => _uri;
Maybe some blank lines between decls?

[Cutch, 2015-04-22 14:05:09]

https://codereview.chromium.org/1013563002/diff/160001/runtime/observatory/li...
File runtime/observatory/lib/src/app/location_manager.dart (right):

https://codereview.chromium.org/1013563002/diff/160001/runtime/observatory/li...
runtime/observatory/lib/src/app/location_manager.dart:16: Uri get uri => _uri;
On 2015/04/16 17:23:44, turnidge wrote:
> Maybe some blank lines between decls?

Done.

[Cutch, 2015-04-22 14:06:00]

Committed patchset #10 (id:180001) manually as 45348 (presubmit successful).