Add callers and callees to profiler output

Code regions now have list of callers and callees.
Address ticks track exclusive / inclusive separately.
Code regions only receive one inclusive tick per sample regardless of how many times they appear in the sample's stack trace.
Provide useful name for most stub functions.

R=asiva@google.com, turnidge@google.com

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

[siva, 2014-02-21 22:23:48]

LGTM with comments.

https://codereview.chromium.org/168833005/diff/1/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/168833005/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:358: }
These two functions are identical except for the table on which they operate,
maybe you could have a helper method

AddCallEntry(ZoneGrowableArray<CallEntry>* table, intptr_t index) {
  ....
  ....
}

AddCaller(intptr_t index) {
  AddCallEntry(callers_table_, index);
}

AddCallee(intptr_t index) {
  AddCallEntry(callees_table_, index);
}

https://codereview.chromium.org/168833005/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:416: JSONArray callers(&obj, "callees");
name the variable callees instead of callers to make it more readable?

https://codereview.chromium.org/168833005/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:517: intptr_t FindIndex(uword pc) {
const function too?

[Cutch, 2014-02-24 15:18:56]

https://codereview.chromium.org/168833005/diff/1/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/168833005/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:358: }
On 2014/02/21 22:23:48, siva wrote:
> These two functions are identical except for the table on which they operate,
> maybe you could have a helper method
> 
> AddCallEntry(ZoneGrowableArray<CallEntry>* table, intptr_t index) {
>   ....
>   ....
> }
> 
> AddCaller(intptr_t index) {
>   AddCallEntry(callers_table_, index);
> }
> 
> AddCallee(intptr_t index) {
>   AddCallEntry(callees_table_, index);
> }

Done.

https://codereview.chromium.org/168833005/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:416: JSONArray callers(&obj, "callees");
On 2014/02/21 22:23:48, siva wrote:
> name the variable callees instead of callers to make it more readable?

Done.

https://codereview.chromium.org/168833005/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:517: intptr_t FindIndex(uword pc) {
On 2014/02/21 22:23:48, siva wrote:
> const function too?

Done.

[Cutch, 2014-02-24 19:21:55]

Todd, siva reviewed the backend changes but I'd like you to review the frontend
changes.

[turnidge, 2014-02-24 20:12:59]

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
File runtime/bin/vmservice/client/lib/src/observatory/model.dart (right):

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:63: class CodeCaller
{
This isn't necessarily a caller, right?  It can be a caller or a callee?  How
would you feel about the name CallInfo or CallCountInfo?

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:88: int
calleesCount(Code code) => _individualCallCount(callees, code);
Maybe add simple comments.

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:98: int
_individualCallCount(List<CodeCaller> calls, Code code) {
Maybe just _callCount?

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:186:
assert(_codeObjectsInImportOrder.length == codes.length);
Maybe a comment here, along the lines of:

"Once all code objects have been processed, visit each code object and resolve
its callers and callees."

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:189: Map codeService
= codes[i];
codeService sounds like a service that returns codes.  Would be clearer with
just codes[i].

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:190:
_loadCallData(codeModel.callers, codeService['callers']);
Would this work as an assignment?

codeModel.callers = _loadCallData(codes[i].callers);

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:196: void
_loadCallData(List<CodeCaller> calls, List data) {
How about a different name?  Maybe "_resolveCalls" or something.

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:210:
codeCaller.code_or_index = _codeObjectsInImportOrder[index];
Do you need this second loop?  Why not resolve the index in the first loop like
this:

var codeCaller = new CodeCaller();
var index = int.parse(data[i]);
assert(index >= 0);
assert(index < _codeObjectsInImportOrder.length);
codeCaller.code = _codeObjectsInImportOrder[index];
calls.add(codeCaller);

This would also mean that code_or_index could become code.

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
File runtime/bin/vmservice/client/lib/src/observatory/view_model.dart (right):

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/view_model.dart:42: final
List<TableTreeRow> _children = new List<TableTreeRow>();
Remove _children?  I'm not sure that it is needed.

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
File
runtime/bin/vmservice/client/lib/src/observatory_elements/isolate_profile.dart
(right):

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory_elements/isolate_profile.dart:71:
var tte = shadowRoot.querySelector(_id);
I can guess what this does, but I want to chat about it.

[Cutch, 2014-02-25 16:44:46]

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
File runtime/bin/vmservice/client/lib/src/observatory/model.dart (right):

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:63: class CodeCaller
{
On 2014/02/24 20:13:00, turnidge wrote:
> This isn't necessarily a caller, right?  It can be a caller or a callee?  How
> would you feel about the name CallInfo or CallCountInfo?

Done.

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:88: int
calleesCount(Code code) => _individualCallCount(callees, code);
On 2014/02/24 20:13:00, turnidge wrote:
> Maybe add simple comments.

Done.

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:98: int
_individualCallCount(List<CodeCaller> calls, Code code) {
On 2014/02/24 20:13:00, turnidge wrote:
> Maybe just _callCount?

Done.

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:186:
assert(_codeObjectsInImportOrder.length == codes.length);
On 2014/02/24 20:13:00, turnidge wrote:
> Maybe a comment here, along the lines of:
> 
> "Once all code objects have been processed, visit each code object and resolve
> its callers and callees."

Done.

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:196: void
_loadCallData(List<CodeCaller> calls, List data) {
On 2014/02/24 20:13:00, turnidge wrote:
> How about a different name?  Maybe "_resolveCalls" or something.

Done.

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/model.dart:210:
codeCaller.code_or_index = _codeObjectsInImportOrder[index];
On 2014/02/24 20:13:00, turnidge wrote:
> Do you need this second loop?  Why not resolve the index in the first loop
like
> this:
> 
> var codeCaller = new CodeCaller();
> var index = int.parse(data[i]);
> assert(index >= 0);
> assert(index < _codeObjectsInImportOrder.length);
> codeCaller.code = _codeObjectsInImportOrder[index];
> calls.add(codeCaller);
> 
> This would also mean that code_or_index could become code.

Done.

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
File runtime/bin/vmservice/client/lib/src/observatory/view_model.dart (right):

https://codereview.chromium.org/168833005/diff/170001/runtime/bin/vmservice/c...
runtime/bin/vmservice/client/lib/src/observatory/view_model.dart:42: final
List<TableTreeRow> _children = new List<TableTreeRow>();
On 2014/02/24 20:13:00, turnidge wrote:
> Remove _children?  I'm not sure that it is needed.

Done.

[turnidge, 2014-02-25 19:14:10]

lgtm

[Cutch, 2014-02-25 21:42:07]

Committed patchset #10 manually as r33036 (presubmit successful).