diff options
| -rw-r--r-- | runtime/indirect_reference_table-inl.h | 6 | ||||
| -rw-r--r-- | runtime/indirect_reference_table.cc | 21 | ||||
| -rw-r--r-- | runtime/indirect_reference_table.h | 8 | ||||
| -rw-r--r-- | runtime/jni_internal.cc | 9 | ||||
| -rw-r--r-- | runtime/jni_internal.h | 3 | ||||
| -rw-r--r-- | runtime/thread.cc | 7 |
6 files changed, 42 insertions, 12 deletions
diff --git a/runtime/indirect_reference_table-inl.h b/runtime/indirect_reference_table-inl.h index 42a9757951..790f4d0c17 100644 --- a/runtime/indirect_reference_table-inl.h +++ b/runtime/indirect_reference_table-inl.h @@ -71,12 +71,16 @@ inline bool IndirectReferenceTable::CheckEntry(const char* what, IndirectRef ire return true; } +template<ReadBarrierOption kReadBarrierOption> inline mirror::Object* IndirectReferenceTable::Get(IndirectRef iref) const { if (!GetChecked(iref)) { return kInvalidIndirectRefObject; } - mirror::Object* obj = table_[ExtractIndex(iref)]; + mirror::Object** root = &table_[ExtractIndex(iref)]; + mirror::Object* obj = *root; if (LIKELY(obj != kClearedJniWeakGlobal)) { + // The read barrier or VerifyObject won't handle kClearedJniWeakGlobal. + obj = ReadBarrier::BarrierForWeakRoot<mirror::Object, kReadBarrierOption>(root); VerifyObject(obj); } return obj; diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc index 432481bee3..756ac9606e 100644 --- a/runtime/indirect_reference_table.cc +++ b/runtime/indirect_reference_table.cc @@ -266,11 +266,22 @@ void IndirectReferenceTable::VisitRoots(RootCallback* callback, void* arg, uint3 void IndirectReferenceTable::Dump(std::ostream& os) const { os << kind_ << " table dump:\n"; - ReferenceTable::Table entries(table_, table_ + Capacity()); - // Remove NULLs. - for (int i = entries.size() - 1; i >= 0; --i) { - if (entries[i] == NULL) { - entries.erase(entries.begin() + i); + ReferenceTable::Table entries; + for (size_t i = 0; i < Capacity(); ++i) { + mirror::Object** root = &table_[i]; + mirror::Object* obj = *root; + if (UNLIKELY(obj == nullptr)) { + // Remove NULLs. + } else if (UNLIKELY(obj == kClearedJniWeakGlobal)) { + // ReferenceTable::Dump() will handle kClearedJniWeakGlobal + // while the read barrier won't. + entries.push_back(obj); + } else { + // We need a read barrier if weak globals. Since this is for + // debugging where performance isn't top priority, we + // unconditionally enable the read barrier, which is conservative. + obj = ReadBarrier::BarrierForWeakRoot<mirror::Object, kWithReadBarrier>(root); + entries.push_back(obj); } } ReferenceTable::Dump(os, entries); diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h index 5015410874..5b3ed685c7 100644 --- a/runtime/indirect_reference_table.h +++ b/runtime/indirect_reference_table.h @@ -263,14 +263,16 @@ class IndirectReferenceTable { * * Returns kInvalidIndirectRefObject if iref is invalid. */ + template<ReadBarrierOption kReadBarrierOption = kWithReadBarrier> mirror::Object* Get(IndirectRef iref) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) ALWAYS_INLINE; // 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_LOCKS_REQUIRED(Locks::mutator_lock_) { - return Get(iref); + return Get<kReadBarrierOption>(iref); } /* @@ -366,7 +368,9 @@ class IndirectReferenceTable { std::unique_ptr<MemMap> table_mem_map_; // Mem map where we store the extended debugging info. std::unique_ptr<MemMap> slot_mem_map_; - /* bottom of the stack */ + // bottom of the stack. If a JNI weak global table, do not directly + // access the object references in this as they are weak roots. Use + // Get() that has a read barrier. mirror::Object** table_; /* bit mask, ORed into all irefs */ IndirectRefKind kind_; diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc index b51e1d586c..a660183bcf 100644 --- a/runtime/jni_internal.cc +++ b/runtime/jni_internal.cc @@ -2441,7 +2441,9 @@ class JNI { switch (kind) { case kLocal: { ScopedObjectAccess soa(env); - if (static_cast<JNIEnvExt*>(env)->locals.Get(ref) != kInvalidIndirectRefObject) { + // The local refs don't need a read barrier. + if (static_cast<JNIEnvExt*>(env)->locals.Get<kWithoutReadBarrier>(ref) != + kInvalidIndirectRefObject) { return JNILocalRefType; } return JNIInvalidRefType; @@ -3118,7 +3120,9 @@ mirror::Object* JavaVMExt::DecodeWeakGlobal(Thread* self, IndirectRef ref) { while (UNLIKELY(!allow_new_weak_globals_)) { weak_globals_add_condition_.WaitHoldingLocks(self); } - return weak_globals_.Get(ref); + // The weak globals do need a read barrier as they are weak roots. + mirror::Object* obj = weak_globals_.Get<kWithReadBarrier>(ref); + return obj; } void JavaVMExt::DumpReferenceTables(std::ostream& os) { @@ -3298,6 +3302,7 @@ void* JavaVMExt::FindCodeForNativeMethod(mirror::ArtMethod* m) { void JavaVMExt::SweepJniWeakGlobals(IsMarkedCallback* callback, void* arg) { MutexLock mu(Thread::Current(), weak_globals_lock_); for (mirror::Object** entry : weak_globals_) { + // Since this is called by the GC, we don't need a read barrier. mirror::Object* obj = *entry; mirror::Object* new_obj = callback(obj, arg); if (new_obj == nullptr) { diff --git a/runtime/jni_internal.h b/runtime/jni_internal.h index 7e76e11c71..4072da4695 100644 --- a/runtime/jni_internal.h +++ b/runtime/jni_internal.h @@ -129,6 +129,9 @@ class JavaVMExt : public JavaVM { // TODO: Make the other members of this class also private. // JNI weak global references. Mutex weak_globals_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER; + // 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_); ConditionVariable weak_globals_add_condition_ GUARDED_BY(weak_globals_lock_); diff --git a/runtime/thread.cc b/runtime/thread.cc index 55bec1e9fd..1355aa1143 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -1253,7 +1253,8 @@ mirror::Object* Thread::DecodeJObject(jobject obj) const { // The "kinds" below are sorted by the frequency we expect to encounter them. if (kind == kLocal) { IndirectReferenceTable& locals = tlsPtr_.jni_env->locals; - result = locals.Get(ref); + // Local references do not need a read barrier. + result = locals.Get<kWithoutReadBarrier>(ref); } else if (kind == kHandleScopeOrInvalid) { // TODO: make stack indirect reference table lookup more efficient. // Check if this is a local reference in the handle scope. @@ -1266,7 +1267,9 @@ mirror::Object* Thread::DecodeJObject(jobject obj) const { } } else if (kind == kGlobal) { JavaVMExt* const vm = Runtime::Current()->GetJavaVM(); - result = vm->globals.SynchronizedGet(const_cast<Thread*>(this), &vm->globals_lock, ref); + // Strong global references do not need a read barrier. + result = vm->globals.SynchronizedGet<kWithoutReadBarrier>( + const_cast<Thread*>(this), &vm->globals_lock, ref); } else { DCHECK_EQ(kind, kWeakGlobal); result = Runtime::Current()->GetJavaVM()->DecodeWeakGlobal(const_cast<Thread*>(this), ref); |