From db8e0e6423cfe842465e8397e6489fcd26deb4f2 Mon Sep 17 00:00:00 2001 From: Daniel Andersson Date: Fri, 28 Aug 2015 10:00:05 -0700 Subject: [PATCH] - Add marking task running on separate thread. - A task has its own MarkingVisitor and SkippedCodeFunctions. - Weak processing still happens on main thread, with tasks paused. - Finalization (detaching code) happens in task. - Cmdline-flag "marker_tasks=0" reverts to old behavior. BUG= Review URL: https://codereview.chromium.org//1309033007 . --- runtime/vm/gc_marker.cc | 167 ++++++++++++++++++++++++++++++++--- runtime/vm/gc_marker.h | 15 +++- runtime/vm/thread.cc | 16 ++-- runtime/vm/thread.h | 9 +- runtime/vm/thread_registry.h | 10 ++- 5 files changed, 188 insertions(+), 29 deletions(-) diff --git a/runtime/vm/gc_marker.cc b/runtime/vm/gc_marker.cc index 4ad7e8f1ddc5..42a08bd6e7c3 100644 --- a/runtime/vm/gc_marker.cc +++ b/runtime/vm/gc_marker.cc @@ -22,6 +22,10 @@ namespace dart { +DEFINE_FLAG(int, marker_tasks, 1, + "The number of tasks to spawn during old gen GC marking (0 means " + "perform all marking on main thread)."); + typedef StoreBufferBlock PointerBlock; // TODO(koda): Rename to PointerBlock. typedef StoreBuffer MarkingStack; // TODO(koda): Create shared base class. @@ -541,6 +545,108 @@ void GCMarker::ProcessObjectIdTable(Isolate* isolate) { } +class MarkTask : public ThreadPool::Task { + public: + MarkTask(GCMarker* marker, + Isolate* isolate, + Heap* heap, + PageSpace* page_space, + MarkingStack* marking_stack, + DelaySet* delay_set, + bool collect_code, + bool visit_prologue_weak_persistent_handles) + : marker_(marker), + isolate_(isolate), + heap_(heap), + page_space_(page_space), + marking_stack_(marking_stack), + delay_set_(delay_set), + collect_code_(collect_code), + visit_prologue_weak_persistent_handles_( + visit_prologue_weak_persistent_handles) { + } + + virtual void Run() { + Thread::EnterIsolateAsHelper(isolate_, true); + { + StackZone stack_zone(Thread::Current()); + Zone* zone = stack_zone.GetZone(); + SkippedCodeFunctions* skipped_code_functions = + collect_code_ ? new(zone) SkippedCodeFunctions() : NULL; + MarkingVisitor visitor(isolate_, heap_, page_space_, marking_stack_, + delay_set_, skipped_code_functions); + // Phase 1: Populate and drain marking stack in task. + // TODO(koda): Split root iteration work among multiple tasks. + marker_->IterateRoots(isolate_, &visitor, + visit_prologue_weak_persistent_handles_); + visitor.DrainMarkingStack(); + marker_->TaskSync(); + // Phase 2: Weak processing and follow-up marking on main thread. + marker_->TaskSync(); + // Phase 3: Finalize results from all markers (detach code, etc.). + marker_->FinalizeResultsFrom(&visitor); + } + Thread::ExitIsolateAsHelper(true); + // This task is done. Notify the original thread. + marker_->TaskNotifyDone(); + } + + private: + GCMarker* marker_; + Isolate* isolate_; + Heap* heap_; + PageSpace* page_space_; + MarkingStack* marking_stack_; + DelaySet* delay_set_; + bool collect_code_; + bool visit_prologue_weak_persistent_handles_; + + DISALLOW_COPY_AND_ASSIGN(MarkTask); +}; + + +void GCMarker::MainSync(intptr_t num_tasks) { + MonitorLocker ml(&monitor_); + while (done_count_ < num_tasks) { + ml.Wait(); + } + done_count_ = 0; // Tasks may now resume. + // TODO(koda): Add barrier utility with two condition variables to allow for + // Notify rather than NotifyAll. Also use it for safepoints. + ml.NotifyAll(); +} + + +void GCMarker::TaskNotifyDone() { + MonitorLocker ml(&monitor_); + ++done_count_; + // TODO(koda): Add barrier utility with two condition variables to allow for + // Notify rather than NotifyAll. Also use it for safepoints. + ml.NotifyAll(); +} + + +void GCMarker::TaskSync() { + MonitorLocker ml(&monitor_); + ++done_count_; + ml.NotifyAll(); // Notify controller that this thread reached end of phase. + ASSERT(done_count_ > 0); + while (done_count_ > 0) { + // Wait for the controller to release into next phase. + ml.Wait(); + } +} + + +void GCMarker::FinalizeResultsFrom(MarkingVisitor* visitor) { + { + MonitorLocker ml(&monitor_); + marked_bytes_ += visitor->marked_bytes(); + } + visitor->Finalize(); +} + + void GCMarker::MarkObjects(Isolate* isolate, PageSpace* page_space, bool invoke_api_callbacks, @@ -553,18 +659,55 @@ void GCMarker::MarkObjects(Isolate* isolate, Zone* zone = stack_zone.GetZone(); MarkingStack marking_stack; DelaySet delay_set; - SkippedCodeFunctions* skipped_code_functions = - collect_code ? new(zone) SkippedCodeFunctions() : NULL; - MarkingVisitor mark(isolate, heap_, page_space, &marking_stack, - &delay_set, skipped_code_functions); - IterateRoots(isolate, &mark, !invoke_api_callbacks); - mark.DrainMarkingStack(); - IterateWeakReferences(isolate, &mark); - MarkingWeakVisitor mark_weak; - IterateWeakRoots(isolate, &mark_weak, invoke_api_callbacks); - // TODO(koda): Add hand-over callback. - marked_bytes_ = mark.marked_bytes(); - mark.Finalize(); + const bool visit_prologue_weak_persistent_handles = !invoke_api_callbacks; + marked_bytes_ = 0; + const int num_tasks = FLAG_marker_tasks; + if (num_tasks == 0) { + // Mark everything on main thread. + SkippedCodeFunctions* skipped_code_functions = + collect_code ? new(zone) SkippedCodeFunctions() : NULL; + MarkingVisitor mark(isolate, heap_, page_space, &marking_stack, + &delay_set, skipped_code_functions); + IterateRoots(isolate, &mark, visit_prologue_weak_persistent_handles); + mark.DrainMarkingStack(); + IterateWeakReferences(isolate, &mark); + MarkingWeakVisitor mark_weak; + IterateWeakRoots(isolate, &mark_weak, + !visit_prologue_weak_persistent_handles); + // All marking done; detach code, etc. + FinalizeResultsFrom(&mark); + } else { + if (num_tasks > 1) { + // TODO(koda): Support multiple: + // 1. non-concurrent tasks, after splitting root iteration work, then + // 2. concurrent tasks, after synchronizing headers. + FATAL("Multiple marking tasks not yet supported"); + } + // Phase 1: Populate and drain marking stack in task. + MarkTask* mark_task = + new MarkTask(this, isolate, heap_, page_space, &marking_stack, + &delay_set, collect_code, + visit_prologue_weak_persistent_handles); + ThreadPool* pool = Dart::thread_pool(); + pool->Run(mark_task); + MainSync(num_tasks); + // Phase 2: Weak processing and follow-up marking on main thread. + SkippedCodeFunctions* skipped_code_functions = + collect_code ? new(zone) SkippedCodeFunctions() : NULL; + MarkingVisitor mark(isolate, heap_, page_space, &marking_stack, + &delay_set, skipped_code_functions); + IterateWeakReferences(isolate, &mark); + MarkingWeakVisitor mark_weak; + IterateWeakRoots(isolate, &mark_weak, + !visit_prologue_weak_persistent_handles); + // TODO(koda): Move this into Phase 3 after making ISL_Print thread-safe + // (used in SkippedCodeFunctions::DetachCode). + FinalizeResultsFrom(&mark); + MainSync(num_tasks); + // Phase 3: Finalize results from all markers (detach code, etc.). + MainSync(num_tasks); + // Finalization complete and all tasks exited. + } delay_set.ClearReferences(); ProcessWeakTables(page_space); ProcessObjectIdTable(isolate); diff --git a/runtime/vm/gc_marker.h b/runtime/vm/gc_marker.h index 4280a65e97a0..87fed84feff0 100644 --- a/runtime/vm/gc_marker.h +++ b/runtime/vm/gc_marker.h @@ -22,7 +22,8 @@ class RawWeakProperty; // of the mark-sweep collection. The marking bit used is defined in RawObject. class GCMarker : public ValueObject { public: - explicit GCMarker(Heap* heap) : heap_(heap), marked_bytes_(0) { } + explicit GCMarker(Heap* heap) + : heap_(heap), marked_bytes_(0), done_count_(0) { } ~GCMarker() { } void MarkObjects(Isolate* isolate, @@ -45,10 +46,22 @@ class GCMarker : public ValueObject { void ProcessWeakTables(PageSpace* page_space); void ProcessObjectIdTable(Isolate* isolate); + // Synchronization between GCMarker's main thread and its marking tasks. + // Called by main thread: wait for 'num_tasks' tasks, then let them resume. + void MainSync(intptr_t num_tasks); + // Called by tasks: notify main thread; wait until it lets us continue. + void TaskSync(); + // Called by tasks: notify main thread, but don't wait (used for exiting). + void TaskNotifyDone(); + // Called by anyone: finalize and accumulate stats from 'visitor'. + void FinalizeResultsFrom(MarkingVisitor* visitor); + Monitor monitor_; // Protects marked_bytes_ and done_count_. Heap* heap_; uintptr_t marked_bytes_; + intptr_t done_count_; + friend class MarkTask; DISALLOW_IMPLICIT_CONSTRUCTORS(GCMarker); }; diff --git a/runtime/vm/thread.cc b/runtime/vm/thread.cc index e023331a7610..a52c1bc5b883 100644 --- a/runtime/vm/thread.cc +++ b/runtime/vm/thread.cc @@ -133,19 +133,19 @@ CACHED_CONSTANTS_LIST(INIT_VALUE) } -void Thread::Schedule(Isolate* isolate) { +void Thread::Schedule(Isolate* isolate, bool bypass_safepoint) { State st; - if (isolate->thread_registry()->RestoreStateTo(this, &st)) { + if (isolate->thread_registry()->RestoreStateTo(this, &st, bypass_safepoint)) { ASSERT(isolate->thread_registry()->Contains(this)); state_ = st; } } -void Thread::Unschedule() { +void Thread::Unschedule(bool bypass_safepoint) { ThreadRegistry* reg = isolate_->thread_registry(); ASSERT(reg->Contains(this)); - reg->SaveStateFrom(this, state_); + reg->SaveStateFrom(this, state_, bypass_safepoint); ClearState(); } @@ -189,7 +189,7 @@ void Thread::ExitIsolate() { } -void Thread::EnterIsolateAsHelper(Isolate* isolate) { +void Thread::EnterIsolateAsHelper(Isolate* isolate, bool bypass_safepoint) { Thread* thread = Thread::Current(); ASSERT(thread != NULL); ASSERT(thread->isolate() == NULL); @@ -205,15 +205,15 @@ void Thread::EnterIsolateAsHelper(Isolate* isolate) { // Do not update isolate->mutator_thread, but perform sanity check: // this thread should not be both the main mutator and helper. ASSERT(!isolate->MutatorThreadIsCurrentThread()); - thread->Schedule(isolate); + thread->Schedule(isolate, bypass_safepoint); } -void Thread::ExitIsolateAsHelper() { +void Thread::ExitIsolateAsHelper(bool bypass_safepoint) { Thread* thread = Thread::Current(); Isolate* isolate = thread->isolate(); ASSERT(isolate != NULL); - thread->Unschedule(); + thread->Unschedule(bypass_safepoint); // TODO(koda): Move store_buffer_block_ into State. thread->StoreBufferRelease(); thread->isolate_ = NULL; diff --git a/runtime/vm/thread.h b/runtime/vm/thread.h index 8f6497ae5415..904d751bc42c 100644 --- a/runtime/vm/thread.h +++ b/runtime/vm/thread.h @@ -83,8 +83,9 @@ class Thread { // "helper" to gain limited concurrent access to the isolate. One example is // SweeperTask (which uses the class table, which is copy-on-write). // TODO(koda): Properly synchronize heap access to expand allowed operations. - static void EnterIsolateAsHelper(Isolate* isolate); - static void ExitIsolateAsHelper(); + static void EnterIsolateAsHelper(Isolate* isolate, + bool bypass_safepoint = false); + static void ExitIsolateAsHelper(bool bypass_safepoint = false); // Called when the current thread transitions from mutator to collector. // Empties the store buffer block into the isolate. @@ -287,8 +288,8 @@ CACHED_CONSTANTS_LIST(DECLARE_MEMBERS) static void SetCurrent(Thread* current); - void Schedule(Isolate* isolate); - void Unschedule(); + void Schedule(Isolate* isolate, bool bypass_safepoint = false); + void Unschedule(bool bypass_safepoint = false); friend class ApiZone; friend class Isolate; diff --git a/runtime/vm/thread_registry.h b/runtime/vm/thread_registry.h index cd94c3f9b4f8..cb106269c2a5 100644 --- a/runtime/vm/thread_registry.h +++ b/runtime/vm/thread_registry.h @@ -48,10 +48,11 @@ class ThreadRegistry { CheckSafepointLocked(); } - bool RestoreStateTo(Thread* thread, Thread::State* state) { + bool RestoreStateTo(Thread* thread, Thread::State* state, + bool bypass_safepoint) { MonitorLocker ml(monitor_); // Wait for any rendezvous in progress. - while (in_rendezvous_) { + while (!bypass_safepoint && in_rendezvous_) { ml.Wait(Monitor::kNoTimeout); } Entry* entry = FindEntry(thread); @@ -83,14 +84,15 @@ class ThreadRegistry { return false; } - void SaveStateFrom(Thread* thread, const Thread::State& state) { + void SaveStateFrom(Thread* thread, const Thread::State& state, + bool bypass_safepoint) { MonitorLocker ml(monitor_); Entry* entry = FindEntry(thread); ASSERT(entry != NULL); ASSERT(entry->scheduled); entry->scheduled = false; entry->state = state; - if (in_rendezvous_) { + if (!bypass_safepoint && in_rendezvous_) { // Don't wait for this thread. ASSERT(remaining_ > 0); if (--remaining_ == 0) {