Old-gen marking on separate thread

runtime/vm/gc_marker.cc

Index: runtime/vm/gc_marker.cc
diff --git a/runtime/vm/gc_marker.cc b/runtime/vm/gc_marker.cc
index 4ad7e8f1ddc586ac1859384b955fbc710a37f005..426121dd27e3b12b2d788f3debf52cdee841a5b1 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.
 
@@ -421,7 +425,7 @@ void GCMarker::IterateRoots(Isolate* isolate,
   isolate->VisitObjectPointers(visitor,
                                visit_prologue_weak_persistent_handles,
                                StackFrameIterator::kDontValidateFrames);
-  heap_->new_space()->VisitObjectPointers(visitor);
+  isolate->heap()->new_space()->VisitObjectPointers(visitor);

[Ivan Posva, 2015/08/28 05:23:22]

The GCMarker still has a heap_ reference, why has this changed?

[koda, 2015/08/28 13:27:53]

Because I made this method static, since it doesn't really use any state of
GCMarker (and if it will, we need to ensure that access is thread-safe).

 }
 
 
@@ -541,6 +545,102 @@ 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 invoke_api_callbacks)
+      : marker_(marker),
+        isolate_(isolate),
+        heap_(heap),
+        page_space_(page_space),
+        marking_stack_(marking_stack),
+        delay_set_(delay_set),
+        collect_code_(collect_code),
+        invoke_api_callbacks_(invoke_api_callbacks) {
+  }
+
+  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);
+      // TODO(koda): Split root iteration work among multiple tasks.
+      GCMarker::IterateRoots(isolate_, &visitor, !invoke_api_callbacks_);
+      visitor.DrainMarkingStack();
+      marker_->TaskSync();
+      // Wait for weak processing on main thread...
+      marker_->TaskSync();
+      // All marking done; 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 invoke_api_callbacks_;
+
+  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() {
+  TaskNotifyDone();
+  MonitorLocker ml(&monitor_);
+  while (done_count_ > 0) {
+    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 +653,51 @@ 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();
+    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, !invoke_api_callbacks);
+      mark.DrainMarkingStack();
+      IterateWeakReferences(isolate, &mark);
+      MarkingWeakVisitor mark_weak;
+      IterateWeakRoots(isolate, &mark_weak, invoke_api_callbacks);
+      // 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");
+      }
+      // Spawn a marking task on a separate thread.
+      MarkTask* mark_task =
+          new MarkTask(this, isolate, heap_, page_space, &marking_stack,
+                       &delay_set, collect_code, invoke_api_callbacks);
+      ThreadPool* pool = Dart::thread_pool();
+      pool->Run(mark_task);
+      MainSync(num_tasks);
+      {
+        // Perform weak processing 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, invoke_api_callbacks);
+        // All marking done; detach code, etc.
+        FinalizeResultsFrom(&mark);
+      }
+      MainSync(num_tasks);
+      // Wait for mark task finalization...
+      MainSync(num_tasks);
+    }
     delay_set.ClearReferences();
     ProcessWeakTables(page_space);
     ProcessObjectIdTable(isolate);