From a91a4bc1f8960f64c5f7e4616d46e21b8e1bfba2 Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Fri, 13 Jun 2014 16:44:55 -0700 Subject: Add read barriers for the class and the intern tables. Add read barriers for the strong roots in the intern table and the (strong) roots in the class table to make possible concurrent scanning of them. Bug: 12687968 Change-Id: If6edc33a37e65a8494e66dc3b144138b1530367f --- runtime/class_linker.cc | 24 +++++++++++++++-------- runtime/class_linker.h | 2 ++ runtime/indirect_reference_table-inl.h | 2 +- runtime/indirect_reference_table.cc | 2 +- runtime/intern_table.cc | 36 +++++++++++++++++++--------------- runtime/intern_table.h | 14 ++++++++----- runtime/monitor.h | 2 +- runtime/read_barrier-inl.h | 4 ++-- runtime/read_barrier.h | 2 +- 9 files changed, 53 insertions(+), 35 deletions(-) (limited to 'runtime') diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 7385382359..d684a50731 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -1137,8 +1137,10 @@ void ClassLinker::VisitClasses(ClassVisitor* visitor, void* arg) { MoveImageClassesToClassTable(); } WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_); - for (const std::pair& it : class_table_) { - if (!visitor(it.second, arg)) { + for (std::pair& it : class_table_) { + mirror::Class** root = &it.second; + mirror::Class* c = ReadBarrier::BarrierForRoot(root); + if (!visitor(c, arg)) { return; } } @@ -2353,7 +2355,8 @@ bool ClassLinker::RemoveClass(const char* descriptor, const mirror::ClassLoader* for (auto it = class_table_.lower_bound(hash), end = class_table_.end(); it != end && it->first == hash; ++it) { - mirror::Class* klass = it->second; + mirror::Class** root = &it->second; + mirror::Class* klass = ReadBarrier::BarrierForRoot(root); if (klass->GetClassLoader() == class_loader && klass->DescriptorEquals(descriptor)) { class_table_.erase(it); return true; @@ -2397,12 +2400,14 @@ mirror::Class* ClassLinker::LookupClassFromTableLocked(const char* descriptor, size_t hash) { auto end = class_table_.end(); for (auto it = class_table_.lower_bound(hash); it != end && it->first == hash; ++it) { - mirror::Class* klass = it->second; + mirror::Class** root = &it->second; + mirror::Class* klass = ReadBarrier::BarrierForRoot(root); if (klass->GetClassLoader() == class_loader && klass->DescriptorEquals(descriptor)) { if (kIsDebugBuild) { // Check for duplicates in the table. for (++it; it != end && it->first == hash; ++it) { - mirror::Class* klass2 = it->second; + mirror::Class** root2 = &it->second; + mirror::Class* klass2 = ReadBarrier::BarrierForRoot(root2); CHECK(!(klass2->GetClassLoader() == class_loader && klass2->DescriptorEquals(descriptor))) << PrettyClass(klass) << " " << klass << " " << klass->GetClassLoader() << " " @@ -2494,7 +2499,8 @@ void ClassLinker::LookupClasses(const char* descriptor, std::vectorfirst == hash; ++it) { - mirror::Class* klass = it->second; + mirror::Class** root = &it->second; + mirror::Class* klass = ReadBarrier::BarrierForRoot(root); if (klass->DescriptorEquals(descriptor)) { result.push_back(klass); } @@ -4362,8 +4368,10 @@ void ClassLinker::DumpAllClasses(int flags) { std::vector all_classes; { ReaderMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_); - for (const std::pair& it : class_table_) { - all_classes.push_back(it.second); + for (std::pair& it : class_table_) { + mirror::Class** root = &it.second; + mirror::Class* klass = ReadBarrier::BarrierForRoot(root); + all_classes.push_back(klass); } } diff --git a/runtime/class_linker.h b/runtime/class_linker.h index a1d7bc6bd5..6d96aa2637 100644 --- a/runtime/class_linker.h +++ b/runtime/class_linker.h @@ -573,6 +573,8 @@ class ClassLinker { // mirror::Class* instances. Results should be compared for a matching // Class::descriptor_ and Class::class_loader_. typedef std::multimap Table; + // This contains strong roots. To enable concurrent root scanning of + // the class table, be careful to use a read barrier when accessing this. Table class_table_ GUARDED_BY(Locks::classlinker_classes_lock_); std::vector> new_class_roots_; diff --git a/runtime/indirect_reference_table-inl.h b/runtime/indirect_reference_table-inl.h index 790f4d0c17..b787233b37 100644 --- a/runtime/indirect_reference_table-inl.h +++ b/runtime/indirect_reference_table-inl.h @@ -80,7 +80,7 @@ inline mirror::Object* IndirectReferenceTable::Get(IndirectRef iref) const { mirror::Object* obj = *root; if (LIKELY(obj != kClearedJniWeakGlobal)) { // The read barrier or VerifyObject won't handle kClearedJniWeakGlobal. - obj = ReadBarrier::BarrierForWeakRoot(root); + obj = ReadBarrier::BarrierForRoot(root); VerifyObject(obj); } return obj; diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc index 756ac9606e..98e1d21d93 100644 --- a/runtime/indirect_reference_table.cc +++ b/runtime/indirect_reference_table.cc @@ -280,7 +280,7 @@ void IndirectReferenceTable::Dump(std::ostream& os) const { // 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(root); + obj = ReadBarrier::BarrierForRoot(root); entries.push_back(obj); } } diff --git a/runtime/intern_table.cc b/runtime/intern_table.cc index f12043e944..14305006e4 100644 --- a/runtime/intern_table.cc +++ b/runtime/intern_table.cc @@ -38,6 +38,16 @@ size_t InternTable::Size() const { return strong_interns_.size() + weak_interns_.size(); } +size_t InternTable::StrongSize() const { + MutexLock mu(Thread::Current(), *Locks::intern_table_lock_); + return strong_interns_.size(); +} + +size_t InternTable::WeakSize() const { + MutexLock mu(Thread::Current(), *Locks::intern_table_lock_); + return weak_interns_.size(); +} + void InternTable::DumpForSigQuit(std::ostream& os) const { MutexLock mu(Thread::Current(), *Locks::intern_table_lock_); os << "Intern table: " << strong_interns_.size() << " strong; " @@ -83,24 +93,21 @@ void InternTable::VisitRoots(RootCallback* callback, void* arg, VisitRootFlags f } mirror::String* InternTable::LookupStrong(mirror::String* s, int32_t hash_code) { - return Lookup(&strong_interns_, s, hash_code); + return Lookup(&strong_interns_, s, hash_code); } mirror::String* InternTable::LookupWeak(mirror::String* s, int32_t hash_code) { // Weak interns need a read barrier because they are weak roots. - return Lookup(&weak_interns_, s, hash_code); + return Lookup(&weak_interns_, s, hash_code); } -template mirror::String* InternTable::Lookup(Table* table, mirror::String* s, int32_t hash_code) { - CHECK_EQ(table == &weak_interns_, kReadBarrierOption == kWithReadBarrier) - << "Only weak_interns_ needs a read barrier."; Locks::intern_table_lock_->AssertHeld(Thread::Current()); for (auto it = table->lower_bound(hash_code), end = table->end(); it != end && it->first == hash_code; ++it) { - mirror::String** weak_root = &it->second; - mirror::String* existing_string = - ReadBarrier::BarrierForWeakRoot(weak_root); + mirror::String* existing_string; + mirror::String** root = &it->second; + existing_string = ReadBarrier::BarrierForRoot(root); if (existing_string->Equals(s)) { return existing_string; } @@ -130,7 +137,7 @@ mirror::String* InternTable::InsertWeak(mirror::String* s, int32_t hash_code) { } void InternTable::RemoveStrong(mirror::String* s, int32_t hash_code) { - Remove(&strong_interns_, s, hash_code); + Remove(&strong_interns_, s, hash_code); } void InternTable::RemoveWeak(mirror::String* s, int32_t hash_code) { @@ -138,18 +145,15 @@ void InternTable::RemoveWeak(mirror::String* s, int32_t hash_code) { if (runtime->IsActiveTransaction()) { runtime->RecordWeakStringRemoval(s, hash_code); } - Remove(&weak_interns_, s, hash_code); + Remove(&weak_interns_, s, hash_code); } -template void InternTable::Remove(Table* table, mirror::String* s, int32_t hash_code) { - CHECK_EQ(table == &weak_interns_, kReadBarrierOption == kWithReadBarrier) - << "Only weak_interns_ needs a read barrier."; for (auto it = table->lower_bound(hash_code), end = table->end(); it != end && it->first == hash_code; ++it) { - mirror::String** weak_root = &it->second; - mirror::String* existing_string = - ReadBarrier::BarrierForWeakRoot(weak_root); + mirror::String* existing_string; + mirror::String** root = &it->second; + existing_string = ReadBarrier::BarrierForRoot(root); if (existing_string == s) { table->erase(it); return; diff --git a/runtime/intern_table.h b/runtime/intern_table.h index 3df2aebb47..6dc7f7b606 100644 --- a/runtime/intern_table.h +++ b/runtime/intern_table.h @@ -64,6 +64,8 @@ class InternTable { bool ContainsWeak(mirror::String* s) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); size_t Size() const; + size_t StrongSize() const; + size_t WeakSize() const; void VisitRoots(RootCallback* callback, void* arg, VisitRootFlags flags); @@ -83,7 +85,6 @@ class InternTable { SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); mirror::String* LookupWeak(mirror::String* s, int32_t hash_code) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - template mirror::String* Lookup(Table* table, mirror::String* s, int32_t hash_code) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); mirror::String* InsertStrong(mirror::String* s, int32_t hash_code) @@ -96,7 +97,6 @@ class InternTable { void RemoveWeak(mirror::String* s, int32_t hash_code) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_); - template void Remove(Table* table, mirror::String* s, int32_t hash_code) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_); @@ -117,12 +117,16 @@ class InternTable { bool log_new_roots_ GUARDED_BY(Locks::intern_table_lock_); bool allow_new_interns_ GUARDED_BY(Locks::intern_table_lock_); ConditionVariable new_intern_condition_ GUARDED_BY(Locks::intern_table_lock_); + // Since this contains (strong) roots, they need a read barrier to + // enable concurrent intern table (strong) root scan. Do not + // directly access the strings in it. Use functions that contain + // read barriers. Table strong_interns_ GUARDED_BY(Locks::intern_table_lock_); std::vector> new_strong_intern_roots_ GUARDED_BY(Locks::intern_table_lock_); - // Since weak_interns_ contain weak roots, they need a read - // barrier. Do not directly access the strings in it. Use functions - // that contain read barriers. + // Since this contains (weak) roots, they need a read barrier. Do + // not directly access the strings in it. Use functions that contain + // read barriers. Table weak_interns_ GUARDED_BY(Locks::intern_table_lock_); }; diff --git a/runtime/monitor.h b/runtime/monitor.h index bd0e23cafe..a28823d184 100644 --- a/runtime/monitor.h +++ b/runtime/monitor.h @@ -95,7 +95,7 @@ class Monitor { template mirror::Object* GetObject() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { - return ReadBarrier::BarrierForWeakRoot(&obj_); + return ReadBarrier::BarrierForRoot(&obj_); } void SetObject(mirror::Object* object); diff --git a/runtime/read_barrier-inl.h b/runtime/read_barrier-inl.h index e252b7bb83..fd43d78835 100644 --- a/runtime/read_barrier-inl.h +++ b/runtime/read_barrier-inl.h @@ -44,8 +44,8 @@ inline MirrorType* ReadBarrier::Barrier( } template -inline MirrorType* ReadBarrier::BarrierForWeakRoot(MirrorType** weak_root) { - MirrorType* ref = *weak_root; +inline MirrorType* ReadBarrier::BarrierForRoot(MirrorType** root) { + MirrorType* ref = *root; const bool with_read_barrier = kReadBarrierOption == kWithReadBarrier; if (with_read_barrier && kUseBakerReadBarrier) { // To be implemented. diff --git a/runtime/read_barrier.h b/runtime/read_barrier.h index 7232a3febe..451d13c5db 100644 --- a/runtime/read_barrier.h +++ b/runtime/read_barrier.h @@ -39,7 +39,7 @@ class ReadBarrier { SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); template - ALWAYS_INLINE static MirrorType* BarrierForWeakRoot(MirrorType** weak_root) + ALWAYS_INLINE static MirrorType* BarrierForRoot(MirrorType** root) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); }; -- cgit v1.2.3