Rename `kJniTransitionOrInvalid` to `kJniTransition`.

And remove `IndirectReferenceTable::SynchronizedGet()`.

Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Bug: 172332525
Change-Id: Ice599b33f4d494fccc0efd3b3a3574e930088f89

8 files changed, 31 insertions, 43 deletions

diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc
index 2cf1d83485..a228d70166 100644
--- a/runtime/indirect_reference_table.cc
+++ b/runtime/indirect_reference_table.cc
@@ -14,10 +14,10 @@
  * limitations under the License.
  */
 
-#include "base/bit_utils.h"
-#include "base/globals.h"
 #include "indirect_reference_table-inl.h"
 
+#include "base/bit_utils.h"
+#include "base/globals.h"
 #include "base/mutator_locked_dumpable.h"
 #include "base/systrace.h"
 #include "base/utils.h"
@@ -26,6 +26,7 @@
 #include "jni/jni_internal.h"
 #include "mirror/object-inl.h"
 #include "nth_caller_visitor.h"
+#include "object_callbacks.h"
 #include "reference_table.h"
 #include "runtime-inl.h"
 #include "scoped_thread_state_change-inl.h"
@@ -41,10 +42,10 @@ static constexpr bool kDebugIRT = false;
 // Maximum table size we allow.
 static constexpr size_t kMaxTableSizeInBytes = 128 * MB;
 
