Background finalization.

runtime/vm/scavenger.cc

Index: runtime/vm/scavenger.cc
diff --git a/runtime/vm/scavenger.cc b/runtime/vm/scavenger.cc
index 306a78a4978ac104cb62f3d870690a344235ff12..e3949251b82fc3e9d04b670ba075d38606bd0160 100644
--- a/runtime/vm/scavenger.cc
+++ b/runtime/vm/scavenger.cc
@@ -193,10 +193,14 @@ class ScavengerVisitor : public ObjectPointerVisitor {
 
 class ScavengerWeakVisitor : public HandleVisitor {
  public:
-  explicit ScavengerWeakVisitor(Scavenger* scavenger)
-      :  HandleVisitor(Thread::Current()),
-         scavenger_(scavenger) {
-    ASSERT(scavenger->heap_->isolate() == Thread::Current()->isolate());
+  explicit ScavengerWeakVisitor(
+      Thread* thread,
+      Scavenger* scavenger,
+      MallocGrowableArray<FinalizablePersistentHandle*>* finalization_queue)
+      :  HandleVisitor(thread),
+         scavenger_(scavenger),
+         finalization_queue_(finalization_queue) {
+    ASSERT(scavenger->heap_->isolate() == thread->isolate());
   }
 
   void VisitHandle(uword addr) {
@@ -204,7 +208,8 @@ class ScavengerWeakVisitor : public HandleVisitor {
       reinterpret_cast<FinalizablePersistentHandle*>(addr);
     RawObject** p = handle->raw_addr();
     if (scavenger_->IsUnreachable(p)) {
-      handle->UpdateUnreachable(thread()->isolate());
+      handle->UpdateUnreachableFinalizeLater(thread()->isolate());
+      finalization_queue_->Add(handle);

[siva, 2016/06/02 20:18:05]

handle->UpdateUnreachable(thread()->isolate(), finalization_queue);

     } else {
       handle->UpdateRelocated(thread()->isolate());
     }
@@ -212,11 +217,63 @@ class ScavengerWeakVisitor : public HandleVisitor {
 
  private:
   Scavenger* scavenger_;
+  MallocGrowableArray<FinalizablePersistentHandle*>* finalization_queue_;

[siva, 2016/06/02 20:18:05]

Maybe typedef MallocGrowableArray<FinalizablePersistentHandle*>
to FinalizationQueue.

[rmacnak, 2016/06/03 01:08:31]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(ScavengerWeakVisitor);
 };
 
 
+class BackgroundFinalizer : public ThreadPool::Task {
+ public:
+  explicit BackgroundFinalizer(

[siva, 2016/06/02 20:18:05]

Why explicit? seems to have more than 1 argument.

[rmacnak, 2016/06/03 01:08:31]

Removed.

+      Isolate* isolate,
+      MallocGrowableArray<FinalizablePersistentHandle*>* handles) :
+          isolate_(isolate),
+          handles_(handles) {
+    MonitorLocker ml(isolate->heap()->finalization_tasks_lock());
+    isolate->heap()->set_finalization_tasks(
+        isolate->heap()->finalization_tasks() + 1);
+    ml.Notify();
+  }
+
+  virtual ~BackgroundFinalizer() { }
+
+  void Run() {
+    bool result = Thread::EnterIsolateAsHelper(isolate_,
+                                               Thread::kFinalizerTask,
+                                               true /* bypass_safepoint */);
+    ASSERT(result);
+
+    {
+      TIMELINE_FUNCTION_GC_DURATION(Thread::Current(),
+                                    "BackgroundFinalization");
+
+      for (intptr_t i = 0; i < handles_->length(); i++) {
+        FinalizablePersistentHandle* handle = (*handles_)[i];
+        FinalizablePersistentHandle::Finalize(isolate_, handle);
+      }
+      delete handles_;
+    }
+
+    // Exit isolate cleanly *before* notifying it, to avoid shutdown race.

[siva, 2016/06/02 20:18:05]

notifying it? (what is it).

+    Thread::ExitIsolateAsHelper();
+
+    {
+      Heap* heap = isolate_->heap();
+      MonitorLocker ml(heap->finalization_tasks_lock());
+      heap->set_finalization_tasks(heap->finalization_tasks() - 1);
+      ml.Notify();

[siva, 2016/06/02 20:18:05]

seems odd that the thread has exited the isolate but is accessing the isolate's
heap.

Why is it necessary to exit the isolate before notifying that one finalization
task is done?

[rmacnak, 2016/06/03 01:08:31]

This pattern comes from the sweeper task. This avoids running
ExitIsolateAsHelper after isolate shutdown.

+    }
+  }
+
+ private:
+  Isolate* isolate_;
+  MallocGrowableArray<FinalizablePersistentHandle*>* handles_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(BackgroundFinalizer);
+};
+
+
 // Visitor used to verify that all old->new references have been added to the
 // StoreBuffers.
 class VerifyStoreBufferPointerVisitor : public ObjectPointerVisitor {
@@ -812,8 +869,16 @@ void Scavenger::Scavenge(bool invoke_api_callbacks) {
     int64_t middle = OS::GetCurrentTimeMicros();
     {
       TIMELINE_FUNCTION_GC_DURATION(thread, "WeakHandleProcessing");
-      ScavengerWeakVisitor weak_visitor(this);
+      MallocGrowableArray<FinalizablePersistentHandle*>* finalization_queue =
+          new MallocGrowableArray<FinalizablePersistentHandle*>();
+      ScavengerWeakVisitor weak_visitor(thread, this, finalization_queue);
       IterateWeakRoots(isolate, &weak_visitor);
+      if (finalization_queue->length() > 0) {
+        Dart::thread_pool()->Run(new BackgroundFinalizer(isolate,
+                                                         finalization_queue));

[siva, 2016/06/02 20:18:05]

I am assuming the BackgroundFinalizer object created here is deleted by the
thread pool.

[rmacnak, 2016/06/03 01:08:31]

Yes, tasks are deleted by the thread pool after they finish.

+      } else {
+        delete finalization_queue;
+      }
     }
     ProcessWeakReferences();
     page_space->ReleaseDataLock();