Merge "ART: checkpoint mechanism optimization"

4 files changed, 35 insertions, 9 deletions

diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc
index 2ad8e9c..734c935 100644
--- a/runtime/gc/collector/concurrent_copying.cc
+++ b/runtime/gc/collector/concurrent_copying.cc
@@ -298,7 +298,11 @@ class EmptyCheckpoint : public Closure {
     Thread* self = Thread::Current();
     CHECK(thread == self || thread->IsSuspended() || thread->GetState() == kWaitingPerformingGc)
         << thread->GetState() << " thread " << thread << " self " << self;
-    concurrent_copying_->GetBarrier().Pass(self);
+    // If thread is a running mutator, then act on behalf of the garbage collector.
+    // See the code in ThreadList::RunCheckpoint.
+    if (thread->GetState() == kRunnable) {
+      concurrent_copying_->GetBarrier().Pass(self);
+    }
   }
 
  private:
@@ -431,6 +435,11 @@ void ConcurrentCopying::IssueEmptyCheckpoint() {
   ThreadList* thread_list = Runtime::Current()->GetThreadList();
   gc_barrier_->Init(self, 0);
   size_t barrier_count = thread_list->RunCheckpoint(&check_point);
+  // If there are no threads to wait which implys that all the checkpoint functions are finished,
+  // then no need to release the mutator lock.
+  if (barrier_count == 0) {
+    return;
+  }
   // Release locks then wait for all mutator threads to pass the barrier.
   Locks::mutator_lock_->SharedUnlock(self);
   {
diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc
index d7a9292..cd63d26 100644
--- a/runtime/gc/collector/mark_sweep.cc
+++ b/runtime/gc/collector/mark_sweep.cc
@@ -989,7 +989,11 @@ class CheckpointMarkThreadRoots : public Closure {
       mark_sweep_->GetHeap()->RevokeRosAllocThreadLocalBuffers(thread);
       ATRACE_END();
     }
-    mark_sweep_->GetBarrier().Pass(self);
+    // If thread is a running mutator, then act on behalf of the garbage collector.
+    // See the code in ThreadList::RunCheckpoint.
+    if (thread->GetState() == kRunnable) {
+      mark_sweep_->GetBarrier().Pass(self);
+    }
   }
 
  private:
@@ -1006,7 +1010,11 @@ void MarkSweep::MarkRootsCheckpoint(Thread* self,
   // run through the barrier including self.
   size_t barrier_count = thread_list->RunCheckpoint(&check_point);
   // Release locks then wait for all mutator threads to pass the barrier.
-  // TODO: optimize to not release locks when there are no threads to wait for.
+  // If there are no threads to wait which implys that all the checkpoint functions are finished,
+  // then no need to release locks.
+  if (barrier_count == 0) {
+    return;
+  }
   Locks::heap_bitmap_lock_->ExclusiveUnlock(self);
   Locks::mutator_lock_->SharedUnlock(self);
   {
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 5a60c87..3f3add8 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -1061,7 +1061,11 @@ class TrimIndirectReferenceTableClosure : public Closure {
     ATRACE_BEGIN("Trimming reference table");
     thread->GetJniEnv()->locals.Trim();
     ATRACE_END();
-    barrier_->Pass(Thread::Current());
+    // If thread is a running mutator, then act on behalf of the trim thread.
+    // See the code in ThreadList::RunCheckpoint.
+    if (thread->GetState() == kRunnable) {
+      barrier_->Pass(Thread::Current());
+    }
   }
 
  private:
@@ -1079,7 +1083,9 @@ void Heap::TrimIndirectReferenceTables(Thread* self) {
   TrimIndirectReferenceTableClosure closure(&barrier);
   ScopedThreadStateChange tsc(self, kWaitingForCheckPointsToRun);
   size_t barrier_count = Runtime::Current()->GetThreadList()->RunCheckpoint(&closure);
-  barrier.Increment(self, barrier_count);
+  if (barrier_count != 0) {
+    barrier.Increment(self, barrier_count);
+  }
   ATRACE_END();
 }
 
diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc
index 58e5b9d..ef24efc 100644
--- a/runtime/thread_list.cc
+++ b/runtime/thread_list.cc
@@ -174,7 +174,9 @@ class DumpCheckpoint FINAL : public Closure {
       MutexLock mu(self, *Locks::logging_lock_);
       *os_ << local_os.str();
     }
-    barrier_.Pass(self);
+    if (thread->GetState() == kRunnable) {
+      barrier_.Pass(self);
+    }
   }
 
   void WaitForThreadsToRunThroughCheckpoint(size_t threads_running_checkpoint) {
@@ -202,7 +204,9 @@ void ThreadList::Dump(std::ostream& os) {
   }
   DumpCheckpoint checkpoint(&os);
   size_t threads_running_checkpoint = RunCheckpoint(&checkpoint);
-  checkpoint.WaitForThreadsToRunThroughCheckpoint(threads_running_checkpoint);
+  if (threads_running_checkpoint != 0) {
+    checkpoint.WaitForThreadsToRunThroughCheckpoint(threads_running_checkpoint);
+  }
 }
 
 void ThreadList::AssertThreadsAreSuspended(Thread* self, Thread* ignore1, Thread* ignore2) {
@@ -324,8 +328,7 @@ size_t ThreadList::RunCheckpoint(Closure* checkpoint_function) {
     Thread::resume_cond_->Broadcast(self);
   }
 
-  // Add one for self.
-  return count + suspended_count_modified_threads.size() + 1;
+  return count;
 }
 
 // Request that a checkpoint function be run on all active (non-suspended)