-const char* GetIndirectRefKindString(const IndirectRefKind& kind) {
+const char* GetIndirectRefKindString(IndirectRefKind kind) {
   switch (kind) {
-    case kJniTransitionOrInvalid:
-      return "JniTransitionOrInvalid";
+    case kJniTransition:
+      return "JniTransition";
     case kLocal:
       return "Local";
     case kGlobal:
@@ -113,15 +114,14 @@ void SmallIrtAllocator::Deallocate(IrtEntry* unneeded) {
   small_irt_freelist_ = unneeded;
 }
 
-IndirectReferenceTable::IndirectReferenceTable(IndirectRefKind desired_kind,
-                                               ResizableCapacity resizable)
+IndirectReferenceTable::IndirectReferenceTable(IndirectRefKind kind, ResizableCapacity resizable)
     : segment_state_(kIRTFirstSegment),
       table_(nullptr),
-      kind_(desired_kind),
+      kind_(kind),
       max_entries_(0u),
       current_num_holes_(0),
       resizable_(resizable) {
-  CHECK_NE(desired_kind, kJniTransitionOrInvalid);
+  CHECK_NE(kind, kJniTransition);
 }
 
 bool IndirectReferenceTable::Initialize(size_t max_count, std::string* error_msg) {
@@ -420,7 +420,7 @@ bool IndirectReferenceTable::Remove(IRTSegmentState previous_state, IndirectRef
   // TODO: We should eagerly check the ref kind against the `kind_` instead of
   // relying on this weak check and postponing the rest until `CheckEntry()` below.
   // Passing the wrong kind shall currently result in misleading warnings.
-  if (GetIndirectRefKind(iref) == kJniTransitionOrInvalid) {
+  if (GetIndirectRefKind(iref) == kJniTransition) {
     auto* self = Thread::Current();
     ScopedObjectAccess soa(self);
     if (self->IsJniTransitionReference(reinterpret_cast<jobject>(iref))) {
diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h
index 545958e91a..30688c854b 100644
--- a/runtime/indirect_reference_table.h
+++ b/runtime/indirect_reference_table.h
@@ -75,8 +75,6 @@ class Object;
 // If there's more than one segment, we don't guarantee that the table will fill completely before
 // we fail due to lack of space. We do ensure that the current segment will pack tightly, which
 // should satisfy JNI requirements (e.g. EnsureLocalCapacity).
-//
-// Only SynchronizedGet is synchronized.
 
 // Indirect reference definition.  This must be interchangeable with JNI's jobject, and it's
 // convenient to let null be null, so we use void*.
@@ -91,16 +89,17 @@ using IndirectRef = void*;
 
 // Indirect reference kind, used as the two low bits of IndirectRef.
 //
-// For convenience these match up with enum jobjectRefType from jni.h.
+// For convenience these match up with enum jobjectRefType from jni.h, except that
+// we use value 0 for JNI transitions instead of marking invalid reference type.
 enum IndirectRefKind {
-  kJniTransitionOrInvalid = 0,  // <<JNI transition frame reference or invalid reference>>
-  kLocal                  = 1,  // <<local reference>>
-  kGlobal                 = 2,  // <<global reference>>
-  kWeakGlobal             = 3,  // <<weak global reference>>
-  kLastKind               = kWeakGlobal
+  kJniTransition = 0,  // <<JNI transition frame reference>>
+  kLocal         = 1,  // <<local reference>>
+  kGlobal        = 2,  // <<global reference>>
+  kWeakGlobal    = 3,  // <<weak global reference>>
+  kLastKind      = kWeakGlobal
 };
 std::ostream& operator<<(std::ostream& os, IndirectRefKind rhs);
-const char* GetIndirectRefKindString(const IndirectRefKind& kind);
+const char* GetIndirectRefKindString(IndirectRefKind kind);
 
 // Table definition.
 //
@@ -256,13 +255,6 @@ class IndirectReferenceTable {
   ObjPtr<mirror::Object> Get(IndirectRef iref) const REQUIRES_SHARED(Locks::mutator_lock_)
       ALWAYS_INLINE;
 
-  // Synchronized get which reads a reference, acquiring a lock if necessary.
-  template<ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
-  ObjPtr<mirror::Object> SynchronizedGet(IndirectRef iref) const
-      REQUIRES_SHARED(Locks::mutator_lock_) {
-    return Get<kReadBarrierOption>(iref);
-  }
-
   // Updates an existing indirect reference to point to a new object.
   void Update(IndirectRef iref, ObjPtr<mirror::Object> obj) REQUIRES_SHARED(Locks::mutator_lock_);
 
@@ -404,10 +396,10 @@ class IndirectReferenceTable {
   // Mem map where we store the indirect refs. If it's invalid, and table_ is non-null, then
   // table_ is valid, but was allocated via allocSmallIRT();
   MemMap table_mem_map_;
-  // bottom of the stack. Do not directly access the object references
+  // Bottom of the stack. Do not directly access the object references
   // in this as they are roots. Use Get() that has a read barrier.
   IrtEntry* table_;
-  // bit mask, ORed into all irefs.
+  // Bit mask, ORed into all irefs.
   const IndirectRefKind kind_;
 
   // max #of entries allowed (modulo resizing).
diff --git a/runtime/jni/check_jni.cc b/runtime/jni/check_jni.cc
index 4c7b1aa306..127d1a3a61 100644
--- a/runtime/jni/check_jni.cc
+++ b/runtime/jni/check_jni.cc
@@ -56,7 +56,7 @@ namespace art {
 // declared as a friend by JniVmExt and JniEnvExt.
 inline IndirectReferenceTable* GetIndirectReferenceTable(ScopedObjectAccess& soa,
                                                          IndirectRefKind kind) {
-  DCHECK_NE(kind, kJniTransitionOrInvalid);
+  DCHECK_NE(kind, kJniTransition);
   JNIEnvExt* env = soa.Env();
   IndirectReferenceTable* irt =
       (kind == kLocal) ? &env->locals_
@@ -723,7 +723,7 @@ class ScopedCheck {
     IndirectRefKind found_kind;
     if (expected_kind == kLocal) {
       found_kind = IndirectReferenceTable::GetIndirectRefKind(obj);
-      if (found_kind == kJniTransitionOrInvalid &&
+      if (found_kind == kJniTransition &&
           obj != nullptr &&
           self->IsJniTransitionReference(obj)) {
         found_kind = kLocal;
@@ -866,7 +866,7 @@ class ScopedCheck {
     bool expect_null = false;
     bool okay = true;
     std::string error_msg;
-    if (ref_kind == kJniTransitionOrInvalid) {
+    if (ref_kind == kJniTransition) {
       if (!soa.Self()->IsJniTransitionReference(java_object)) {
         okay = false;
         error_msg = "use of invalid jobject";
diff --git a/runtime/jni/java_vm_ext.cc b/runtime/jni/java_vm_ext.cc
index 44ff6eb5aa..c55c7be10b 100644
--- a/runtime/jni/java_vm_ext.cc
+++ b/runtime/jni/java_vm_ext.cc
@@ -44,7 +44,6 @@
 #include "nativehelper/scoped_local_ref.h"
 #include "nativehelper/scoped_utf_chars.h"
 #include "nativeloader/native_loader.h"
-#include "object_callbacks.h"
 #include "parsed_options.h"
 #include "runtime-inl.h"
 #include "runtime_options.h"
@@ -832,7 +831,7 @@ void JavaVMExt::BroadcastForNewWeakGlobals() {
 }
 
 ObjPtr<mirror::Object> JavaVMExt::DecodeGlobal(IndirectRef ref) {
-  return globals_.SynchronizedGet(ref);
+  return globals_.Get(ref);
 }
 
 void JavaVMExt::UpdateGlobal(Thread* self, IndirectRef ref, ObjPtr<mirror::Object> result) {
@@ -849,7 +848,7 @@ ObjPtr<mirror::Object> JavaVMExt::DecodeWeakGlobal(Thread* self, IndirectRef ref
   // if MayAccessWeakGlobals is false.
   DCHECK_EQ(IndirectReferenceTable::GetIndirectRefKind(ref), kWeakGlobal);
   if (LIKELY(MayAccessWeakGlobals(self))) {
-    return weak_globals_.SynchronizedGet(ref);
+    return weak_globals_.Get(ref);
   }
   MutexLock mu(self, *Locks::jni_weak_globals_lock_);
   return DecodeWeakGlobalLocked(self, ref);
@@ -877,7 +876,7 @@ ObjPtr<mirror::Object> JavaVMExt::DecodeWeakGlobalDuringShutdown(Thread* self, I
   if (!gUseReadBarrier) {
     DCHECK(allow_accessing_weak_globals_.load(std::memory_order_seq_cst));
   }
-  return weak_globals_.SynchronizedGet(ref);
+  return weak_globals_.Get(ref);
 }
 
 bool JavaVMExt::IsWeakGlobalCleared(Thread* self, IndirectRef ref) {
diff --git a/runtime/jni/java_vm_ext.h b/runtime/jni/java_vm_ext.h
index 8d81af1ed5..8c8b1b3177 100644
--- a/runtime/jni/java_vm_ext.h
+++ b/runtime/jni/java_vm_ext.h
@@ -253,7 +253,6 @@ class JavaVMExt : public JavaVM {
   // Extra diagnostics.
   const std::string trace_;
 
-  // Not guarded by globals_lock since we sometimes use SynchronizedGet in Thread::DecodeJObject.
   IndirectReferenceTable globals_;
 
   // No lock annotation since UnloadNativeLibraries is called on libraries_ but locks the
@@ -265,10 +264,8 @@ class JavaVMExt : public JavaVM {
 
   // Since weak_globals_ contain weak roots, be careful not to
   // directly access the object references in it. Use Get() with the
-  // read barrier enabled.
-  // Not guarded by weak_globals_lock since we may use SynchronizedGet in DecodeWeakGlobal.
+  // read barrier enabled or disabled based on the use case.
   IndirectReferenceTable weak_globals_;
-  // Not guarded by weak_globals_lock since we may use SynchronizedGet in DecodeWeakGlobal.
   Atomic<bool> allow_accessing_weak_globals_;
   ConditionVariable weak_globals_add_condition_ GUARDED_BY(Locks::jni_weak_globals_lock_);
 
diff --git a/runtime/jni/jni_internal.cc b/runtime/jni/jni_internal.cc
index ac449a8806..ea34fd37df 100644
--- a/runtime/jni/jni_internal.cc
+++ b/runtime/jni/jni_internal.cc
@@ -2854,7 +2854,7 @@ class JNI {
       return JNIGlobalRefType;
     case kWeakGlobal:
       return JNIWeakGlobalRefType;
-    case kJniTransitionOrInvalid:
+    case kJniTransition:
       // Assume value is in a JNI transition frame.
       return JNILocalRefType;
     }
diff --git a/runtime/reflection.cc b/runtime/reflection.cc
index 8f31e0bb68..7ddf64d7dd 100644
--- a/runtime/reflection.cc
+++ b/runtime/reflection.cc
@@ -1066,8 +1066,8 @@ void UpdateReference(Thread* self, jobject obj, ObjPtr<mirror::Object> result) {
   IndirectRefKind kind = IndirectReferenceTable::GetIndirectRefKind(ref);
   if (kind == kLocal) {
     self->GetJniEnv()->UpdateLocal(obj, result);
-  } else if (kind == kJniTransitionOrInvalid) {
-    LOG(FATAL) << "Unsupported UpdateReference for kind kJniTransitionOrInvalid";
+  } else if (kind == kJniTransition) {
+    LOG(FATAL) << "Unsupported UpdateReference for kind kJniTransition";
   } else if (kind == kGlobal) {
     self->GetJniEnv()->GetVm()->UpdateGlobal(self, ref, result);
   } else {
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 27493e8c37..4312102b1a 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -2772,7 +2772,7 @@ ObjPtr<mirror::Object> Thread::DecodeJObject(jobject obj) const {
     IndirectReferenceTable& locals = tlsPtr_.jni_env->locals_;
     // Local references do not need a read barrier.
     result = locals.Get<kWithoutReadBarrier>(ref);
-  } else if (kind == kJniTransitionOrInvalid) {
+  } else if (kind == kJniTransition) {
     // The `jclass` for a static method points to the CompressedReference<> in the
     // `ArtMethod::declaring_class_`. Other `jobject` arguments point to spilled stack
     // references but a StackReference<> is just a subclass of CompressedReference<>.