Clean up `IndirectReferenceTable` visiting.

Move weak global sweeping to the IRT class and clean up the
entry visiting to avoid the need for the `IrtIterator`.
Move `JNIEnvExt::UpdateLocal` to the `jni_env_ext-inl.h`
because it requires `indirect_reference_table-inl.h`.

Test: m test-art-host-gtest
Test: tesrunner.py --host --optimizing
Bug: 172332525
Change-Id: Ia96d31e0d8d701c93d7752bf76f633719a836f3f

6 files changed, 36 insertions, 68 deletions

diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc
index 1e1adf0755..2cf1d83485 100644
--- a/runtime/indirect_reference_table.cc
+++ b/runtime/indirect_reference_table.cc
@@ -27,7 +27,7 @@
 #include "mirror/object-inl.h"
 #include "nth_caller_visitor.h"
 #include "reference_table.h"
-#include "runtime.h"
+#include "runtime-inl.h"
 #include "scoped_thread_state_change-inl.h"
 #include "thread.h"
 
@@ -528,7 +528,8 @@ void IndirectReferenceTable::Trim() {
 
 void IndirectReferenceTable::VisitRoots(RootVisitor* visitor, const RootInfo& root_info) {
   BufferedRootVisitor<kDefaultBufferedRootCount> root_visitor(visitor, root_info);
-  for (auto ref : *this) {
+  for (size_t i = 0, capacity = Capacity(); i != capacity; ++i) {
+    GcRoot<mirror::Object>* ref = table_[i].GetReference();
     if (!ref->IsNull()) {
       root_visitor.VisitRoot(*ref);
       DCHECK(!ref->IsNull());
@@ -536,6 +537,24 @@ void IndirectReferenceTable::VisitRoots(RootVisitor* visitor, const RootInfo& ro
   }
 }
 
+void IndirectReferenceTable::SweepJniWeakGlobals(IsMarkedVisitor* visitor) {
+  CHECK_EQ(kind_, kWeakGlobal);
+  MutexLock mu(Thread::Current(), *Locks::jni_weak_globals_lock_);
+  Runtime* const runtime = Runtime::Current();
+  for (size_t i = 0, capacity = Capacity(); i != capacity; ++i) {
+    GcRoot<mirror::Object>* entry = table_[i].GetReference();
+    // Need to skip null here to distinguish between null entries and cleared weak ref entries.
+    if (!entry->IsNull()) {
+      mirror::Object* obj = entry->Read<kWithoutReadBarrier>();
+      mirror::Object* new_obj = visitor->IsMarked(obj);
+      if (new_obj == nullptr) {
+        new_obj = runtime->GetClearedJniWeakGlobal();
+      }
+      *entry = GcRoot<mirror::Object>(new_obj);
+    }
+  }
+}
+
 void IndirectReferenceTable::Dump(std::ostream& os) const {
   os << kind_ << " table dump:\n";
   ReferenceTable::Table entries;
diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h
index 18d07307e1..545958e91a 100644
--- a/runtime/indirect_reference_table.h
+++ b/runtime/indirect_reference_table.h
@@ -37,6 +37,7 @@
 
 namespace art {
 
+class IsMarkedVisitor;
 class RootInfo;
 
 namespace mirror {
@@ -181,42 +182,6 @@ class IrtEntry {
 static_assert(sizeof(IrtEntry) == 2 * sizeof(uint32_t), "Unexpected sizeof(IrtEntry)");
 static_assert(IsPowerOfTwo(sizeof(IrtEntry)), "Unexpected sizeof(IrtEntry)");
 
-class IrtIterator {
- public:
-  IrtIterator(IrtEntry* table, size_t i, size_t capacity) REQUIRES_SHARED(Locks::mutator_lock_)
-      : table_(table), i_(i), capacity_(capacity) {
-    // capacity_ is used in some target; has warning with unused attribute.
-    UNUSED(capacity_);
-  }
-
-  IrtIterator& operator++() REQUIRES_SHARED(Locks::mutator_lock_) {
-    ++i_;
-    return *this;
-  }
-
-  GcRoot<mirror::Object>* operator*() REQUIRES_SHARED(Locks::mutator_lock_) {
-    // This does not have a read barrier as this is used to visit roots.
-    return table_[i_].GetReference();
-  }
-
-  bool equals(const IrtIterator& rhs) const {
-    return (i_ == rhs.i_ && table_ == rhs.table_);
-  }
-
- private:
-  IrtEntry* const table_;
-  size_t i_;
-  const size_t capacity_;
-};
-
-bool inline operator==(const IrtIterator& lhs, const IrtIterator& rhs) {
-  return lhs.equals(rhs);
-}
-
-bool inline operator!=(const IrtIterator& lhs, const IrtIterator& rhs) {
-  return !lhs.equals(rhs);
-}
-
 // We initially allocate local reference tables with a very small number of entries, packing
 // multiple tables into a single page. If we need to expand one, we allocate them in units of
 // pages.
@@ -340,15 +305,6 @@ class IndirectReferenceTable {
   // without recovering holes. Thus this is a conservative estimate.
   size_t FreeCapacity() const;
 
-  // Note IrtIterator does not have a read barrier as it's used to visit roots.
-  IrtIterator begin() {
-    return IrtIterator(table_, 0, Capacity());
-  }
-
-  IrtIterator end() {
-    return IrtIterator(table_, Capacity(), Capacity());
-  }
-
   void VisitRoots(RootVisitor* visitor, const RootInfo& root_info)
       REQUIRES_SHARED(Locks::mutator_lock_);
 
@@ -377,6 +333,10 @@ class IndirectReferenceTable {
   bool IsValidReference(IndirectRef, /*out*/std::string* error_msg) const
       REQUIRES_SHARED(Locks::mutator_lock_);
 
+  void SweepJniWeakGlobals(IsMarkedVisitor* visitor)
+      REQUIRES_SHARED(Locks::mutator_lock_)
+      REQUIRES(!Locks::jni_weak_globals_lock_);
+
  private:
   static constexpr uint32_t kShiftedSerialMask = (1u << kIRTSerialBits) - 1;
 
diff --git a/runtime/jni/java_vm_ext.cc b/runtime/jni/java_vm_ext.cc
index 248335a78a..f28532b001 100644
--- a/runtime/jni/java_vm_ext.cc
+++ b/runtime/jni/java_vm_ext.cc
@@ -1169,23 +1169,6 @@ void* JavaVMExt::FindCodeForNativeMethod(ArtMethod* m, std::string* error_msg, b
   return native_method;
 }
 
-void JavaVMExt::SweepJniWeakGlobals(IsMarkedVisitor* visitor) {
-  MutexLock mu(Thread::Current(), *Locks::jni_weak_globals_lock_);
-  Runtime* const runtime = Runtime::Current();
-  for (auto* entry : weak_globals_) {
-    // Need to skip null here to distinguish between null entries and cleared weak ref entries.
-    if (!entry->IsNull()) {
-      // Since this is called by the GC, we don't need a read barrier.
-      mirror::Object* obj = entry->Read<kWithoutReadBarrier>();
-      mirror::Object* new_obj = visitor->IsMarked(obj);
-      if (new_obj == nullptr) {
-        new_obj = runtime->GetClearedJniWeakGlobal();
-      }
-      *entry = GcRoot<mirror::Object>(new_obj);
-    }
-  }
-}
-
 void JavaVMExt::TrimGlobals() {
   WriterMutexLock mu(Thread::Current(), *Locks::jni_globals_lock_);
   globals_.Trim();
diff --git a/runtime/jni/java_vm_ext.h b/runtime/jni/java_vm_ext.h
index b97088aaf7..8d81af1ed5 100644
--- a/runtime/jni/java_vm_ext.h
+++ b/runtime/jni/java_vm_ext.h
@@ -165,7 +165,9 @@ class JavaVMExt : public JavaVM {
 
   void SweepJniWeakGlobals(IsMarkedVisitor* visitor)
       REQUIRES_SHARED(Locks::mutator_lock_)
-      REQUIRES(!Locks::jni_weak_globals_lock_);
+      REQUIRES(!Locks::jni_weak_globals_lock_) {
+    weak_globals_.SweepJniWeakGlobals(visitor);
+  }
 
   ObjPtr<mirror::Object> DecodeGlobal(IndirectRef ref)
       REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/jni/jni_env_ext-inl.h b/runtime/jni/jni_env_ext-inl.h
index 7609a9e01a..d66ac1a160 100644
--- a/runtime/jni/jni_env_ext-inl.h
+++ b/runtime/jni/jni_env_ext-inl.h
@@ -19,6 +19,7 @@
 
 #include "jni_env_ext.h"
 
+#include "indirect_reference_table-inl.h"
 #include "mirror/object.h"
 
 namespace art {
@@ -49,6 +50,10 @@ inline T JNIEnvExt::AddLocalReference(ObjPtr<mirror::Object> obj) {
   return reinterpret_cast<T>(ref);
 }
 
+inline void JNIEnvExt::UpdateLocal(IndirectRef iref, ObjPtr<mirror::Object> obj) {
+  locals_.Update(iref, obj);
+}
+
 }  // namespace art
 
 #endif  // ART_RUNTIME_JNI_JNI_ENV_EXT_INL_H_
diff --git a/runtime/jni/jni_env_ext.h b/runtime/jni/jni_env_ext.h
index 5952b3b68d..3614213b60 100644
--- a/runtime/jni/jni_env_ext.h
+++ b/runtime/jni/jni_env_ext.h
@@ -63,9 +63,8 @@ class JNIEnvExt : public JNIEnv {
       REQUIRES_SHARED(Locks::mutator_lock_)
       REQUIRES(!Locks::alloc_tracker_lock_);
 
-  void UpdateLocal(IndirectRef iref, ObjPtr<mirror::Object> obj) REQUIRES_SHARED(Locks::mutator_lock_) {
-    locals_.Update(iref, obj);
-  }
+  void UpdateLocal(IndirectRef iref, ObjPtr<mirror::Object> obj)
+      REQUIRES_SHARED(Locks::mutator_lock_);
 
   jobject NewLocalRef(mirror::Object* obj) REQUIRES_SHARED(Locks::mutator_lock_);
   void DeleteLocalRef(jobject obj) REQUIRES_SHARED(Locks::mutator_lock_);