Initial Timeline Events

Initial Timeline Events

- Each isolate has its own TimelineEventBuffer
- Each isolate has TimelineEventStreams for API, Compiler, Embedder, Isolate, and GC
- Compiler, Isolate, Embedder, and GC are being inserted into the stream
- Basic unit tests
- Dart API for injecting timeline events
- Arbitrary number of native and Dart arguments can be attached to each event
- JSON printing
- Output can be loaded by about://tracing
- Add --timeline-trace-dir flag

Short term follow up CLS:
- Service protocol requests to enable / disable tracing and retrieve a trace
- UI for Observatory
- Dart code EventStreams and Events

R=asiva@google.com

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

[Cutch, 2015-06-05 20:17:22]

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

[Cutch, 2015-06-05 20:18:43]

PTAL

https://codereview.chromium.org/1170503004/diff/20001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

https://codereview.chromium.org/1170503004/diff/20001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:1407: if (timeline_event_buffer_ != NULL) {
Temporary

[rmacnak, 2015-06-05 21:22:58]

https://codereview.chromium.org/1170503004/diff/20001/runtime/vm/timeline.cc
File runtime/vm/timeline.cc (right):

https://codereview.chromium.org/1170503004/diff/20001/runtime/vm/timeline.cc#...
runtime/vm/timeline.cc:91: obj.AddProperty("ph", "X");
ph?

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:1074: tds.SetArgument("function",
function.PrettyNameCString());
Is this string in the right zone? Doesn't the current zone go away when
compilation finishes?

Probably should be a qualified name so we have a chance of identifying the
function later.

[Cutch, 2015-06-05 21:34:43]

https://codereview.chromium.org/1170503004/diff/20001/runtime/vm/timeline.cc
File runtime/vm/timeline.cc (right):

https://codereview.chromium.org/1170503004/diff/20001/runtime/vm/timeline.cc#...
runtime/vm/timeline.cc:91: obj.AddProperty("ph", "X");
On 2015/06/05 21:22:58, rmacnak wrote:
> ph?

https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAy...

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:1074: tds.SetArgument("function",
function.PrettyNameCString());
On 2015/06/05 21:22:58, rmacnak wrote:
> Is this string in the right zone? Doesn't the current zone go away when
> compilation finishes?
> 

The string is copied when stored in the Event.

> Probably should be a qualified name so we have a chance of identifying the
> function later.

Agreed.

[rmacnak, 2015-06-08 22:11:30]

If I patch this in, I see

runtime/vm/virtual_memory_linux.cc:50: error: munmap failed

[New Thread 0xf72f0b40 (LWP 27519)]

Program received signal SIGABRT, Aborted.
0xf7fdb430 in __kernel_vsyscall ()
(gdb) bt
#0  0xf7fdb430 in __kernel_vsyscall ()
#1  0xf7cbf607 in raise () from /lib/i386-linux-gnu/libc.so.6
#2  0xf7cc2a33 in abort () from /lib/i386-linux-gnu/libc.so.6
#3  0x082db2d5 in dart::DynamicAssertionHelper::Fail (
    this=this@entry=0xffffb564, format=format@entry=0xaa95c53 "%s")
    at runtime/platform/assert.cc:41
#4  0x08621556 in dart::unmap (size=<optimized out>, address=<optimized out>)
    at runtime/vm/virtual_memory_linux.cc:50
#5  0x0853f198 in dart::HeapPage::Deallocate (this=0xf23b1000)
    at runtime/vm/pages.cc:81
#6  0x0853f7a5 in FreePages (pages=<optimized out>, this=0xacb0850)
    at runtime/vm/pages.cc:309
#7  dart::PageSpace::~PageSpace (this=0xacb0850, __in_chrg=<optimized out>)
    at runtime/vm/pages.cc:177
#8  0x084345a2 in dart::Heap::~Heap (this=0xacbefc0, __in_chrg=<optimized out>)
    at runtime/vm/heap.cc:70
