Clean up descriptor and member name comparisons.

Add helper functions for these comparisons to keep comments
related to ordering in one place.

Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Change-Id: I510a1cbbb46a22ddbd39d0530198cd004b476a8d

8 files changed, 43 insertions, 24 deletions

diff --git a/libdexfile/dex/dex_file-inl.h b/libdexfile/dex/dex_file-inl.h
index 2cd3781023..dce8a85217 100644
--- a/libdexfile/dex/dex_file-inl.h
+++ b/libdexfile/dex/dex_file-inl.h
@@ -32,7 +32,22 @@
 
 namespace art {
 
-inline std::string_view StringViewFromUtf16Length(const char* utf8_data, size_t utf16_length) {
+inline int DexFile::CompareDescriptors(std::string_view lhs, std::string_view rhs) {
+  // Note: `std::string_view::compare()` uses lexicographical comparison and treats the `char`
+  // as unsigned; for Modified-UTF-8 without embedded nulls this is consistent with the
+  // `CompareModifiedUtf8ToModifiedUtf8AsUtf16CodePointValues()` ordering.
+  return lhs.compare(rhs);
+}
+
+inline int DexFile::CompareMemberNames(std::string_view lhs, std::string_view rhs) {
+  // Note: `std::string_view::compare()` uses lexicographical comparison and treats the `char`
+  // as unsigned; for Modified-UTF-8 without embedded nulls this is consistent with the
+  // `CompareModifiedUtf8ToModifiedUtf8AsUtf16CodePointValues()` ordering.
+  return lhs.compare(rhs);
+}
+
+inline std::string_view DexFile::StringViewFromUtf16Length(const char* utf8_data,
+                                                           size_t utf16_length) {
   size_t utf8_length = LIKELY(utf8_data[utf16_length] == 0)  // Is ASCII?
                            ? utf16_length
                            : utf16_length + strlen(utf8_data + utf16_length);
diff --git a/libdexfile/dex/dex_file.h b/libdexfile/dex/dex_file.h
index 5d67c6cca4..1069da12c1 100644
--- a/libdexfile/dex/dex_file.h
+++ b/libdexfile/dex/dex_file.h
@@ -904,6 +904,11 @@ class DexFile {
   static inline bool StringEquals(const DexFile* df1, dex::StringIndex sidx1,
                                   const DexFile* df2, dex::StringIndex sidx2);
 
+  static int CompareDescriptors(std::string_view lhs, std::string_view rhs);
+  static int CompareMemberNames(std::string_view lhs, std::string_view rhs);
+
+  static std::string_view StringViewFromUtf16Length(const char* utf8_data, size_t utf16_length);
+
  protected:
   // First Dex format version supporting default methods.
   static constexpr uint32_t kDefaultMethodsVersion = 37;
diff --git a/libdexfile/dex/method_reference.h b/libdexfile/dex/method_reference.h
index 0472f2d492..209ab7a994 100644
--- a/libdexfile/dex/method_reference.h
+++ b/libdexfile/dex/method_reference.h
@@ -56,16 +56,15 @@ struct MethodReferenceValueComparator {
     // Compare the class descriptors first.
     const dex::MethodId& mid1 = mr1.GetMethodId();
     const dex::MethodId& mid2 = mr2.GetMethodId();
-    // Note: `std::string_view::compare()` uses lexicographical comparison and treats the `char`
-    // as unsigned; for Modified-UTF-8 without embedded nulls this is consistent with the
-    // `CompareModifiedUtf8ToModifiedUtf8AsUtf16CodePointValues()` ordering.
-    int descriptor_diff = mr1.dex_file->GetTypeDescriptorView(mid1.class_idx_).compare(
-                              mr2.dex_file->GetTypeDescriptorView(mid2.class_idx_));
+    int descriptor_diff = DexFile::CompareDescriptors(
+        mr1.dex_file->GetTypeDescriptorView(mid1.class_idx_),
+        mr2.dex_file->GetTypeDescriptorView(mid2.class_idx_));
     if (descriptor_diff != 0) {
       return descriptor_diff < 0;
     }
     // Compare names second.
-    int name_diff = strcmp(mr1.dex_file->GetMethodName(mid1), mr2.dex_file->GetMethodName(mid2));
+    int name_diff = DexFile::CompareMemberNames(mr1.dex_file->GetMethodNameView(mid1),
+                                                mr2.dex_file->GetMethodNameView(mid2));
     if (name_diff != 0) {
       return name_diff < 0;
     }
diff --git a/libdexfile/dex/proto_reference.h b/libdexfile/dex/proto_reference.h
index dc9a447e63..a12091cc46 100644
--- a/libdexfile/dex/proto_reference.h
+++ b/libdexfile/dex/proto_reference.h
@@ -62,7 +62,7 @@ struct ProtoReferenceValueComparator {
     // Compare return type first.
     const dex::ProtoId& prid1 = lhs.ProtoId();
     const dex::ProtoId& prid2 = rhs.ProtoId();
-    int return_type_diff = lhs.ReturnType().compare(rhs.ReturnType());
+    int return_type_diff = DexFile::CompareDescriptors(lhs.ReturnType(), rhs.ReturnType());
     if (return_type_diff != 0) {
       return return_type_diff < 0;
     }
@@ -77,7 +77,7 @@ struct ProtoReferenceValueComparator {
       std::string_view r_param = rhs.dex_file->GetTypeDescriptorView(
           rhs.dex_file->GetTypeId(params2->GetTypeItem(i).type_idx_));
 
-      int param_diff = l_param.compare(r_param);
+      int param_diff = DexFile::CompareDescriptors(l_param, r_param);
       if (param_diff != 0) {
         return param_diff < 0;
       }
diff --git a/libdexfile/dex/signature-inl.h b/libdexfile/dex/signature-inl.h
index f2760c3002..27906d3829 100644
--- a/libdexfile/dex/signature-inl.h
+++ b/libdexfile/dex/signature-inl.h
@@ -86,7 +86,7 @@ inline int Signature::Compare(const Signature& rhs) const {
         dex_file_->GetTypeId(proto_id_->return_type_idx_));
     std::string_view rhs_return_type = rhs.dex_file_->GetTypeDescriptorView(
         rhs.dex_file_->GetTypeId(rhs.proto_id_->return_type_idx_));
-    int cmp_result = lhs_return_type.compare(rhs_return_type);
+    int cmp_result = DexFile::CompareDescriptors(lhs_return_type, rhs_return_type);
     if (cmp_result != 0) {
       return cmp_result;
     }
@@ -105,7 +105,7 @@ inline int Signature::Compare(const Signature& rhs) const {
             dex_file_->GetTypeId(lhs_params->GetTypeItem(i - 1u).type_idx_));
         std::string_view rhs_param_type = rhs.dex_file_->GetTypeDescriptorView(
             rhs.dex_file_->GetTypeId(rhs_params->GetTypeItem(i - 1u).type_idx_));
-        int cmp_result = lhs_param_type.compare(rhs_param_type);
+        int cmp_result = DexFile::CompareDescriptors(lhs_param_type, rhs_param_type);
         if (cmp_result != 0) {
           return cmp_result;
         }
diff --git a/libdexfile/dex/type_lookup_table.cc b/libdexfile/dex/type_lookup_table.cc
index fa5352e74c..f4e2313445 100644
--- a/libdexfile/dex/type_lookup_table.cc
+++ b/libdexfile/dex/type_lookup_table.cc
@@ -177,7 +177,7 @@ std::string_view TypeLookupTable::GetStringData(const Entry& entry) const {
   DCHECK(dex_data_begin_ != nullptr);
   const uint8_t* ptr = dex_data_begin_ + entry.GetStringOffset();
   uint32_t utf16_length = DecodeUnsignedLeb128(&ptr);
-  return StringViewFromUtf16Length(reinterpret_cast<const char*>(ptr), utf16_length);
+  return DexFile::StringViewFromUtf16Length(reinterpret_cast<const char*>(ptr), utf16_length);
 }
 
 }  // namespace art
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index c69a6721bc..61be9a0055 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -793,7 +793,7 @@ static std::tuple<bool, ArtMethod*> FindDeclaredClassMethod(ObjPtr<mirror::Class
     // Do not use ArtMethod::GetNameView() to avoid reloading dex file through the same
     // declaring class from different methods and also avoid the runtime method check.
     const dex::MethodId& method_id = get_method_id(mid);
-    return name.compare(dex_file.GetMethodNameView(method_id));
+    return DexFile::CompareMemberNames(name, dex_file.GetMethodNameView(method_id));
   };
   auto signature_cmp = [&](uint32_t mid) REQUIRES_SHARED(Locks::mutator_lock_) ALWAYS_INLINE {
     // Do not use ArtMethod::GetSignature() to avoid reloading dex file through the same
@@ -1054,11 +1054,12 @@ static std::tuple<bool, ArtField*> FindFieldByNameAndType(const DexFile& dex_fil
   };
   auto name_cmp = [&](uint32_t mid) REQUIRES_SHARED(Locks::mutator_lock_) ALWAYS_INLINE {
     const dex::FieldId& field_id = get_field_id(mid);
-    return name.compare(dex_file.GetFieldNameView(field_id));
+    return DexFile::CompareMemberNames(name, dex_file.GetFieldNameView(field_id));
   };
   auto type_cmp = [&](uint32_t mid) REQUIRES_SHARED(Locks::mutator_lock_) ALWAYS_INLINE {
     const dex::FieldId& field_id = get_field_id(mid);
-    return type.compare(dex_file.GetTypeDescriptorView(dex_file.GetTypeId(field_id.type_idx_)));
+    return DexFile::CompareDescriptors(
+        type, dex_file.GetTypeDescriptorView(dex_file.GetTypeId(field_id.type_idx_)));
   };
   auto get_name_idx = [&](uint32_t mid) REQUIRES_SHARED(Locks::mutator_lock_) ALWAYS_INLINE {
     const dex::FieldId& field_id = get_field_id(mid);
diff --git a/runtime/native/java_lang_reflect_Executable.cc b/runtime/native/java_lang_reflect_Executable.cc
index e9b7545926..1440909032 100644
--- a/runtime/native/java_lang_reflect_Executable.cc
+++ b/runtime/native/java_lang_reflect_Executable.cc
@@ -255,6 +255,11 @@ static jint Executable_compareMethodParametersInternal(JNIEnv* env,
   this_method = this_method->GetInterfaceMethodIfProxy(kRuntimePointerSize);
   other_method = other_method->GetInterfaceMethodIfProxy(kRuntimePointerSize);
 
+  // Get dex files early. (`ArtMethod::GetParameterTypeList()` includes `GetDexFile()`,
+  // so the compiler should deduplicate these subexpressions after inlining.)
+  const DexFile* this_dex_file = this_method->GetDexFile();
+  const DexFile* other_dex_file = other_method->GetDexFile();
+
   const dex::TypeList* this_list = this_method->GetParameterTypeList();
   const dex::TypeList* other_list = other_method->GetParameterTypeList();
 
@@ -278,15 +283,9 @@ static jint Executable_compareMethodParametersInternal(JNIEnv* env,
   }
 
   for (int32_t i = 0; i < this_size; ++i) {
-    const std::string_view lhs_data = this_method->GetDexFile()->GetTypeDescriptorView(
-        this_list->GetTypeItem(i).type_idx_);
-    const std::string_view rhs_data = other_method->GetDexFile()->GetTypeDescriptorView(
-        other_list->GetTypeItem(i).type_idx_);
-
-    // Note: `std::string_view::compare()` uses lexicographical comparison and treats the `char`
-    // as unsigned; for Modified-UTF-8 without embedded nulls this is consistent with the
-    // `CompareModifiedUtf8ToModifiedUtf8AsUtf16CodePointValues()` ordering.
-    int cmp = lhs_data.compare(rhs_data);
+    int cmp = DexFile::CompareDescriptors(
+        this_dex_file->GetTypeDescriptorView(this_list->GetTypeItem(i).type_idx_),
+        other_dex_file->GetTypeDescriptorView(other_list->GetTypeItem(i).type_idx_));
     if (cmp != 0) {
       return (cmp < 0) ? -1 : 1;
     }