Skip to content

Commit

Permalink
Simplify timeline backend
Browse files Browse the repository at this point in the history
Now that we have a central list of all threads that are interacting with the VM we can stop relying on the Isolate's thread registry to cache timeline blocks.

- Move from caching blocks in the Isolate's thread registry and caching them in the Thread directly.
- This allows cached blocks to have events from multiple isolates, increasing usage density and reducing the number of blocks being cached at any given time.
- We no longer need a distinct global block, this allows me to remove a bunch of dead code.

R=turnidge@google.com

Review URL: https://codereview.chromium.org/1402383003 .
  • Loading branch information
johnmccutchan committed Oct 16, 2015
1 parent a08d80b commit da946fb
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 240 deletions.
6 changes: 2 additions & 4 deletions runtime/vm/dart_api_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5887,8 +5887,7 @@ DART_EXPORT bool Dart_TimelineGetTrace(Dart_StreamConsumer consumer,
}
Thread* T = Thread::Current();
StackZone zone(T);
// Reclaim all blocks cached by isolate.
Timeline::ReclaimIsolateBlocks();
Timeline::ReclaimCachedBlocksFromThreads();
JSONStream js;
IsolateTimelineEventFilter filter(isolate);
timeline_recorder->PrintTraceEvent(&js, &filter);
Expand All @@ -5912,8 +5911,7 @@ DART_EXPORT bool Dart_GlobalTimelineGetTrace(Dart_StreamConsumer consumer,
}
Thread* T = Thread::Current();
StackZone zone(T);
// Reclaim all blocks cached in the system.
Timeline::ReclaimAllBlocks();
Timeline::ReclaimCachedBlocksFromThreads();
JSONStream js;
TimelineEventFilter filter;
timeline_recorder->PrintTraceEvent(&js, &filter);
Expand Down
8 changes: 4 additions & 4 deletions runtime/vm/dart_api_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9426,7 +9426,7 @@ TEST_CASE(Timeline_Dart_TimelineDuration) {
Dart_TimelineDuration("testDurationEvent", 0, 1);
// Check that it is in the output.
TimelineEventRecorder* recorder = Timeline::recorder();
Timeline::ReclaimIsolateBlocks();
Timeline::ReclaimCachedBlocksFromThreads();
JSONStream js;
IsolateTimelineEventFilter filter(isolate);
recorder->PrintJSON(&js, &filter);
Expand All @@ -9443,7 +9443,7 @@ TEST_CASE(Timeline_Dart_TimelineInstant) {
Dart_TimelineInstant("testInstantEvent");
// Check that it is in the output.
TimelineEventRecorder* recorder = Timeline::recorder();
Timeline::ReclaimIsolateBlocks();
Timeline::ReclaimCachedBlocksFromThreads();
JSONStream js;
IsolateTimelineEventFilter filter(isolate);
recorder->PrintJSON(&js, &filter);
Expand All @@ -9465,7 +9465,7 @@ TEST_CASE(Timeline_Dart_TimelineAsyncDisabled) {
Dart_TimelineAsyncEnd("testAsyncEvent", async_id);
// Check that testAsync is not in the output.
TimelineEventRecorder* recorder = Timeline::recorder();
Timeline::ReclaimIsolateBlocks();
Timeline::ReclaimCachedBlocksFromThreads();
JSONStream js;
TimelineEventFilter filter;
recorder->PrintJSON(&js, &filter);
Expand All @@ -9488,7 +9488,7 @@ TEST_CASE(Timeline_Dart_TimelineAsync) {

// Check that it is in the output.
TimelineEventRecorder* recorder = Timeline::recorder();
Timeline::ReclaimIsolateBlocks();
Timeline::ReclaimCachedBlocksFromThreads();
JSONStream js;
IsolateTimelineEventFilter filter(isolate);
recorder->PrintJSON(&js, &filter);
Expand Down
11 changes: 1 addition & 10 deletions runtime/vm/isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1616,7 +1616,7 @@ void Isolate::LowLevelShutdown() {
set_message_handler(NULL);

// Before analyzing the isolate's timeline blocks- reclaim all cached blocks.
ReclaimTimelineBlocks();
Timeline::ReclaimCachedBlocksFromThreads();

// Dump all timing data for the isolate.
if (FLAG_timing) {
Expand Down Expand Up @@ -1718,15 +1718,6 @@ void Isolate::Shutdown() {
}


void Isolate::ReclaimTimelineBlocks() {
TimelineEventRecorder* recorder = Timeline::recorder();
if (recorder == NULL) {
return;
}
thread_registry_->ReclaimTimelineBlocks();
}


Dart_IsolateCreateCallback Isolate::create_callback_ = NULL;
Dart_IsolateInterruptCallback Isolate::interrupt_callback_ = NULL;
Dart_IsolateUnhandledExceptionCallback
Expand Down
1 change: 0 additions & 1 deletion runtime/vm/isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,6 @@ class Isolate : public BaseIsolate {

void LowLevelShutdown();
void Shutdown();
void ReclaimTimelineBlocks();

void BuildName(const char* name_prefix);
void PrintInvokedFunctions();
Expand Down
1 change: 1 addition & 0 deletions runtime/vm/thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ Thread::Thread(bool init_vm_constants)
thread_interrupt_data_(NULL),
isolate_(NULL),
heap_(NULL),
timeline_block_(NULL),
store_buffer_block_(NULL),
log_(new class Log()),
deopt_id_(0),
Expand Down
6 changes: 3 additions & 3 deletions runtime/vm/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ class Thread {
Zone* zone;
uword top_exit_frame_info;
StackResource* top_resource;
TimelineEventBlock* timeline_block;
LongJumpScope* long_jump_base;
#if defined(DEBUG)
HandleScope* top_handle_scope;
Expand Down Expand Up @@ -312,12 +311,12 @@ LEAF_RUNTIME_ENTRY_LIST(DEFINE_OFFSET_METHOD)

// Only safe to access when holding |timeline_block_lock_|.
TimelineEventBlock* timeline_block() const {
return state_.timeline_block;
return timeline_block_;
}

// Only safe to access when holding |timeline_block_lock_|.
void set_timeline_block(TimelineEventBlock* block) {
state_.timeline_block = block;
timeline_block_ = block;
}

class Log* log() const;
Expand Down Expand Up @@ -419,6 +418,7 @@ LEAF_RUNTIME_ENTRY_LIST(DEFINE_OFFSET_METHOD)
Heap* heap_;
State state_;
Mutex timeline_block_lock_;
TimelineEventBlock* timeline_block_;
StoreBufferBlock* store_buffer_block_;
class Log* log_;
intptr_t deopt_id_; // Compilation specific counter.
Expand Down
50 changes: 1 addition & 49 deletions runtime/vm/thread_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
namespace dart {

ThreadRegistry::~ThreadRegistry() {
ReclaimTimelineBlocks();
// Delete monitor.
delete monitor_;
}
Expand Down Expand Up @@ -67,54 +66,7 @@ void ThreadRegistry::PruneThread(Thread* thread) {
if (found_index < 0) {
return;
}
{
TimelineEventRecorder* recorder = Timeline::recorder();
if (recorder != NULL) {
// Cleanup entry.
Entry& entry_to_remove = entries_[found_index];
ReclaimTimelineBlockLocked(&entry_to_remove);
}
}
if (found_index != (length - 1)) {
// Swap with last entry.
entries_.Swap(found_index, length - 1);
}
entries_.RemoveLast();
}


void ThreadRegistry::ReclaimTimelineBlocks() {
// Each thread that is scheduled in this isolate may have a cached timeline
// block. Mark these timeline blocks as finished.
MonitorLocker ml(monitor_);
TimelineEventRecorder* recorder = Timeline::recorder();
if (recorder != NULL) {
for (intptr_t i = 0; i < entries_.length(); i++) {
// NOTE: It is only safe to access |entry.state| here.
Entry& entry = entries_[i];
ReclaimTimelineBlockLocked(&entry);
}
}
}


void ThreadRegistry::ReclaimTimelineBlockLocked(Entry* entry) {
if (entry == NULL) {
return;
}
TimelineEventRecorder* recorder = Timeline::recorder();
if (!entry->scheduled && (entry->state.timeline_block != NULL)) {
// Currently unscheduled thread.
recorder->FinishBlock(entry->state.timeline_block);
entry->state.timeline_block = NULL;
} else if (entry->scheduled) {
// Currently scheduled thread.
Thread* thread = entry->thread;
// Take |Thread| lock.
MutexLocker thread_lock(thread->timeline_block_lock());
recorder->FinishBlock(thread->timeline_block());
thread->set_timeline_block(NULL);
}
entries_.RemoveAt(found_index);
}


Expand Down
3 changes: 0 additions & 3 deletions runtime/vm/thread_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,6 @@ class ThreadRegistry {
return NULL;
}

// NOTE: Lock should be taken before this function is called.
void ReclaimTimelineBlockLocked(Entry* entry);

// Note: Lock should be taken before this function is called.
void CheckSafepointLocked();

Expand Down
Loading

0 comments on commit da946fb

Please sign in to comment.