#9  0x084a1ea4 in dart::Isolate::~Isolate (this=0xacc25c8, 
    __in_chrg=<optimized out>) at runtime/vm/isolate.cc:691

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/isolate.cc#n...
runtime/vm/isolate.cc:1407: if (timeline_event_buffer_ != NULL) {
Behind a flag.

[siva, 2015-06-09 22:37:57]

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:1122: "CompileOptimizedFunction");
VMTagScope and TimelineDurationScope seem to always show up together, I wonder
if it makes sense to have a convenience macro.

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/isolate.h
File runtime/vm/isolate.h (right):

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/isolate.h#ne...
runtime/vm/isolate.h:532: TimelineEventBuffer* timeline_event_buffer() {
_buffer() const {

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/timeline.h
File runtime/vm/timeline.h (right):

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/timeline.h#n...
runtime/vm/timeline.h:45: void PrintJSON(JSONStream* stream);
const function

[Cutch, 2015-06-10 18:20:57]

PTAL

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:1122: "CompileOptimizedFunction");
On 2015/06/09 22:37:56, siva wrote:
> VMTagScope and TimelineDurationScope seem to always show up together, I wonder
> if it makes sense to have a convenience macro.

Acknowledged.

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/isolate.h
File runtime/vm/isolate.h (right):

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/isolate.h#ne...
runtime/vm/isolate.h:532: TimelineEventBuffer* timeline_event_buffer() {
On 2015/06/09 22:37:56, siva wrote:
> _buffer() const {

Done.

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/timeline.h
File runtime/vm/timeline.h (right):

https://codereview.chromium.org/1170503004/diff/40001/runtime/vm/timeline.h#n...
runtime/vm/timeline.h:45: void PrintJSON(JSONStream* stream);
On 2015/06/09 22:37:56, siva wrote:
> const function

Done.

[rmacnak, 2015-06-10 20:25:15]

https://codereview.chromium.org/1170503004/diff/100001/runtime/include/dart_a...
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/1170503004/diff/100001/runtime/include/dart_a...
runtime/include/dart_api.h:2965: * NOTE: On Posix platforms this call uses
gettimeofday andww on Windows platforms
andww -> and

https://codereview.chromium.org/1170503004/diff/100001/runtime/vm/deopt_instr...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/1170503004/diff/100001/runtime/vm/deopt_instr...
runtime/vm/deopt_instructions.cc:109:
const_cast<char*>(function.QualifiedUserVisibleNameCString()));
The whole leaf runtime entry is in a NoSafepointScope, so either use name()
instead or QualifiedUserVisibleNameCString needs to concatenate C strings
instead of Dart strings.

https://codereview.chromium.org/1170503004/diff/100001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

https://codereview.chromium.org/1170503004/diff/100001/runtime/vm/isolate.cc#...
runtime/vm/isolate.cc:1473: if (timeline_event_buffer_ != NULL) {
Behind a flag

[Cutch, 2015-06-10 21:20:58]

https://codereview.chromium.org/1170503004/diff/100001/runtime/include/dart_a...
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/1170503004/diff/100001/runtime/include/dart_a...
runtime/include/dart_api.h:2965: * NOTE: On Posix platforms this call uses
gettimeofday andww on Windows platforms
On 2015/06/10 20:25:14, rmacnak wrote:
> andww -> and

Done.

https://codereview.chromium.org/1170503004/diff/100001/runtime/vm/deopt_instr...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/1170503004/diff/100001/runtime/vm/deopt_instr...
runtime/vm/deopt_instructions.cc:109:
const_cast<char*>(function.QualifiedUserVisibleNameCString()));
On 2015/06/10 20:25:14, rmacnak wrote:
> The whole leaf runtime entry is in a NoSafepointScope, so either use name()
> instead or QualifiedUserVisibleNameCString needs to concatenate C strings
> instead of Dart strings. 

Done.

https://codereview.chromium.org/1170503004/diff/100001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

https://codereview.chromium.org/1170503004/diff/100001/runtime/vm/isolate.cc#...
runtime/vm/isolate.cc:1473: if (timeline_event_buffer_ != NULL) {
On 2015/06/10 20:25:14, rmacnak wrote:
> Behind a flag

This will be gone before I commit.

[siva, 2015-06-12 19:50:35]

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2918: * this call uses GetSystemTimeAsFileTime to get
the current time.
This call doesn't seem to use any time value, not sure what this comment means.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2935: * this call uses GetSystemTimeAsFileTime to get
the current time.
Ditto comment about this comment.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2942: * current isolate.
The comment is wrong.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2950: * this call uses GetSystemTimeAsFileTime to get
the current time.
Ditto.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2953: int64_t async_id);
Why does one have to keep passing the label in every time, isn't the id
associated with the label when calling Dart_TimelineAsyncBegin

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2958: * current isolate.
The comment is wrong

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2966: * this call uses GetSystemTimeAsFileTime to get
the current time.
Ditto.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2969: 
I am wondering if it makes sense to add this API functions in
dart_debugger_api.h as these are tools related functions, I guess the name
'dart_debugger_api.h' should have been 'dart_tools_api.h' or something like
that.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/compiler.cc...
runtime/vm/compiler.cc:1078: }
I think you should really switch this code to a macro to make it more readable,
currently it is a bit jarring to see this here like this.

maybe:

TIMELINE_FUNCTION_COMPILATION_DURATION(isolate, "Function", function);

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/compiler.cc...
runtime/vm/compiler.cc:1134: }
TIMELINE_FUNCTION_COMPILATION_DURATION(isolate, "OptimizedFunction", function);

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5621: CHECK_ISOLATE(isolate);
Normally we do a check of all arguments in the API call implementations  and
report errors (as they are coming from external code).

