Observatory: Include profiler samples in the timeline view.

Observatory: Include profiler samples in the timeline view.

 - Add pre-isolate service RPC that emits profiler samples in about:tracing format.
 - Make timeline UI automatically merge the event timeline with the sample timelines.

R=johnmccutchan@google.com

Committed: https://github.com/dart-lang/sdk/commit/a0e0ac78cbb063582ff2a36dbf79dde90d4c6154

[rmacnak, 2015-12-15 19:15:58]

Description was changed from

==========
Samples trace.


stashing


stashing

BUG=
==========

to

==========
Add service RPC to emit profiler samples in trace event format.

Make timeline UI automatically merge samples with the main timeline.
==========

[rmacnak, 2015-12-15 19:17:47]

Description was changed from

==========
Add service RPC to emit profiler samples in trace event format.

Make timeline UI automatically merge samples with the main timeline.
==========

to

==========
Observatory: Include profiler samples in the timeline view.

 - Add pre-isolate service RPC that emits profiler samples in about:tracing
format.
 - Make timeline UI automatically merge the event timeline with the sample
timelines.
==========

[rmacnak, 2015-12-15 19:17:54]

Description was changed from

==========
Observatory: Include profiler samples in the timeline view.

 - Add pre-isolate service RPC that emits profiler samples in about:tracing
format.
 - Make timeline UI automatically merge the event timeline with the sample
timelines.
==========

to

==========
Observatory: Include profiler samples in the timeline view.

 - Add pre-isolate service RPC that emits profiler samples in about:tracing
format.
 - Make timeline UI automatically merge the event timeline with the sample
timelines.
==========

[rmacnak, 2015-12-15 19:30:13]

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

[Cutch, 2015-12-15 21:54:59]

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler.h
File runtime/vm/profiler.h (right):

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler.h#n...
runtime/vm/profiler.h:567: void set_timeline_trie(ProfileTrieNode* trie) {
timeline_trie_ = trie; }
rewrite to:

ASSERT(timeline_trie_ == NULL);
timeline_trie_ = trie;

so it can only be set once.

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2048: intptr_t id = *next_id;
const intptr_t id = *next_id++;

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2060: frame.AddProperty("category", "Dart");
Do we need to emit a category for each stack frame? This really bloats the JSON
size.

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2083: obj.AddProperty("stackDepth",
Factor this out into a PrintProfileHeaderJson() and share thec ode between
PrintTimelineJSON and PrintProfileJSON.

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2102: event.AddProperty("name", "DartSample");
Can we shorten the name to be "DS" or even drop it entirely?

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2107: event.AddProperty("cat",
VMTag::TagName(sample->vm_tag()));
cat should be hard coded to "Dart"

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.h (right):

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.h:255: void set_frame_id(intptr_t id) { frame_id_ =
id; }
Rewrite to be:

ASSERT(frame_id_ == -1);
frame_id_ = id;

So it can only be set once.

[rmacnak, 2015-12-16 00:14:21]

Also added logic to only handle the trace to about:trace after all the responses
are received.

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler.h
File runtime/vm/profiler.h (right):

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler.h#n...
runtime/vm/profiler.h:567: void set_timeline_trie(ProfileTrieNode* trie) {
timeline_trie_ = trie; }
On 2015/12/15 21:54:58, Cutch wrote:
> rewrite to:
> 
> ASSERT(timeline_trie_ == NULL);
> timeline_trie_ = trie;
> 
> so it can only be set once.

Done.

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.cc (right):

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2048: intptr_t id = *next_id;
On 2015/12/15 21:54:59, Cutch wrote:
> const intptr_t id = *next_id++;

Doesn't mean the same thing. Led to the crashes.

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2060: frame.AddProperty("category", "Dart");
On 2015/12/15 21:54:59, Cutch wrote:
> Do we need to emit a category for each stack frame? This really bloats the
JSON
> size.

The samples all have random looking colors if this is omitted.

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2102: event.AddProperty("name", "DartSample");
On 2015/12/15 21:54:59, Cutch wrote:
> Can we shorten the name to be "DS" or even drop it entirely?

Works when dropped.

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.cc:2107: event.AddProperty("cat",
VMTag::TagName(sample->vm_tag()));
On 2015/12/15 21:54:59, Cutch wrote:
> cat should be hard coded to "Dart"

Done.

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
File runtime/vm/profiler_service.h (right):

https://codereview.chromium.org/1525913002/diff/80001/runtime/vm/profiler_ser...
runtime/vm/profiler_service.h:255: void set_frame_id(intptr_t id) { frame_id_ =
id; }
On 2015/12/15 21:54:59, Cutch wrote:
> Rewrite to be:
> 
> ASSERT(frame_id_ == -1);
> frame_id_ = id;
> 
> So it can only be set once.

Done.

[Cutch, 2015-12-16 14:47:15]

LGTM with one suggestion

https://codereview.chromium.org/1525913002/diff/120001/runtime/observatory/we...
File runtime/observatory/web/timeline.js (right):

https://codereview.chromium.org/1525913002/diff/120001/runtime/observatory/we...
runtime/observatory/web/timeline.js:71: console.log(xhr.statusText);
Should this path also decrement pendingRequests and call updateTimeline when it
hits zero? This way if requesting the profile for an isolate that has been
deleted we will still see the timeline and the profiles for other isolates.

[rmacnak, 2015-12-16 18:09:05]

https://codereview.chromium.org/1525913002/diff/120001/runtime/observatory/we...
File runtime/observatory/web/timeline.js (right):

https://codereview.chromium.org/1525913002/diff/120001/runtime/observatory/we...
runtime/observatory/web/timeline.js:71: console.log(xhr.statusText);
On 2015/12/16 14:47:15, Cutch wrote:
> Should this path also decrement pendingRequests and call updateTimeline when
it
> hits zero? This way if requesting the profile for an isolate that has been
> deleted we will still see the timeline and the profiles for other isolates.

Yes, done.

[rmacnak, 2015-12-16 18:11:47]

Description was changed from

==========
Observatory: Include profiler samples in the timeline view.

 - Add pre-isolate service RPC that emits profiler samples in about:tracing
format.
 - Make timeline UI automatically merge the event timeline with the sample
timelines.
==========

to

==========
Observatory: Include profiler samples in the timeline view.

 - Add pre-isolate service RPC that emits profiler samples in about:tracing
format.
 - Make timeline UI automatically merge the event timeline with the sample
timelines.

R=johnmccutchan@google.com

Committed:
https://github.com/dart-lang/sdk/commit/a0e0ac78cbb063582ff2a36dbf79dde90d4c6154
==========

[rmacnak, 2015-12-16 18:11:48]

Committed patchset #8 (id:140001) manually as
a0e0ac78cbb063582ff2a36dbf79dde90d4c6154 (presubmit successful).