Stop holding onto TimelineEvents

Stop holding onto TimelineEvents

Due to the need to reclaim TimelineEventBlocks from the system, it is not safe to hold onto a TimelineEvent other than briefly to fill it out.

- Switch TimelineDurationScope to emit two events (begin, end) instead of one duration event.
- When setting duration arguments, hold them in the TimelineDurationScope and then emit them with the end event.

The above two changes allows TimelineDurationScope to work without holding onto a TimelineEvent.

To stop the block from being reclaimed while we are filling out an event, calls to StartEvent return with a lock held that is unlocked when CompleteEvent is called.

- Update tests.

R=turnidge@google.com

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

[Cutch, 2015-10-07 23:53:01]

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

[turnidge, 2015-10-08 17:59:19]

lgtm w/ 1 idea

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

https://codereview.chromium.org/1395803002/diff/20001/runtime/vm/timeline.cc#...
runtime/vm/timeline.cc:335: }
Perhaps you could steal the whole arguments array and skip the loop?

arguments_length_ = arguments_length;
arguments_ = arguments;

https://codereview.chromium.org/1395803002/diff/20001/runtime/vm/timeline.cc#...
runtime/vm/timeline.cc:529: event->StealArguments(arguments_length_,
arguments_);
If StealArguments steals the whole array, follow this by:

arguments_length_ = 0;
arguments_ = NULL;

and remove the call to FreeArguments.

https://codereview.chromium.org/1395803002/diff/20001/runtime/vm/timeline.cc#...
runtime/vm/timeline.cc:541: arguments_length_ = length;
You could choose to guard the allocation of arguments_ here...

if (arguments_length_ > 0) {
  calloc
}

[Cutch, 2015-10-08 20:08:11]

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

https://codereview.chromium.org/1395803002/diff/20001/runtime/vm/timeline.cc#...
runtime/vm/timeline.cc:335: }
On 2015/10/08 17:59:19, turnidge wrote:
> Perhaps you could steal the whole arguments array and skip the loop?
> 
> arguments_length_ = arguments_length;
> arguments_ = arguments;

Done.

https://codereview.chromium.org/1395803002/diff/20001/runtime/vm/timeline.cc#...
runtime/vm/timeline.cc:529: event->StealArguments(arguments_length_,
arguments_);
On 2015/10/08 17:59:19, turnidge wrote:
> If StealArguments steals the whole array, follow this by:
> 
> arguments_length_ = 0;
> arguments_ = NULL;
> 
> and remove the call to FreeArguments.

Done.

https://codereview.chromium.org/1395803002/diff/20001/runtime/vm/timeline.cc#...
runtime/vm/timeline.cc:541: arguments_length_ = length;
On 2015/10/08 17:59:19, turnidge wrote:
> You could choose to guard the allocation of arguments_ here...
> 
> if (arguments_length_ > 0) {
>   calloc
> }

Done.

[Cutch, 2015-10-08 21:56:29]

Committed patchset #3 (id:40001) manually as
7137f9ef061c5b55a75e27b0018a2955c352bbd4 (presubmit successful).