e.g:
- NULL check for label
- end_micros being > start_micros

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5633: CHECK_ISOLATE(isolate);
Ditto comment about checking label parameter.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5645: CHECK_ISOLATE(isolate);
Ditto.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5660: }
Also doesn't async_id have to be a value that you was handed out above, is there
a way to verify that?

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
File runtime/vm/dart_api_impl_test.cc (right):

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl_test.cc:9198: Dart_TimelineAsyncEnd("testAsyncEvent",
async_id);
Not sure what purpose this test achieves, it seems to have no return value to
check.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl_test.cc:9211: // Call Dart_TimelineAsyncEnd with a
negative async_id.
incorrect comment.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/deopt_instr...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/deopt_instr...
runtime/vm/deopt_instructions.cc:144: const Code& code = Code::Handle(code_);
Handle(zone(), code_);

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/deopt_instr...
runtime/vm/deopt_instructions.cc:145: const Function& function =
Function::Handle(code.function());
Ditto.

[Cutch, 2015-06-12 23:18:56]

PTAL

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2918: * this call uses GetSystemTimeAsFileTime to get
the current time.
On 2015/06/12 19:50:34, siva wrote:
> This call doesn't seem to use any time value, not sure what this comment
means.

All of these calls internally sample a clock. The comment is there so that
developers know which clock is used internally. That is, any time values passed
into the system (e.g. Dart_TimelineDuration) need to use the same clock.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2935: * this call uses GetSystemTimeAsFileTime to get
the current time.
On 2015/06/12 19:50:34, siva wrote:
> Ditto comment about this comment.

Acknowledged.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2942: * current isolate.
On 2015/06/12 19:50:34, siva wrote:
> The comment is wrong.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2950: * this call uses GetSystemTimeAsFileTime to get
the current time.
On 2015/06/12 19:50:34, siva wrote:
> Ditto.

Acknowledged.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2953: int64_t async_id);
On 2015/06/12 19:50:34, siva wrote:
> Why does one have to keep passing the label in every time, isn't the id
> associated with the label when calling Dart_TimelineAsyncBegin

Currently, no. I'm trying to avoid tracking open async_ids to avoid internal
overhead.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2958: * current isolate.
On 2015/06/12 19:50:34, siva wrote:
> The comment is wrong

Done.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2966: * this call uses GetSystemTimeAsFileTime to get
the current time.
On 2015/06/12 19:50:34, siva wrote:
> Ditto.

Acknowledged.

https://codereview.chromium.org/1170503004/diff/170001/runtime/include/dart_a...
runtime/include/dart_api.h:2969: 
On 2015/06/12 19:50:34, siva wrote:
> I am wondering if it makes sense to add this API functions in
> dart_debugger_api.h as these are tools related functions, I guess the name
> 'dart_debugger_api.h' should have been 'dart_tools_api.h' or something like
> that.

Acknowledged.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/compiler.cc...
runtime/vm/compiler.cc:1078: }
On 2015/06/12 19:50:34, siva wrote:
> I think you should really switch this code to a macro to make it more
readable,
> currently it is a bit jarring to see this here like this.
> 
> maybe:
> 
> TIMELINE_FUNCTION_COMPILATION_DURATION(isolate, "Function", function);

Done.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/compiler.cc...
runtime/vm/compiler.cc:1134: }
On 2015/06/12 19:50:34, siva wrote:
> TIMELINE_FUNCTION_COMPILATION_DURATION(isolate, "OptimizedFunction",
function);

