Enable lockless decoding of weak globals
Will help speed up decoding weak DexCache roots.
Change-Id: I9a68beb4106cbd383111a30e249c9b0149064e78
diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h
index c398555..d13526b 100644
--- a/runtime/indirect_reference_table.h
+++ b/runtime/indirect_reference_table.h
@@ -290,9 +290,7 @@
// Synchronized get which reads a reference, acquiring a lock if necessary.
template<ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- mirror::Object* SynchronizedGet(Thread* /*self*/, ReaderWriterMutex* /*mutex*/,
- IndirectRef iref) const
- SHARED_REQUIRES(Locks::mutator_lock_) {
+ mirror::Object* SynchronizedGet(IndirectRef iref) const SHARED_REQUIRES(Locks::mutator_lock_) {
return Get<kReadBarrierOption>(iref);
}
diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc
index ef7a924..8060e3d 100644
--- a/runtime/java_vm_ext.cc
+++ b/runtime/java_vm_ext.cc
@@ -375,7 +375,7 @@
unchecked_functions_(&gJniInvokeInterface),
weak_globals_lock_("JNI weak global reference table lock", kJniWeakGlobalsLock),
weak_globals_(kWeakGlobalsInitial, kWeakGlobalsMax, kWeakGlobal),
- allow_new_weak_globals_(true),
+ allow_accessing_weak_globals_(true),
weak_globals_add_condition_("weak globals add condition", weak_globals_lock_) {
functions = unchecked_functions_;
SetCheckJniEnabled(runtime_options.Exists(RuntimeArgumentMap::CheckJni));
@@ -473,8 +473,7 @@
return nullptr;
}
MutexLock mu(self, weak_globals_lock_);
- while (UNLIKELY((!kUseReadBarrier && !allow_new_weak_globals_) ||
- (kUseReadBarrier && !self->GetWeakRefAccessEnabled()))) {
+ while (UNLIKELY(!MayAccessWeakGlobals(self))) {
weak_globals_add_condition_.WaitHoldingLocks(self);
}
IndirectRef ref = weak_globals_.Add(IRT_FIRST_SEGMENT, obj);
@@ -542,14 +541,19 @@
}
void JavaVMExt::DisallowNewWeakGlobals() {
- MutexLock mu(Thread::Current(), weak_globals_lock_);
- allow_new_weak_globals_ = false;
+ Thread* const self = Thread::Current();
+ MutexLock mu(self, weak_globals_lock_);
+ // DisallowNewWeakGlobals is only called by CMS during the pause. It is required to have the
+ // mutator lock exclusively held so that we don't have any threads in the middle of
+ // DecodeWeakGlobal.
+ Locks::mutator_lock_->AssertExclusiveHeld(self);
+ allow_accessing_weak_globals_.StoreSequentiallyConsistent(false);
}
void JavaVMExt::AllowNewWeakGlobals() {
Thread* self = Thread::Current();
MutexLock mu(self, weak_globals_lock_);
- allow_new_weak_globals_ = true;
+ allow_accessing_weak_globals_.StoreSequentiallyConsistent(true);
weak_globals_add_condition_.Broadcast(self);
}
@@ -557,7 +561,7 @@
// Lock and unlock once to ensure that no threads are still in the
// middle of adding new weak globals.
MutexLock mu(Thread::Current(), weak_globals_lock_);
- CHECK(!allow_new_weak_globals_);
+ CHECK(!allow_accessing_weak_globals_.LoadSequentiallyConsistent());
}
void JavaVMExt::BroadcastForNewWeakGlobals() {
@@ -567,8 +571,8 @@
weak_globals_add_condition_.Broadcast(self);
}
-mirror::Object* JavaVMExt::DecodeGlobal(Thread* self, IndirectRef ref) {
- return globals_.SynchronizedGet(self, &globals_lock_, ref);
+mirror::Object* JavaVMExt::DecodeGlobal(IndirectRef ref) {
+ return globals_.SynchronizedGet(ref);
}
void JavaVMExt::UpdateGlobal(Thread* self, IndirectRef ref, mirror::Object* result) {
@@ -576,7 +580,25 @@
globals_.Update(ref, result);
}
+inline bool JavaVMExt::MayAccessWeakGlobals(Thread* self) const {
+ return MayAccessWeakGlobalsUnlocked(self);
+}
+
+inline bool JavaVMExt::MayAccessWeakGlobalsUnlocked(Thread* self) const {
+ return kUseReadBarrier ? self->GetWeakRefAccessEnabled() :
+ allow_accessing_weak_globals_.LoadSequentiallyConsistent();
+}
+
mirror::Object* JavaVMExt::DecodeWeakGlobal(Thread* self, IndirectRef ref) {
+ // It is safe to access GetWeakRefAccessEnabled without the lock since CC uses checkpoints to call
+ // SetWeakRefAccessEnabled, and the other collectors only modify allow_accessing_weak_globals_
+ // when the mutators are paused.
+ // This only applies in the case where MayAccessWeakGlobals goes from false to true. In the other
+ // case, it may be racy, this is benign since DecodeWeakGlobalLocked does the correct behavior
+ // if MayAccessWeakGlobals is false.
+ if (LIKELY(MayAccessWeakGlobalsUnlocked(self))) {
+ return weak_globals_.SynchronizedGet(ref);
+ }
MutexLock mu(self, weak_globals_lock_);
return DecodeWeakGlobalLocked(self, ref);
}
@@ -585,8 +607,7 @@
if (kDebugLocking) {
weak_globals_lock_.AssertHeld(self);
}
- while (UNLIKELY((!kUseReadBarrier && !allow_new_weak_globals_) ||
- (kUseReadBarrier && !self->GetWeakRefAccessEnabled()))) {
+ while (UNLIKELY(!MayAccessWeakGlobals(self))) {
weak_globals_add_condition_.WaitHoldingLocks(self);
}
return weak_globals_.Get(ref);
diff --git a/runtime/java_vm_ext.h b/runtime/java_vm_ext.h
index e80266f..d68a85f 100644
--- a/runtime/java_vm_ext.h
+++ b/runtime/java_vm_ext.h
@@ -126,7 +126,7 @@
void SweepJniWeakGlobals(IsMarkedVisitor* visitor)
SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(!weak_globals_lock_);
- mirror::Object* DecodeGlobal(Thread* self, IndirectRef ref)
+ mirror::Object* DecodeGlobal(IndirectRef ref)
SHARED_REQUIRES(Locks::mutator_lock_);
void UpdateGlobal(Thread* self, IndirectRef ref, mirror::Object* result)
@@ -155,6 +155,12 @@
REQUIRES(!globals_lock_);
private:
+ // Return true if self can currently access weak globals.
+ bool MayAccessWeakGlobalsUnlocked(Thread* self) const SHARED_REQUIRES(Locks::mutator_lock_);
+ bool MayAccessWeakGlobals(Thread* self) const
+ SHARED_REQUIRES(Locks::mutator_lock_)
+ REQUIRES(weak_globals_lock_);
+
Runtime* const runtime_;
// Used for testing. By default, we'll LOG(FATAL) the reason.
@@ -184,8 +190,10 @@
// Since weak_globals_ contain weak roots, be careful not to
// directly access the object references in it. Use Get() with the
// read barrier enabled.
- IndirectReferenceTable weak_globals_ GUARDED_BY(weak_globals_lock_);
- bool allow_new_weak_globals_ GUARDED_BY(weak_globals_lock_);
+ // Not guarded by weak_globals_lock since we may use SynchronizedGet in DecodeWeakGlobal.
+ IndirectReferenceTable weak_globals_;
+ // Not guarded by weak_globals_lock since we may use SynchronizedGet in DecodeWeakGlobal.
+ Atomic<bool> allow_accessing_weak_globals_;
ConditionVariable weak_globals_add_condition_ GUARDED_BY(weak_globals_lock_);
DISALLOW_COPY_AND_ASSIGN(JavaVMExt);
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 63534b1..9929487 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -1728,7 +1728,7 @@
result = nullptr;
}
} else if (kind == kGlobal) {
- result = tlsPtr_.jni_env->vm->DecodeGlobal(const_cast<Thread*>(this), ref);
+ result = tlsPtr_.jni_env->vm->DecodeGlobal(ref);
} else {
DCHECK_EQ(kind, kWeakGlobal);
result = tlsPtr_.jni_env->vm->DecodeWeakGlobal(const_cast<Thread*>(this), ref);