commit c11d9b8870de5f860b13c84003ade7b3f3125a52
author Mathieu Chartier <mathieuc@google.com> Thu Sep 19 10:01:59 2013 -0700
committer Mathieu Chartier <mathieuc@google.com> Fri Sep 20 10:36:49 2013 -0700
tree 89b0250d35a950774f98fa1abfa9cc296c9d85db
parent 261d3cda32b28782d894be0244e617f78182ee3b

Re-enable concurrent system weak sweeping.

Enabled by disallowing new system weaks during the pause and
re-allowing it after the system weaks have been swept. Reduces
GC pause by ~1ms.

Fixes pause regression caused by fix for
Bug: 10626133

Change-Id: If49d33e7ef19cb728ed3cef5187acfa53b9b05d8

runtime/jni_internal.cc

diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc
index d72ddf6..0a00284 100644
--- a/runtime/jni_internal.cc
+++ b/runtime/jni_internal.cc
@@ -86,14 +86,7 @@
 
 static jweak AddWeakGlobalReference(ScopedObjectAccess& soa, Object* obj)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
-  if (obj == NULL) {
-    return NULL;
-  }
-  JavaVMExt* vm = soa.Vm();
-  IndirectReferenceTable& weak_globals = vm->weak_globals;
-  WriterMutexLock mu(soa.Self(), vm->weak_globals_lock);
-  IndirectRef ref = weak_globals.Add(IRT_FIRST_SEGMENT, obj);
-  return reinterpret_cast<jweak>(ref);
+  return soa.Vm()->AddWeakGlobalReference(soa.Self(), obj);
 }
 
 static bool IsBadJniVersion(int version) {
@@ -854,17 +847,9 @@
   }
 
   static void DeleteWeakGlobalRef(JNIEnv* env, jweak obj) {
-    if (obj == NULL) {
-      return;
-    }
-    ScopedObjectAccess soa(env);
-    JavaVMExt* vm = soa.Vm();
-    IndirectReferenceTable& weak_globals = vm->weak_globals;
-    WriterMutexLock mu(soa.Self(), vm->weak_globals_lock);
-
-    if (!weak_globals.Remove(IRT_FIRST_SEGMENT, obj)) {
-      LOG(WARNING) << "JNI WARNING: DeleteWeakGlobalRef(" << obj << ") "
-                   << "failed to find entry";
+    if (obj != nullptr) {
+      ScopedObjectAccess soa(env);
+      soa.Vm()->DeleteWeakGlobalRef(soa.Self(), obj);
     }
   }
 
@@ -2996,10 +2981,12 @@
       pin_table("pin table", kPinTableInitial, kPinTableMax),
       globals_lock("JNI global reference table lock"),
       globals(gGlobalsInitial, gGlobalsMax, kGlobal),
-      weak_globals_lock("JNI weak global reference table lock"),
-      weak_globals(kWeakGlobalsInitial, kWeakGlobalsMax, kWeakGlobal),
       libraries_lock("JNI shared libraries map lock", kLoadLibraryLock),
-      libraries(new Libraries) {
+      libraries(new Libraries),
+      weak_globals_lock_("JNI weak global reference table lock"),
+      weak_globals_(kWeakGlobalsInitial, kWeakGlobalsMax, kWeakGlobal),
+      allow_new_weak_globals_(true),
+      weak_globals_add_condition_("weak globals add condition", weak_globals_lock_) {
   functions = unchecked_functions = &gJniInvokeInterface;
   if (options->check_jni_) {
     SetCheckJniEnabled(true);
@@ -3010,6 +2997,26 @@
   delete libraries;
 }
 
+jweak JavaVMExt::AddWeakGlobalReference(Thread* self, mirror::Object* obj) {
+  if (obj == nullptr) {
+    return nullptr;
+  }
+  MutexLock mu(self, weak_globals_lock_);
+  while (UNLIKELY(!allow_new_weak_globals_)) {
+    weak_globals_add_condition_.WaitHoldingLocks(self);
+  }
+  IndirectRef ref = weak_globals_.Add(IRT_FIRST_SEGMENT, obj);
+  return reinterpret_cast<jweak>(ref);
+}
+
+void JavaVMExt::DeleteWeakGlobalRef(Thread* self, jweak obj) {
+  MutexLock mu(self, weak_globals_lock_);
+  if (!weak_globals_.Remove(IRT_FIRST_SEGMENT, obj)) {
+    LOG(WARNING) << "JNI WARNING: DeleteWeakGlobalRef(" << obj << ") "
+                 << "failed to find entry";
+  }
+}
+
 void JavaVMExt::SetCheckJniEnabled(bool enabled) {
   check_jni = enabled;
   functions = enabled ? GetCheckJniInvokeInterface() : &gJniInvokeInterface;
@@ -3031,9 +3038,9 @@
     os << "; globals=" << globals.Capacity();
   }
   {
-    ReaderMutexLock mu(self, weak_globals_lock);
-    if (weak_globals.Capacity() > 0) {
-      os << " (plus " << weak_globals.Capacity() << " weak)";
+    MutexLock mu(self, weak_globals_lock_);
+    if (weak_globals_.Capacity() > 0) {
+      os << " (plus " << weak_globals_.Capacity() << " weak)";
     }
   }
   os << '\n';
@@ -3044,6 +3051,35 @@
   }
 }
 
+void JavaVMExt::DisallowNewWeakGlobals() {
+  MutexLock mu(Thread::Current(), weak_globals_lock_);
+  allow_new_weak_globals_ = false;
+}
+
+void JavaVMExt::AllowNewWeakGlobals() {
+  Thread* self = Thread::Current();
+  MutexLock mu(self, weak_globals_lock_);
+  allow_new_weak_globals_ = true;
+  weak_globals_add_condition_.Broadcast(self);
+}
+
+void JavaVMExt::SweepWeakGlobals(IsMarkedTester is_marked, void* arg) {
+  MutexLock mu(Thread::Current(), weak_globals_lock_);
+  for (const Object** entry : weak_globals_) {
+    if (!is_marked(*entry, arg)) {
+      *entry = kClearedJniWeakGlobal;
+    }
+  }
+}
+
+mirror::Object* JavaVMExt::DecodeWeakGlobal(Thread* self, IndirectRef ref) {
+  MutexLock mu(self, weak_globals_lock_);
+  while (UNLIKELY(!allow_new_weak_globals_)) {
+    weak_globals_add_condition_.WaitHoldingLocks(self);
+  }
+  return const_cast<mirror::Object*>(weak_globals_.Get(ref));
+}
+
 void JavaVMExt::DumpReferenceTables(std::ostream& os) {
   Thread* self = Thread::Current();
   {
@@ -3051,8 +3087,8 @@
     globals.Dump(os);
   }
   {
-    ReaderMutexLock mu(self, weak_globals_lock);
-    weak_globals.Dump(os);
+    MutexLock mu(self, weak_globals_lock_);
+    weak_globals_.Dump(os);
   }
   {
     MutexLock mu(self, pins_lock);