Done.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5621: CHECK_ISOLATE(isolate);
On 2015/06/12 19:50:34, siva wrote:
> Normally we do a check of all arguments in the API call implementations  and
> report errors (as they are coming from external code).
> 
> e.g:
> - NULL check for label
> - end_micros being > start_micros

Done.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5633: CHECK_ISOLATE(isolate);
On 2015/06/12 19:50:34, siva wrote:
> Ditto comment about checking label parameter.

Done.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5645: CHECK_ISOLATE(isolate);
On 2015/06/12 19:50:34, siva wrote:
> Ditto.

Done.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5660: }
On 2015/06/12 19:50:34, siva wrote:
> Also doesn't async_id have to be a value that you was handed out above, is
there
> a way to verify that?

Not right now. I can start tracking open async ids internally but I would prefer
to avoid the overhead.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
File runtime/vm/dart_api_impl_test.cc (right):

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl_test.cc:9198: Dart_TimelineAsyncEnd("testAsyncEvent",
async_id);
On 2015/06/12 19:50:35, siva wrote:
> Not sure what purpose this test achieves, it seems to have no return value to
> check.

Done.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl_test.cc:9211: // Call Dart_TimelineAsyncEnd with a
negative async_id.
On 2015/06/12 19:50:35, siva wrote:
> incorrect comment.

Done.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/deopt_instr...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/deopt_instr...
runtime/vm/deopt_instructions.cc:144: const Code& code = Code::Handle(code_);
On 2015/06/12 19:50:35, siva wrote:
> Handle(zone(), code_);

Done.

https://codereview.chromium.org/1170503004/diff/170001/runtime/vm/deopt_instr...
runtime/vm/deopt_instructions.cc:145: const Function& function =
Function::Handle(code.function());
On 2015/06/12 19:50:35, siva wrote:
> Ditto.

Done.

[siva, 2015-06-16 15:42:32]

lgtm

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/dart_api_im...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5718: }
Instead of FATALing out here maybe you could change the return type of this
function and use RETURN_NULL_ERROR

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5738: }
Ditto.

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5751: }
Ditto.

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5756: }
Ditto.

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/deopt_instr...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/deopt_instr...
runtime/vm/deopt_instructions.cc:101: TimelineStream* compiler_stream =
isolate->GetCompilerStream();
does compiler_stream need a non NULL ASSERT?

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/timeline.h
File runtime/vm/timeline.h (right):

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/timeline.h#...
runtime/vm/timeline.h:23: };
Since this is only used as a type of a private field can this declaration also
be moved down to the private section.

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/timeline_te...
File runtime/vm/timeline_test.cc (right):

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/timeline_te...
runtime/vm/timeline_test.cc:2: // for details. All rights reserved. Use of this
source code is governed by a
2015

[Cutch, 2015-06-16 17:08:18]

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/dart_api_im...
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5718: }
On 2015/06/16 15:42:31, siva wrote:
> Instead of FATALing out here maybe you could change the return type of this
> function and use RETURN_NULL_ERROR

Done.

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5738: }
On 2015/06/16 15:42:31, siva wrote:
> Ditto.

Done.

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5751: }
On 2015/06/16 15:42:31, siva wrote:
> Ditto.

Done.

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/dart_api_im...
runtime/vm/dart_api_impl.cc:5756: }
On 2015/06/16 15:42:31, siva wrote:
> Ditto.

Done.

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/deopt_instr...
File runtime/vm/deopt_instructions.cc (right):

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/deopt_instr...
runtime/vm/deopt_instructions.cc:101: TimelineStream* compiler_stream =
isolate->GetCompilerStream();
On 2015/06/16 15:42:31, siva wrote:
> does compiler_stream need a non NULL ASSERT?

Done.

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/timeline.h
File runtime/vm/timeline.h (right):

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/timeline.h#...
runtime/vm/timeline.h:23: };
On 2015/06/16 15:42:31, siva wrote:
> Since this is only used as a type of a private field can this declaration also
> be moved down to the private section.

Done.

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/timeline_te...
File runtime/vm/timeline_test.cc (right):

https://codereview.chromium.org/1170503004/diff/190001/runtime/vm/timeline_te...
runtime/vm/timeline_test.cc:2: // for details. All rights reserved. Use of this
source code is governed by a
On 2015/06/16 15:42:31, siva wrote:
> 2015

Done.

[Cutch, 2015-06-16 17:10:20]

Committed patchset #13 (id:230001) manually as
c7187924a95238027624d2441f17c38ae49395f4 (presubmit successful).