From c6934e36d33ab402b7b51c78d46c319fc33e8ef3 Mon Sep 17 00:00:00 2001
From: Vladimir Marko <vmarko@google.com>
Date: Wed, 10 Apr 2019 11:40:01 +0100
Subject: Avoid some read barriers in ClassTable.

And clean up ClassTable.

Test: m test-art-host-gtest
Test: testrunner.py --host --interpreter
Bug: 119486698
Change-Id: Ie413bc5a56eb548352ae1fed0976b75c44e0c0d4
---
 runtime/class_table-inl.h  | 10 +++++-----
 runtime/class_table.cc     | 28 ++++++++++++++--------------
 runtime/class_table.h      |  4 +---
 runtime/mirror/class-inl.h |  5 ++++-
 4 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/runtime/class_table-inl.h b/runtime/class_table-inl.h
index 5f8a0b03c8..d07ad33d21 100644
--- a/runtime/class_table-inl.h
+++ b/runtime/class_table-inl.h
@@ -88,6 +88,10 @@ bool ClassTable::Visit(const Visitor& visitor) {
   return true;
 }
 
+inline bool ClassTable::TableSlot::IsNull() const {
+  return Read<kWithoutReadBarrier>() == nullptr;
+}
+
 template<ReadBarrierOption kReadBarrierOption>
 inline mirror::Class* ClassTable::TableSlot::Read() const {
   const uint32_t before = data_.load(std::memory_order_relaxed);
@@ -127,11 +131,7 @@ inline uint32_t ClassTable::TableSlot::Encode(ObjPtr<mirror::Class> klass, uint3
 
 inline ClassTable::TableSlot::TableSlot(ObjPtr<mirror::Class> klass, uint32_t descriptor_hash)
     : data_(Encode(klass, MaskHash(descriptor_hash))) {
-  if (kIsDebugBuild) {
-    std::string temp;
-    const uint32_t hash = ComputeModifiedUtf8Hash(klass->GetDescriptor(&temp));
-    CHECK_EQ(descriptor_hash, hash);
-  }
+  DCHECK_EQ(descriptor_hash, HashDescriptor(klass));
 }
 
 template <typename Filter>
diff --git a/runtime/class_table.cc b/runtime/class_table.cc
index 8d8e93aab0..58f3674804 100644
--- a/runtime/class_table.cc
+++ b/runtime/class_table.cc
@@ -34,15 +34,7 @@ void ClassTable::FreezeSnapshot() {
 }
 
 bool ClassTable::Contains(ObjPtr<mirror::Class> klass) {
-  ReaderMutexLock mu(Thread::Current(), lock_);
-  TableSlot slot(klass);
-  for (ClassSet& class_set : classes_) {
-    auto it = class_set.find(slot);
-    if (it != class_set.end()) {
-      return it->Read() == klass;
-    }
-  }
-  return false;
+  return LookupByDescriptor(klass) == klass;
 }
 
 mirror::Class* ClassTable::LookupByDescriptor(ObjPtr<mirror::Class> klass) {
@@ -191,27 +183,35 @@ bool ClassTable::Remove(const char* descriptor) {
 uint32_t ClassTable::ClassDescriptorHashEquals::operator()(const TableSlot& slot)
     const {
   std::string temp;
-  return ComputeModifiedUtf8Hash(slot.Read()->GetDescriptor(&temp));
+  // No read barrier needed, we're reading a chain of constant references for comparison
+  // with null and retrieval of constant primitive data. See ReadBarrierOption.
+  return ComputeModifiedUtf8Hash(slot.Read<kWithoutReadBarrier>()->GetDescriptor(&temp));
 }
 
 bool ClassTable::ClassDescriptorHashEquals::operator()(const TableSlot& a,
                                                        const TableSlot& b) const {
+  // No read barrier needed, we're reading a chain of constant references for comparison
+  // with null and retrieval of constant primitive data. See ReadBarrierOption.
   if (a.Hash() != b.Hash()) {
     std::string temp;
-    DCHECK(!a.Read()->DescriptorEquals(b.Read()->GetDescriptor(&temp)));
+    DCHECK(!a.Read<kWithoutReadBarrier>()->DescriptorEquals(
+        b.Read<kWithoutReadBarrier>()->GetDescriptor(&temp)));
     return false;
   }
   std::string temp;
-  return a.Read()->DescriptorEquals(b.Read()->GetDescriptor(&temp));
+  return a.Read<kWithoutReadBarrier>()->DescriptorEquals(
+      b.Read<kWithoutReadBarrier>()->GetDescriptor(&temp));
 }
 
 bool ClassTable::ClassDescriptorHashEquals::operator()(const TableSlot& a,
                                                        const DescriptorHashPair& b) const {
+  // No read barrier needed, we're reading a chain of constant references for comparison
+  // with null and retrieval of constant primitive data. See ReadBarrierOption.
   if (!a.MaskedHashEquals(b.second)) {
-    DCHECK(!a.Read()->DescriptorEquals(b.first));
+    DCHECK(!a.Read<kWithoutReadBarrier>()->DescriptorEquals(b.first));
     return false;
   }
-  return a.Read()->DescriptorEquals(b.first);
+  return a.Read<kWithoutReadBarrier>()->DescriptorEquals(b.first);
 }
 
 uint32_t ClassTable::ClassDescriptorHashEquals::operator()(const DescriptorHashPair& pair) const {
diff --git a/runtime/class_table.h b/runtime/class_table.h
index 0b08041dbd..26cd3ec249 100644
--- a/runtime/class_table.h
+++ b/runtime/class_table.h
@@ -64,9 +64,7 @@ class ClassTable {
       return *this;
     }
 
-    bool IsNull() const REQUIRES_SHARED(Locks::mutator_lock_) {
-      return Read<kWithoutReadBarrier>() == nullptr;
-    }
+    bool IsNull() const REQUIRES_SHARED(Locks::mutator_lock_);
 
     uint32_t Hash() const {
       return MaskHash(data_.load(std::memory_order_relaxed));
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 3ee8bfe89b..992c8f3d0e 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -845,7 +845,10 @@ inline bool Class::DescriptorEquals(const char* match) {
       return false;
     }
     ++match;
-    klass = klass->GetComponentType();
+    // No read barrier needed, we're reading a chain of constant references for comparison
+    // with null. Then we follow up below with reading constant references to read constant
+    // primitive data in both proxy and non-proxy paths. See ReadBarrierOption.
+    klass = klass->GetComponentType<kDefaultVerifyFlags, kWithoutReadBarrier>();
   }
   if (klass->IsPrimitive()) {
     return strcmp(Primitive::Descriptor(klass->GetPrimitiveType()), match) == 0;
-- 
cgit v1.2.3-59-g8ed1b