Add read barriers to the weak roots in the intern table.
Bug: 12687968
Change-Id: I424f1df76a7e3d7154fb9f3c951c973d19bd640f
diff --git a/runtime/intern_table.cc b/runtime/intern_table.cc
index 339eb36..f12043e 100644
--- a/runtime/intern_table.cc
+++ b/runtime/intern_table.cc
@@ -82,11 +82,25 @@
// Note: we deliberately don't visit the weak_interns_ table and the immutable image roots.
}
-mirror::String* InternTable::Lookup(Table& table, mirror::String* s, int32_t hash_code) {
+mirror::String* InternTable::LookupStrong(mirror::String* s, int32_t hash_code) {
+ return Lookup<kWithoutReadBarrier>(&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<kWithReadBarrier>(&weak_interns_, s, hash_code);
+}
+
+template<ReadBarrierOption kReadBarrierOption>
+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();
+ for (auto it = table->lower_bound(hash_code), end = table->end();
it != end && it->first == hash_code; ++it) {
- mirror::String* existing_string = it->second;
+ mirror::String** weak_root = &it->second;
+ mirror::String* existing_string =
+ ReadBarrier::BarrierForWeakRoot<mirror::String, kReadBarrierOption>(weak_root);
if (existing_string->Equals(s)) {
return existing_string;
}
@@ -115,19 +129,29 @@
return s;
}
+void InternTable::RemoveStrong(mirror::String* s, int32_t hash_code) {
+ Remove<kWithoutReadBarrier>(&strong_interns_, s, hash_code);
+}
+
void InternTable::RemoveWeak(mirror::String* s, int32_t hash_code) {
Runtime* runtime = Runtime::Current();
if (runtime->IsActiveTransaction()) {
runtime->RecordWeakStringRemoval(s, hash_code);
}
- Remove(weak_interns_, s, hash_code);
+ Remove<kWithReadBarrier>(&weak_interns_, s, hash_code);
}
-void InternTable::Remove(Table& table, mirror::String* s, int32_t hash_code) {
- for (auto it = table.lower_bound(hash_code), end = table.end();
+template<ReadBarrierOption kReadBarrierOption>
+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) {
- if (it->second == s) {
- table.erase(it);
+ mirror::String** weak_root = &it->second;
+ mirror::String* existing_string =
+ ReadBarrier::BarrierForWeakRoot<mirror::String, kReadBarrierOption>(weak_root);
+ if (existing_string == s) {
+ table->erase(it);
return;
}
}
@@ -144,11 +168,11 @@
}
void InternTable::RemoveStrongFromTransaction(mirror::String* s, int32_t hash_code) {
DCHECK(!Runtime::Current()->IsActiveTransaction());
- Remove(strong_interns_, s, hash_code);
+ RemoveStrong(s, hash_code);
}
void InternTable::RemoveWeakFromTransaction(mirror::String* s, int32_t hash_code) {
DCHECK(!Runtime::Current()->IsActiveTransaction());
- Remove(weak_interns_, s, hash_code);
+ RemoveWeak(s, hash_code);
}
static mirror::String* LookupStringFromImage(mirror::String* s)
@@ -202,7 +226,7 @@
if (is_strong) {
// Check the strong table for a match.
- mirror::String* strong = Lookup(strong_interns_, s, hash_code);
+ mirror::String* strong = LookupStrong(s, hash_code);
if (strong != NULL) {
return strong;
}
@@ -214,7 +238,7 @@
}
// There is no match in the strong table, check the weak table.
- mirror::String* weak = Lookup(weak_interns_, s, hash_code);
+ mirror::String* weak = LookupWeak(s, hash_code);
if (weak != NULL) {
// A match was found in the weak table. Promote to the strong table.
RemoveWeak(weak, hash_code);
@@ -227,7 +251,7 @@
}
// Check the strong table for a match.
- mirror::String* strong = Lookup(strong_interns_, s, hash_code);
+ mirror::String* strong = LookupStrong(s, hash_code);
if (strong != NULL) {
return strong;
}
@@ -237,7 +261,7 @@
return InsertWeak(image, hash_code);
}
// Check the weak table for a match.
- mirror::String* weak = Lookup(weak_interns_, s, hash_code);
+ mirror::String* weak = LookupWeak(s, hash_code);
if (weak != NULL) {
return weak;
}
@@ -272,13 +296,14 @@
bool InternTable::ContainsWeak(mirror::String* s) {
MutexLock mu(Thread::Current(), *Locks::intern_table_lock_);
- const mirror::String* found = Lookup(weak_interns_, s, s->GetHashCode());
+ const mirror::String* found = LookupWeak(s, s->GetHashCode());
return found == s;
}
void InternTable::SweepInternTableWeaks(IsMarkedCallback* callback, void* arg) {
MutexLock mu(Thread::Current(), *Locks::intern_table_lock_);
for (auto it = weak_interns_.begin(), end = weak_interns_.end(); it != end;) {
+ // This does not need a read barrier because this is called by GC.
mirror::Object* object = it->second;
mirror::Object* new_object = callback(object, arg);
if (new_object == nullptr) {
diff --git a/runtime/intern_table.h b/runtime/intern_table.h
index 47d5e09..3df2aeb 100644
--- a/runtime/intern_table.h
+++ b/runtime/intern_table.h
@@ -79,15 +79,26 @@
LOCKS_EXCLUDED(Locks::intern_table_lock_)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
- mirror::String* Lookup(Table& table, mirror::String* s, int32_t hash_code)
+ mirror::String* LookupStrong(mirror::String* s, int32_t hash_code)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+ mirror::String* LookupWeak(mirror::String* s, int32_t hash_code)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+ template<ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ 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)
EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_);
mirror::String* InsertWeak(mirror::String* s, int32_t hash_code)
EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_);
- void RemoveWeak(mirror::String* s, int32_t hash_code)
+ void RemoveStrong(mirror::String* s, int32_t hash_code)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_);
- void Remove(Table& table, mirror::String* s, int32_t hash_code)
+ void RemoveWeak(mirror::String* s, int32_t hash_code)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+ EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_);
+ template<ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ void Remove(Table* table, mirror::String* s, int32_t hash_code)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_);
// Transaction rollback access.
@@ -96,8 +107,10 @@
mirror::String* InsertWeakFromTransaction(mirror::String* s, int32_t hash_code)
EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_);
void RemoveStrongFromTransaction(mirror::String* s, int32_t hash_code)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_);
void RemoveWeakFromTransaction(mirror::String* s, int32_t hash_code)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_);
friend class Transaction;
@@ -107,6 +120,9 @@
Table strong_interns_ GUARDED_BY(Locks::intern_table_lock_);
std::vector<std::pair<int32_t, mirror::String*>> 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.
Table weak_interns_ GUARDED_BY(Locks::intern_table_lock_);
};
diff --git a/runtime/monitor.h b/runtime/monitor.h
index 9e6d255..bd0e23c 100644
--- a/runtime/monitor.h
+++ b/runtime/monitor.h
@@ -94,8 +94,8 @@
static bool IsValidLockWord(LockWord lock_word);
template<ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- mirror::Object* GetObject() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
- return ReadBarrier::BarrierForWeakRoot<mirror::Object, kReadBarrierOption>(obj_);
+ mirror::Object* GetObject() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ return ReadBarrier::BarrierForWeakRoot<mirror::Object, kReadBarrierOption>(&obj_);
}
void SetObject(mirror::Object* object);
diff --git a/runtime/read_barrier-inl.h b/runtime/read_barrier-inl.h
index 4302c9e..e252b7b 100644
--- a/runtime/read_barrier-inl.h
+++ b/runtime/read_barrier-inl.h
@@ -44,8 +44,8 @@
}
template <typename MirrorType, ReadBarrierOption kReadBarrierOption>
-inline MirrorType* ReadBarrier::BarrierForWeakRoot(MirrorType* ref) {
- UNUSED(ref);
+inline MirrorType* ReadBarrier::BarrierForWeakRoot(MirrorType** weak_root) {
+ MirrorType* ref = *weak_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 e40e8ea..7232a3f 100644
--- a/runtime/read_barrier.h
+++ b/runtime/read_barrier.h
@@ -39,7 +39,7 @@
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
template <typename MirrorType, ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
- ALWAYS_INLINE static MirrorType* BarrierForWeakRoot(MirrorType* ref)
+ ALWAYS_INLINE static MirrorType* BarrierForWeakRoot(MirrorType** weak_root)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
};
diff --git a/runtime/transaction.h b/runtime/transaction.h
index 6fd86c8..7859126 100644
--- a/runtime/transaction.h
+++ b/runtime/transaction.h
@@ -147,7 +147,9 @@
DCHECK(s != nullptr);
}
- void Undo(InternTable* intern_table) EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_);
+ void Undo(InternTable* intern_table)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
+ EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_);
void VisitRoots(RootCallback* callback, void* arg);
private:
@@ -169,7 +171,8 @@
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
void UndoInternStringTableModifications()
EXCLUSIVE_LOCKS_REQUIRED(Locks::intern_table_lock_)
- EXCLUSIVE_LOCKS_REQUIRED(log_lock_);
+ EXCLUSIVE_LOCKS_REQUIRED(log_lock_)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
void VisitObjectLogs(RootCallback* callback, void* arg)
EXCLUSIVE_LOCKS_REQUIRED(log_lock_)