Refactor String resolution.
Use the same pattern as type resolution and avoid some
unnecessary read barriers in the fast path. Consolidate
naming between ArtField and ArtMethod.
Test: m test-art-host-gtest
Test: testrunner.py --host
Change-Id: Iea69129085f61f04a4add09edd0eadbb7ac9ecb2
diff --git a/runtime/art_field-inl.h b/runtime/art_field-inl.h
index baa5102..c5fb7d5 100644
--- a/runtime/art_field-inl.h
+++ b/runtime/art_field-inl.h
@@ -21,7 +21,7 @@
#include <android-base/logging.h>
-#include "class_linker.h"
+#include "class_linker-inl.h"
#include "dex/dex_file-inl.h"
#include "dex/primitive.h"
#include "gc/accounting/card_table-inl.h"
@@ -339,16 +339,11 @@
return GetDexCache<kWithoutReadBarrier>()->GetDexFile();
}
-inline ObjPtr<mirror::String> ArtField::GetStringName(Thread* self, bool resolve) {
- auto dex_field_index = GetDexFieldIndex();
+inline ObjPtr<mirror::String> ArtField::ResolveNameString() {
+ uint32_t dex_field_index = GetDexFieldIndex();
CHECK_NE(dex_field_index, dex::kDexNoIndex);
- ObjPtr<mirror::DexCache> dex_cache = GetDexCache();
- const DexFile::FieldId& field_id = dex_cache->GetDexFile()->GetFieldId(dex_field_index);
- ObjPtr<mirror::String> name = dex_cache->GetResolvedString(field_id.name_idx_);
- if (resolve && name == nullptr) {
- name = ResolveGetStringName(self, field_id.name_idx_, dex_cache);
- }
- return name;
+ const DexFile::FieldId& field_id = GetDexFile()->GetFieldId(dex_field_index);
+ return Runtime::Current()->GetClassLinker()->ResolveString(field_id.name_idx_, this);
}
template <typename Visitor>
diff --git a/runtime/art_field.cc b/runtime/art_field.cc
index b867621..6cbd9e4 100644
--- a/runtime/art_field.cc
+++ b/runtime/art_field.cc
@@ -52,13 +52,6 @@
return klass;
}
-ObjPtr<mirror::String> ArtField::ResolveGetStringName(Thread* self,
- dex::StringIndex string_idx,
- ObjPtr<mirror::DexCache> dex_cache) {
- StackHandleScope<1> hs(self);
- return Runtime::Current()->GetClassLinker()->ResolveString(string_idx, hs.NewHandle(dex_cache));
-}
-
std::string ArtField::PrettyField(ArtField* f, bool with_type) {
if (f == nullptr) {
return "null";
diff --git a/runtime/art_field.h b/runtime/art_field.h
index 784a862..123595c 100644
--- a/runtime/art_field.h
+++ b/runtime/art_field.h
@@ -201,8 +201,7 @@
const char* GetName() REQUIRES_SHARED(Locks::mutator_lock_);
// Resolves / returns the name from the dex cache.
- ObjPtr<mirror::String> GetStringName(Thread* self, bool resolve)
- REQUIRES_SHARED(Locks::mutator_lock_);
+ ObjPtr<mirror::String> ResolveNameString() REQUIRES_SHARED(Locks::mutator_lock_);
const char* GetTypeDescriptor() REQUIRES_SHARED(Locks::mutator_lock_);
@@ -241,10 +240,6 @@
ObjPtr<mirror::Class> ProxyFindSystemClass(const char* descriptor)
REQUIRES_SHARED(Locks::mutator_lock_);
- ObjPtr<mirror::String> ResolveGetStringName(Thread* self,
- dex::StringIndex string_idx,
- ObjPtr<mirror::DexCache> dex_cache)
- REQUIRES_SHARED(Locks::mutator_lock_);
void GetAccessFlagsDCheck() REQUIRES_SHARED(Locks::mutator_lock_);
void GetOffsetDCheck() REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h
index ec66966..18595cf 100644
--- a/runtime/art_method-inl.h
+++ b/runtime/art_method-inl.h
@@ -245,6 +245,12 @@
}
}
+inline ObjPtr<mirror::String> ArtMethod::ResolveNameString() {
+ DCHECK(!IsProxyMethod());
+ const DexFile::MethodId& method_id = GetDexFile()->GetMethodId(GetDexMethodIndex());
+ return Runtime::Current()->GetClassLinker()->ResolveString(method_id.name_idx_, this);
+}
+
inline const DexFile::CodeItem* ArtMethod::GetCodeItem() {
return GetDexFile()->GetCodeItem(GetCodeItemOffset());
}
diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index 151c36f..af7881d 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -142,16 +142,6 @@
return dex_file->GetIndexForClassDef(*class_def);
}
-ObjPtr<mirror::String> ArtMethod::GetNameAsString(Thread* self) {
- CHECK(!IsProxyMethod());
- StackHandleScope<1> hs(self);
- Handle<mirror::DexCache> dex_cache(hs.NewHandle(GetDexCache()));
- auto* dex_file = dex_cache->GetDexFile();
- uint32_t dex_method_idx = GetDexMethodIndex();
- const DexFile::MethodId& method_id = dex_file->GetMethodId(dex_method_idx);
- return Runtime::Current()->GetClassLinker()->ResolveString(method_id.name_idx_, dex_cache);
-}
-
void ArtMethod::ThrowInvocationTimeError() {
DCHECK(!IsInvokable());
// NOTE: IsDefaultConflicting must be first since the actual method might or might not be abstract
diff --git a/runtime/art_method.h b/runtime/art_method.h
index 012d706..09debb0 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -589,7 +589,7 @@
ALWAYS_INLINE const char* GetName() REQUIRES_SHARED(Locks::mutator_lock_);
- ObjPtr<mirror::String> GetNameAsString(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_);
+ ObjPtr<mirror::String> ResolveNameString() REQUIRES_SHARED(Locks::mutator_lock_);
const DexFile::CodeItem* GetCodeItem() REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h
index 664b917..2536b23 100644
--- a/runtime/class_linker-inl.h
+++ b/runtime/class_linker-inl.h
@@ -61,6 +61,54 @@
return array_class;
}
+inline ObjPtr<mirror::String> ClassLinker::ResolveString(dex::StringIndex string_idx,
+ ArtField* referrer) {
+ Thread::PoisonObjectPointersIfDebug();
+ DCHECK(!Thread::Current()->IsExceptionPending());
+ // We do not need the read barrier for getting the DexCache for the initial resolved type
+ // lookup as both from-space and to-space copies point to the same native resolved types array.
+ ObjPtr<mirror::String> resolved =
+ referrer->GetDexCache<kWithoutReadBarrier>()->GetResolvedString(string_idx);
+ if (resolved == nullptr) {
+ resolved = DoResolveString(string_idx, referrer->GetDexCache());
+ }
+ return resolved;
+}
+
+inline ObjPtr<mirror::String> ClassLinker::ResolveString(dex::StringIndex string_idx,
+ ArtMethod* referrer) {
+ Thread::PoisonObjectPointersIfDebug();
+ DCHECK(!Thread::Current()->IsExceptionPending());
+ // We do not need the read barrier for getting the DexCache for the initial resolved type
+ // lookup as both from-space and to-space copies point to the same native resolved types array.
+ ObjPtr<mirror::String> resolved =
+ referrer->GetDexCache<kWithoutReadBarrier>()->GetResolvedString(string_idx);
+ if (resolved == nullptr) {
+ resolved = DoResolveString(string_idx, referrer->GetDexCache());
+ }
+ return resolved;
+}
+
+inline ObjPtr<mirror::String> ClassLinker::ResolveString(dex::StringIndex string_idx,
+ Handle<mirror::DexCache> dex_cache) {
+ Thread::PoisonObjectPointersIfDebug();
+ DCHECK(!Thread::Current()->IsExceptionPending());
+ ObjPtr<mirror::String> resolved = dex_cache->GetResolvedString(string_idx);
+ if (resolved == nullptr) {
+ resolved = DoResolveString(string_idx, dex_cache);
+ }
+ return resolved;
+}
+
+inline ObjPtr<mirror::String> ClassLinker::LookupString(dex::StringIndex string_idx,
+ ObjPtr<mirror::DexCache> dex_cache) {
+ ObjPtr<mirror::String> resolved = dex_cache->GetResolvedString(string_idx);
+ if (resolved == nullptr) {
+ resolved = DoLookupString(string_idx, dex_cache);
+ }
+ return resolved;
+}
+
inline ObjPtr<mirror::Class> ClassLinker::ResolveType(dex::TypeIndex type_idx,
ObjPtr<mirror::Class> referrer) {
if (kObjPtrPoisoning) {
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 6798796..526c685 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -7665,14 +7665,15 @@
klass->SetReferenceInstanceOffsets(reference_offsets);
}
-ObjPtr<mirror::String> ClassLinker::ResolveString(dex::StringIndex string_idx,
- Handle<mirror::DexCache> dex_cache) {
- DCHECK(dex_cache != nullptr);
- Thread::PoisonObjectPointersIfDebug();
- ObjPtr<mirror::String> resolved = dex_cache->GetResolvedString(string_idx);
- if (resolved != nullptr) {
- return resolved;
- }
+ObjPtr<mirror::String> ClassLinker::DoResolveString(dex::StringIndex string_idx,
+ ObjPtr<mirror::DexCache> dex_cache) {
+ StackHandleScope<1> hs(Thread::Current());
+ Handle<mirror::DexCache> h_dex_cache(hs.NewHandle(dex_cache));
+ return DoResolveString(string_idx, h_dex_cache);
+}
+
+ObjPtr<mirror::String> ClassLinker::DoResolveString(dex::StringIndex string_idx,
+ Handle<mirror::DexCache> dex_cache) {
const DexFile& dex_file = *dex_cache->GetDexFile();
uint32_t utf16_length;
const char* utf8_data = dex_file.StringDataAndUtf16LengthByIdx(string_idx, &utf16_length);
@@ -7683,13 +7684,9 @@
return string;
}
-ObjPtr<mirror::String> ClassLinker::LookupString(dex::StringIndex string_idx,
- ObjPtr<mirror::DexCache> dex_cache) {
+ObjPtr<mirror::String> ClassLinker::DoLookupString(dex::StringIndex string_idx,
+ ObjPtr<mirror::DexCache> dex_cache) {
DCHECK(dex_cache != nullptr);
- ObjPtr<mirror::String> resolved = dex_cache->GetResolvedString(string_idx);
- if (resolved != nullptr) {
- return resolved;
- }
const DexFile& dex_file = *dex_cache->GetDexFile();
uint32_t utf16_length;
const char* utf8_data = dex_file.StringDataAndUtf16LengthByIdx(string_idx, &utf16_length);
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 58ce6eb..85817ac 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -203,6 +203,16 @@
REQUIRES(!Locks::classlinker_classes_lock_)
REQUIRES_SHARED(Locks::mutator_lock_);
+ // Resolve a String with the given index from the DexFile associated with the given `referrer`,
+ // storing the result in the DexCache. The `referrer` is used to identify the target DexCache
+ // to use for resolution.
+ ObjPtr<mirror::String> ResolveString(dex::StringIndex string_idx,
+ ArtField* referrer)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+ ObjPtr<mirror::String> ResolveString(dex::StringIndex string_idx,
+ ArtMethod* referrer)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
// Resolve a String with the given index from the DexFile associated with the given DexCache,
// storing the result in the DexCache.
ObjPtr<mirror::String> ResolveString(dex::StringIndex string_idx,
@@ -885,6 +895,19 @@
ObjPtr<mirror::ClassLoader> class_loader)
REQUIRES_SHARED(Locks::mutator_lock_);
+ // Implementation of ResolveString() called when the string was not found in the dex cache.
+ ObjPtr<mirror::String> DoResolveString(dex::StringIndex string_idx,
+ ObjPtr<mirror::DexCache> dex_cache)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+ ObjPtr<mirror::String> DoResolveString(dex::StringIndex string_idx,
+ Handle<mirror::DexCache> dex_cache)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
+ // Implementation of LookupString() called when the string was not found in the dex cache.
+ ObjPtr<mirror::String> DoLookupString(dex::StringIndex string_idx,
+ ObjPtr<mirror::DexCache> dex_cache)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
// Implementation of ResolveType() called when the type was not found in the dex cache.
ObjPtr<mirror::Class> DoResolveType(dex::TypeIndex type_idx,
ObjPtr<mirror::Class> referrer)
diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index c533f9c..022857a 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -743,33 +743,6 @@
return h_class.Get();
}
-static inline ObjPtr<mirror::String> ResolveString(ClassLinker* class_linker,
- dex::StringIndex string_idx,
- ArtMethod* referrer)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- Thread::PoisonObjectPointersIfDebug();
- ObjPtr<mirror::String> string = referrer->GetDexCache()->GetResolvedString(string_idx);
- if (UNLIKELY(string == nullptr)) {
- StackHandleScope<1> hs(Thread::Current());
- Handle<mirror::DexCache> dex_cache(hs.NewHandle(referrer->GetDexCache()));
- string = class_linker->ResolveString(string_idx, dex_cache);
- }
- return string;
-}
-
-inline ObjPtr<mirror::String> ResolveStringFromCode(ArtMethod* referrer,
- dex::StringIndex string_idx) {
- Thread::PoisonObjectPointersIfDebug();
- ObjPtr<mirror::String> string = referrer->GetDexCache()->GetResolvedString(string_idx);
- if (UNLIKELY(string == nullptr)) {
- StackHandleScope<1> hs(Thread::Current());
- Handle<mirror::DexCache> dex_cache(hs.NewHandle(referrer->GetDexCache()));
- ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
- string = class_linker->ResolveString(string_idx, dex_cache);
- }
- return string;
-}
-
inline void UnlockJniSynchronizedMethod(jobject locked, Thread* self) {
// Save any pending exception over monitor exit call.
mirror::Throwable* saved_exception = nullptr;
diff --git a/runtime/entrypoints/entrypoint_utils.h b/runtime/entrypoints/entrypoint_utils.h
index 1f4475f..9d70b03 100644
--- a/runtime/entrypoints/entrypoint_utils.h
+++ b/runtime/entrypoints/entrypoint_utils.h
@@ -162,11 +162,6 @@
REQUIRES_SHARED(Locks::mutator_lock_)
REQUIRES(!Roles::uninterruptible_);
-inline ObjPtr<mirror::String> ResolveStringFromCode(ArtMethod* referrer,
- dex::StringIndex string_idx)
- REQUIRES_SHARED(Locks::mutator_lock_)
- REQUIRES(!Roles::uninterruptible_);
-
// TODO: annotalysis disabled as monitor semantics are maintained in Java code.
inline void UnlockJniSynchronizedMethod(jobject locked, Thread* self)
NO_THREAD_SAFETY_ANALYSIS REQUIRES(!Roles::uninterruptible_);
diff --git a/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc b/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc
index fa536c7..6275612 100644
--- a/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc
@@ -210,7 +210,8 @@
auto caller_and_outer = GetCalleeSaveMethodCallerAndOuterMethod(self,
CalleeSaveType::kSaveEverything);
ArtMethod* caller = caller_and_outer.caller;
- ObjPtr<mirror::String> result = ResolveStringFromCode(caller, dex::StringIndex(string_idx));
+ ObjPtr<mirror::String> result =
+ Runtime::Current()->GetClassLinker()->ResolveString(dex::StringIndex(string_idx), caller);
if (LIKELY(result != nullptr) && CanReferenceBss(caller_and_outer.outer_method, caller)) {
StoreStringInBss(caller_and_outer.outer_method, dex::StringIndex(string_idx), result);
}
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index 90e89cf..27f761a 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -945,11 +945,9 @@
return true;
}
case EncodedArrayValueIterator::ValueType::kString: {
- StackHandleScope<1> hs(self);
- Handle<mirror::DexCache> dex_cache(hs.NewHandle(referrer->GetDexCache()));
dex::StringIndex index(static_cast<uint32_t>(encoded_value->GetI()));
ClassLinker* cl = Runtime::Current()->GetClassLinker();
- ObjPtr<mirror::String> o = cl->ResolveString(index, dex_cache);
+ ObjPtr<mirror::String> o = cl->ResolveString(index, referrer);
if (UNLIKELY(o.IsNull())) {
DCHECK(self->IsExceptionPending());
return false;
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index 0ee780d..60bf505 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -340,12 +340,8 @@
}
}
ArtMethod* method = shadow_frame.GetMethod();
- ObjPtr<mirror::String> string_ptr = method->GetDexCache()->GetResolvedString(string_idx);
- if (UNLIKELY(string_ptr == nullptr)) {
- StackHandleScope<1> hs(self);
- Handle<mirror::DexCache> dex_cache(hs.NewHandle(method->GetDexCache()));
- string_ptr = Runtime::Current()->GetClassLinker()->ResolveString(string_idx, dex_cache);
- }
+ ObjPtr<mirror::String> string_ptr =
+ Runtime::Current()->GetClassLinker()->ResolveString(string_idx, method);
return string_ptr;
}
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index 31a83f8..227ace0 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -1269,7 +1269,7 @@
for (auto& m : h_klass->GetDeclaredVirtualMethods(kPointerSize)) {
auto* np_method = m.GetInterfaceMethodIfProxy(kPointerSize);
// May cause thread suspension.
- ObjPtr<String> np_name = np_method->GetNameAsString(self);
+ ObjPtr<String> np_name = np_method->ResolveNameString();
if (!np_name->Equals(h_method_name.Get()) || !np_method->EqualParameters(h_args)) {
if (UNLIKELY(self->IsExceptionPending())) {
return nullptr;
@@ -1291,7 +1291,7 @@
}
auto* np_method = m.GetInterfaceMethodIfProxy(kPointerSize);
// May cause thread suspension.
- ObjPtr<String> np_name = np_method->GetNameAsString(self);
+ ObjPtr<String> np_name = np_method->ResolveNameString();
if (np_name == nullptr) {
self->AssertPendingException();
return nullptr;
diff --git a/runtime/native/java_lang_reflect_Executable.cc b/runtime/native/java_lang_reflect_Executable.cc
index a40cb9b..a10db91 100644
--- a/runtime/native/java_lang_reflect_Executable.cc
+++ b/runtime/native/java_lang_reflect_Executable.cc
@@ -320,7 +320,7 @@
ScopedFastNativeObjectAccess soa(env);
ArtMethod* method = ArtMethod::FromReflectedMethod(soa, javaMethod);
method = method->GetInterfaceMethodIfProxy(kRuntimePointerSize);
- return soa.AddLocalReference<jstring>(method->GetNameAsString(soa.Self()));
+ return soa.AddLocalReference<jstring>(method->ResolveNameString());
}
static jclass Executable_getMethodReturnTypeInternal(JNIEnv* env, jobject javaMethod) {
diff --git a/runtime/native/java_lang_reflect_Field.cc b/runtime/native/java_lang_reflect_Field.cc
index 8766692..895b2f9 100644
--- a/runtime/native/java_lang_reflect_Field.cc
+++ b/runtime/native/java_lang_reflect_Field.cc
@@ -464,8 +464,7 @@
static jstring Field_getNameInternal(JNIEnv* env, jobject javaField) {
ScopedFastNativeObjectAccess soa(env);
ArtField* field = soa.Decode<mirror::Field>(javaField)->GetArtField();
- return soa.AddLocalReference<jstring>(
- field->GetStringName(soa.Self(), true /* resolve */));
+ return soa.AddLocalReference<jstring>(field->ResolveNameString());
}
static jobjectArray Field_getDeclaredAnnotations(JNIEnv* env, jobject javaField) {
diff --git a/runtime/reference_table_test.cc b/runtime/reference_table_test.cc
index 0cb5e56..1d54d21 100644
--- a/runtime/reference_table_test.cc
+++ b/runtime/reference_table_test.cc
@@ -290,7 +290,7 @@
}
{
- // Differently sized byte arrays. Should be sorted by identical (non-unique cound).
+ // Differently sized byte arrays. Should be sorted by identical (non-unique count).
StackHandleScope<1> hs(soa.Self());
Handle<mirror::ByteArray> b1_1 = hs.NewHandle(mirror::ByteArray::Alloc(soa.Self(), 1));
rt.Add(b1_1.Get());