dex_cache: rewrite StringDexCachePair as DexCachePair<T>

.. with [T = mirror::String]. This is in preparation for introducing
a dex cache array for MethodTypes, which will be treated the same way.

Test: make test-art-host
bug: 30550796
Change-Id: Ief4455b4c6e4c9dd897f2c40b14b843a57b1dc8e

diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h
index d5b3090..51e5aae 100644 (file)
--- a/runtime/class_linker-inl.h
+++ b/runtime/class_linker-inl.h
@@ -67,9 +67,9 @@ inline mirror::String* ClassLinker::ResolveString(uint32_t string_idx, ArtMethod
   // MethodVerifier refuses methods with string_idx out of bounds.
   DCHECK_LT(string_idx, declaring_class->GetDexFile().NumStringIds());;
   mirror::String* string =
-        mirror::StringDexCachePair::LookupString(declaring_class->GetDexCacheStrings(),
-                                                 string_idx,
-                                                 mirror::DexCache::kDexCacheStringCacheSize).Read();
+        mirror::StringDexCachePair::Lookup(declaring_class->GetDexCacheStrings(),
+                                           string_idx,
+                                           mirror::DexCache::kDexCacheStringCacheSize).Read();
   Thread::PoisonObjectPointersIfDebug();
   if (UNLIKELY(string == nullptr)) {
     StackHandleScope<1> hs(Thread::Current());

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 5106aec..a874c24 100644 (file)
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1308,8 +1308,8 @@ bool ClassLinker::UpdateAppImageClassLoadersAndDexCaches(
           mirror::StringDexCacheType* const strings =
               reinterpret_cast<mirror::StringDexCacheType*>(raw_arrays + layout.StringsOffset());
           for (size_t j = 0; j < num_strings; ++j) {
-            DCHECK_EQ(strings[j].load(std::memory_order_relaxed).string_index, 0u);
-            DCHECK(strings[j].load(std::memory_order_relaxed).string_pointer.IsNull());
+            DCHECK_EQ(strings[j].load(std::memory_order_relaxed).index, 0u);
+            DCHECK(strings[j].load(std::memory_order_relaxed).object.IsNull());
             strings[j].store(image_resolved_strings[j].load(std::memory_order_relaxed),
                              std::memory_order_relaxed);
           }
@@ -2127,8 +2127,8 @@ mirror::DexCache* ClassLinker::AllocDexCache(Thread* self,
   if (kIsDebugBuild) {
     // Sanity check to make sure all the dex cache arrays are empty. b/28992179
     for (size_t i = 0; i < num_strings; ++i) {
-      CHECK_EQ(strings[i].load(std::memory_order_relaxed).string_index, 0u);
-      CHECK(strings[i].load(std::memory_order_relaxed).string_pointer.IsNull());
+      CHECK_EQ(strings[i].load(std::memory_order_relaxed).index, 0u);
+      CHECK(strings[i].load(std::memory_order_relaxed).object.IsNull());
     }
     for (size_t i = 0; i < dex_file.NumTypeIds(); ++i) {
       CHECK(types[i].Read<kWithoutReadBarrier>() == nullptr);

diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index 814adf7..0feb013 100644 (file)
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -254,9 +254,9 @@ static inline String* ResolveString(Thread* self, ShadowFrame& shadow_frame, uin
   DCHECK_LT(string_idx % mirror::DexCache::kDexCacheStringCacheSize,
             declaring_class->GetDexFile().NumStringIds());
   mirror::String* string_ptr =
-      mirror::StringDexCachePair::LookupString(declaring_class->GetDexCacheStrings(),
-                                               string_idx,
-                                               mirror::DexCache::kDexCacheStringCacheSize).Read();
+      mirror::StringDexCachePair::Lookup(declaring_class->GetDexCacheStrings(),
+                                         string_idx,
+                                         mirror::DexCache::kDexCacheStringCacheSize).Read();
   if (UNLIKELY(string_ptr == nullptr)) {
     StackHandleScope<1> hs(self);
     Handle<mirror::DexCache> dex_cache(hs.NewHandle(declaring_class->GetDexCache()));

diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index 548087e..6c1259b 100644 (file)
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -54,9 +54,10 @@ class Constructor;
 class DexCache;
 class IfTable;
 class Method;
-struct StringDexCachePair;
+template <typename T> struct PACKED(8) DexCachePair;
 
-using StringDexCacheType = std::atomic<mirror::StringDexCachePair>;
+using StringDexCachePair = DexCachePair<mirror::String>;
+using StringDexCacheType = std::atomic<StringDexCachePair>;
 
 // C++ mirror of java.lang.Class
 class MANAGED Class FINAL : public Object {

diff --git a/runtime/mirror/dex_cache-inl.h b/runtime/mirror/dex_cache-inl.h
index 220979a..359462d 100644 (file)
--- a/runtime/mirror/dex_cache-inl.h
+++ b/runtime/mirror/dex_cache-inl.h
@@ -39,7 +39,7 @@ inline uint32_t DexCache::ClassSize(PointerSize pointer_size) {
 
 inline mirror::String* DexCache::GetResolvedString(uint32_t string_idx) {
   DCHECK_LT(string_idx, GetDexFile()->NumStringIds());
-  return StringDexCachePair::LookupString(GetStrings(), string_idx, NumStrings()).Read();
+  return StringDexCachePair::Lookup(GetStrings(), string_idx, NumStrings()).Read();
 }
 
 inline void DexCache::SetResolvedString(uint32_t string_idx, mirror::String* resolved) {
@@ -61,10 +61,10 @@ inline void DexCache::ClearString(uint32_t string_idx) {
   DCHECK(Runtime::Current()->IsAotCompiler());
   StringDexCacheType* slot = &GetStrings()[slot_idx];
   // This is racy but should only be called from the transactional interpreter.
-  if (slot->load(std::memory_order_relaxed).string_index == string_idx) {
+  if (slot->load(std::memory_order_relaxed).index == string_idx) {
     StringDexCachePair cleared(
         nullptr,
-        StringDexCachePair::InvalidStringIndexForSlot(slot_idx));
+        StringDexCachePair::InvalidIndexForSlot(slot_idx));
     slot->store(cleared, std::memory_order_relaxed);
   }
 }
@@ -155,11 +155,11 @@ inline void DexCache::VisitReferences(mirror::Class* klass, const Visitor& visit
     mirror::StringDexCacheType* strings = GetStrings();
     for (size_t i = 0, num_strings = NumStrings(); i != num_strings; ++i) {
       StringDexCachePair source = strings[i].load(std::memory_order_relaxed);
-      mirror::String* before = source.string_pointer.Read<kReadBarrierOption>();
+      mirror::String* before = source.object.Read<kReadBarrierOption>();
       GcRoot<mirror::String> root(before);
       visitor.VisitRootIfNonNull(root.AddressWithoutBarrier());
       if (root.Read() != before) {
-        source.string_pointer = GcRoot<String>(root.Read());
+        source.object = GcRoot<String>(root.Read());
         strings[i].store(source, std::memory_order_relaxed);
       }
     }
@@ -175,9 +175,9 @@ inline void DexCache::FixupStrings(mirror::StringDexCacheType* dest, const Visit
   mirror::StringDexCacheType* src = GetStrings();
   for (size_t i = 0, count = NumStrings(); i < count; ++i) {
     StringDexCachePair source = src[i].load(std::memory_order_relaxed);
-    mirror::String* ptr = source.string_pointer.Read<kReadBarrierOption>();
+    mirror::String* ptr = source.object.Read<kReadBarrierOption>();
     mirror::String* new_source = visitor(ptr);
-    source.string_pointer = GcRoot<String>(new_source);
+    source.object = GcRoot<String>(new_source);
     dest[i].store(source, std::memory_order_relaxed);
   }
 }

diff --git a/runtime/mirror/dex_cache.h b/runtime/mirror/dex_cache.h
index 7d4021f..d81dedc 100644 (file)
--- a/runtime/mirror/dex_cache.h
+++ b/runtime/mirror/dex_cache.h
@@ -35,9 +35,9 @@ namespace mirror {
 
 class String;
 
-struct PACKED(8) StringDexCachePair {
-  GcRoot<String> string_pointer;
-  uint32_t string_index;
+template <typename T> struct PACKED(8) DexCachePair {
+  GcRoot<T> object;
+  uint32_t index;
   // The array is initially [ {0,0}, {0,0}, {0,0} ... ]
   // We maintain the invariant that once a dex cache entry is populated,
   // the pointer is always non-0
@@ -45,47 +45,51 @@ struct PACKED(8) StringDexCachePair {
   // {non-0, non-0} OR {0,0}
   //
   // It's generally sufficiently enough then to check if the
-  // lookup string index matches the stored string index (for a >0 string index)
+  // lookup index matches the stored index (for a >0 lookup index)
   // because if it's true the pointer is also non-null.
   //
   // For the 0th entry which is a special case, the value is either
   // {0,0} (initial state) or {non-0, 0} which indicates
-  // that a valid string is stored at that index for a dex string id of 0.
+  // that a valid object is stored at that index for a dex section id of 0.
   //
-  // As an optimization, we want to avoid branching on the string pointer since
-  // it's always non-null if the string id branch succeeds (except for the 0th string id).
+  // As an optimization, we want to avoid branching on the object pointer since
+  // it's always non-null if the id branch succeeds (except for the 0th id).
   // Set the initial state for the 0th entry to be {0,1} which is guaranteed to fail
-  // the lookup string id == stored id branch.
-  StringDexCachePair(String* string, uint32_t string_idx)
-      : string_pointer(string),
-        string_index(string_idx) {}
-  StringDexCachePair() = default;
-  StringDexCachePair(const StringDexCachePair&) = default;
-  StringDexCachePair& operator=(const StringDexCachePair&) = default;
-
-  static void Initialize(StringDexCacheType* strings) {
-    mirror::StringDexCachePair first_elem;
-    first_elem.string_pointer = GcRoot<String>(nullptr);
-    first_elem.string_index = InvalidStringIndexForSlot(0);
-    strings[0].store(first_elem, std::memory_order_relaxed);
+  // the lookup id == stored id branch.
+  DexCachePair(T* object, uint32_t index)
+      : object(object),
+        index(index) {}
+  DexCachePair() = default;
+  DexCachePair(const DexCachePair<T>&) = default;
+  DexCachePair& operator=(const DexCachePair<T>&) = default;
+
+  static void Initialize(std::atomic<DexCachePair<T>>* dex_cache) {
+    DexCachePair<T> first_elem;
+    first_elem.object = GcRoot<T>(nullptr);
+    first_elem.index = InvalidIndexForSlot(0);
+    dex_cache[0].store(first_elem, std::memory_order_relaxed);
   }
 
-  static GcRoot<String> LookupString(StringDexCacheType* dex_cache,
-                                     uint32_t string_idx,
-                                     uint32_t cache_size) {
-    StringDexCachePair index_string = dex_cache[string_idx % cache_size]
-        .load(std::memory_order_relaxed);
-    if (string_idx != index_string.string_index) return GcRoot<String>(nullptr);
-    DCHECK(!index_string.string_pointer.IsNull());
-    return index_string.string_pointer;
+  static GcRoot<T> Lookup(std::atomic<DexCachePair<T>>* dex_cache,
+                          uint32_t idx,
+                          uint32_t cache_size) {
+    DexCachePair<T> element = dex_cache[idx % cache_size].load(std::memory_order_relaxed);
+    if (idx != element.index) {
+      return GcRoot<T>(nullptr);
+    }
+
+    DCHECK(!element.object.IsNull());
+    return element.object;
   }
 
-  static uint32_t InvalidStringIndexForSlot(uint32_t slot) {
+  static uint32_t InvalidIndexForSlot(uint32_t slot) {
     // Since the cache size is a power of two, 0 will always map to slot 0.
     // Use 1 for slot 0 and 0 for all other slots.
     return (slot == 0) ? 1u : 0u;
   }
 };
+
+using StringDexCachePair = DexCachePair<mirror::String>;
 using StringDexCacheType = std::atomic<StringDexCachePair>;
 
 // C++ mirror of java.lang.DexCache.