Make TimelineEventBlocks reclaimable

Make TimelineEventBlocks reclaimable

- Add Timeline::ReclaimAllBlocks and Timeline::ReclaimIsolateBlocks utility functions to ensure that all cached blocks are closed before reporting the timeline
- Adjust Timeline_Dart_GlobalTimelineGetTrace to exercise block reclaiming
- Add ability to reclaim blocks from threads entered into an isolate
- Fix IsolateTimelineEventFilter to actually filter out open blocks
- Make some existing tests call Timeline::ReclaimIsolateBlocks before reporting
- Added implementation and locking notes to timeline.cc

R=turnidge@google.com

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

[Cutch, 2015-09-23 20:00:17]

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

[turnidge, 2015-09-24 17:05:53]

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

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/thread.cc#newcod...
runtime/vm/thread.cc:346: void Thread::CloseTimelineBlock() {
Close -> Reclaim?  (Note from offfline discussion: make sure this is still
used).

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

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/thread_registry....
runtime/vm/thread_registry.cc:73: MutexLocker recorder_lock(&recorder->lock_);
Remove this lock?

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/thread_registry....
runtime/vm/thread_registry.cc:108: //
Odd empty comment here.

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

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/timeline.cc#newc...
runtime/vm/timeline.cc:141: }
This check is redundant (also in callee).

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/timeline.cc#newc...
runtime/vm/timeline.cc:176: }
Consider an ASSERT for isolate != NULL?

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

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/timeline.h#newco...
runtime/vm/timeline.h:474: return !block->IsEmpty() && !block->open() &&
Consider in_use instead of open?

[Cutch, 2015-09-24 18:14:16]

PTAL

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

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/thread.cc#newcod...
runtime/vm/thread.cc:346: void Thread::CloseTimelineBlock() {
On 2015/09/24 17:05:52, turnidge wrote:
> Close -> Reclaim?  (Note from offfline discussion: make sure this is still
> used).

This was unused and is now removed.

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

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/thread_registry....
runtime/vm/thread_registry.cc:73: MutexLocker recorder_lock(&recorder->lock_);
On 2015/09/24 17:05:52, turnidge wrote:
> Remove this lock?

Done.

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/thread_registry....
runtime/vm/thread_registry.cc:108: //
On 2015/09/24 17:05:52, turnidge wrote:
> Odd empty comment here.

Done.

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

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/timeline.cc#newc...
runtime/vm/timeline.cc:141: }
On 2015/09/24 17:05:52, turnidge wrote:
> This check is redundant (also in callee).

Done.

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/timeline.cc#newc...
runtime/vm/timeline.cc:176: }
On 2015/09/24 17:05:52, turnidge wrote:
> Consider an ASSERT for isolate != NULL?

Done.

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

https://codereview.chromium.org/1363033003/diff/1/runtime/vm/timeline.h#newco...
runtime/vm/timeline.h:474: return !block->IsEmpty() && !block->open() &&
On 2015/09/24 17:05:52, turnidge wrote:
> Consider in_use instead of open?

Done.

[turnidge, 2015-09-24 18:30:34]

lgtm

[Cutch, 2015-09-24 18:32:25]

Committed patchset #4 (id:60001) manually as
db3421ecfcd03015d644a33bd4f8627d1d73e19e (presubmit successful).