Change j.l.r.Field to use ArtField index instead of dexFileIndex
Using the dexFileIndex to determine the ArtField a
java.lang.reflect.Field object points to requires us to use a
dex-cache and to update all existing Field objects if this index
changes (for example due to class redefinition). This could be rather
slow. This replaces the dex-file index with the index into the
declaring class's SFields/IFields arrays where the specified ArtMethod
is stored.
Bug: 149236640
Test: ./test.py --host
Change-Id: I9a7d69903ece0ea94e3b3e93b4c8a872ac4073e8
diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc
index d918159..a6b597b 100644
--- a/openjdkjvmti/ti_redefine.cc
+++ b/openjdkjvmti/ti_redefine.cc
@@ -2589,7 +2589,6 @@
}
void Redefiner::ClassRedefinition::UpdateFields(art::ObjPtr<art::mirror::Class> mclass) {
- std::unordered_map<uint32_t, uint32_t> dex_field_index_map;
// TODO The IFields & SFields pointers should be combined like the methods_ arrays were.
for (auto fields_iter : {mclass->GetIFields(), mclass->GetSFields()}) {
for (art::ArtField& field : fields_iter) {
@@ -2603,32 +2602,10 @@
dex_file_->FindFieldId(*new_declaring_id, *new_name_id, *new_type_id);
CHECK(new_field_id != nullptr);
uint32_t new_field_index = dex_file_->GetIndexForFieldId(*new_field_id);
- // We need to keep track of the changes to the index so we can update any
- // java.lang.reflect.Field objects that happen to be on the heap.
- dex_field_index_map[field.GetDexFieldIndex()] = new_field_index;
// We only need to update the index since the other data in the ArtField cannot be updated.
field.SetDexFieldIndex(new_field_index);
}
}
- // walk the heap to update any java/lang/reflect/Field objects that refer to any of these fields.
- // TODO We could combine this and do one heap-walk for all redefined classes.
- art::ObjPtr<art::mirror::Class> reflect_field(art::GetClassRoot<art::mirror::Field>());
- auto vis = [&](art::ObjPtr<art::mirror::Object> obj) REQUIRES_SHARED(art::Locks::mutator_lock_) {
- // j.l.r.Field is a final class.
- if (obj->GetClass() != reflect_field) {
- return;
- }
- art::ObjPtr<art::mirror::Field> fld(art::down_cast<art::mirror::Field*>(obj.Ptr()));
- // Only this class is getting new indexs.
- if (fld->GetDeclaringClass() != mclass) {
- return;
- }
- auto it = dex_field_index_map.find(fld->GetDexFieldIndex());
- if (it != dex_field_index_map.end()) {
- fld->SetDexFieldIndex</*kTransactionActive=*/false>(it->second);
- }
- };
- driver_->runtime_->GetHeap()->VisitObjectsPaused(vis);
}
void Redefiner::ClassRedefinition::CollectNewFieldAndMethodMappings(
diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc
index 0969b1c..1e501a7 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -719,8 +719,8 @@
struct FieldOffsets : public CheckOffsets<mirror::Field> {
FieldOffsets() : CheckOffsets<mirror::Field>(false, "Ljava/lang/reflect/Field;") {
addOffset(OFFSETOF_MEMBER(mirror::Field, access_flags_), "accessFlags");
+ addOffset(OFFSETOF_MEMBER(mirror::Field, art_field_index_), "artFieldIndex");
addOffset(OFFSETOF_MEMBER(mirror::Field, declaring_class_), "declaringClass");
- addOffset(OFFSETOF_MEMBER(mirror::Field, dex_field_index_), "dexFieldIndex");
addOffset(OFFSETOF_MEMBER(mirror::Field, offset_), "offset");
addOffset(OFFSETOF_MEMBER(mirror::Field, type_), "type");
}
diff --git a/runtime/mirror/field-inl.h b/runtime/mirror/field-inl.h
index ac11be1..8a9cec4 100644
--- a/runtime/mirror/field-inl.h
+++ b/runtime/mirror/field-inl.h
@@ -89,7 +89,12 @@
ret->SetType<kTransactionActive>(type.Get());
ret->SetDeclaringClass<kTransactionActive>(field->GetDeclaringClass());
ret->SetAccessFlags<kTransactionActive>(field->GetAccessFlags());
- ret->SetDexFieldIndex<kTransactionActive>(dex_field_index);
+ auto iter_range = field->IsStatic() ? field->GetDeclaringClass()->GetSFields()
+ : field->GetDeclaringClass()->GetIFields();
+ auto position = std::find_if(
+ iter_range.begin(), iter_range.end(), [&](const auto& f) { return &f == field; });
+ DCHECK(position != iter_range.end());
+ ret->SetArtFieldIndex<kTransactionActive>(std::distance(iter_range.begin(), position));
ret->SetOffset<kTransactionActive>(field->GetOffset().Int32Value());
return ret.Get();
}
diff --git a/runtime/mirror/field.cc b/runtime/mirror/field.cc
index 9e566b5..e9669b8 100644
--- a/runtime/mirror/field.cc
+++ b/runtime/mirror/field.cc
@@ -27,47 +27,31 @@
void Field::VisitTarget(ReflectiveValueVisitor* v) {
HeapReflectiveSourceInfo hrsi(kSourceJavaLangReflectField, this);
- ArtField* orig = GetArtField(/*use_dex_cache*/false);
+ ArtField* orig = GetArtField();
ArtField* new_value = v->VisitField(orig, hrsi);
if (orig != new_value) {
- SetDexFieldIndex<false>(new_value->GetDexFieldIndex());
SetOffset<false>(new_value->GetOffset().Int32Value());
SetDeclaringClass<false>(new_value->GetDeclaringClass());
+ auto new_range =
+ IsStatic() ? GetDeclaringClass()->GetSFields() : GetDeclaringClass()->GetIFields();
+ auto position = std::find_if(
+ new_range.begin(), new_range.end(), [&](const auto& f) { return &f == new_value; });
+ DCHECK(position != new_range.end());
+ SetArtFieldIndex<false>(std::distance(new_range.begin(), position));
WriteBarrier::ForEveryFieldWrite(this);
}
- DCHECK_EQ(new_value, GetArtField(/*use_dex_cache*/false));
+ DCHECK_EQ(new_value, GetArtField());
}
-ArtField* Field::GetArtField(bool use_dex_cache) {
+ArtField* Field::GetArtField() {
ObjPtr<mirror::Class> declaring_class = GetDeclaringClass();
- if (UNLIKELY(declaring_class->IsProxyClass())) {
- DCHECK(IsStatic());
- DCHECK_EQ(declaring_class->NumStaticFields(), 2U);
- // 0 == Class[] interfaces; 1 == Class[][] throws;
- if (GetDexFieldIndex() == 0) {
- return &declaring_class->GetSFieldsPtr()->At(0);
- } else {
- DCHECK_EQ(GetDexFieldIndex(), 1U);
- return &declaring_class->GetSFieldsPtr()->At(1);
- }
+ DCHECK_LT(GetArtFieldIndex(),
+ IsStatic() ? declaring_class->NumStaticFields() : declaring_class->NumInstanceFields());
+ if (IsStatic()) {
+ return declaring_class->GetStaticField(GetArtFieldIndex());
+ } else {
+ return declaring_class->GetInstanceField(GetArtFieldIndex());
}
- const ObjPtr<mirror::DexCache> dex_cache = declaring_class->GetDexCache();
- ArtField* art_field = use_dex_cache
- ? dex_cache->GetResolvedField(GetDexFieldIndex(), kRuntimePointerSize)
- : nullptr;
- if (UNLIKELY(art_field == nullptr)) {
- if (IsStatic()) {
- art_field = declaring_class->FindDeclaredStaticField(dex_cache, GetDexFieldIndex());
- } else {
- art_field = declaring_class->FindInstanceField(dex_cache, GetDexFieldIndex());
- }
- CHECK(art_field != nullptr);
- if (use_dex_cache) {
- dex_cache->SetResolvedField(GetDexFieldIndex(), art_field, kRuntimePointerSize);
- }
- }
- CHECK_EQ(declaring_class, art_field->GetDeclaringClass());
- return art_field;
}
} // namespace mirror
diff --git a/runtime/mirror/field.h b/runtime/mirror/field.h
index e9e3c17..dd5ee76 100644
--- a/runtime/mirror/field.h
+++ b/runtime/mirror/field.h
@@ -39,13 +39,13 @@
// C++ mirror of java.lang.reflect.Field.
class MANAGED Field : public AccessibleObject {
public:
- ALWAYS_INLINE uint32_t GetDexFieldIndex() REQUIRES_SHARED(Locks::mutator_lock_) {
- return GetField32(OFFSET_OF_OBJECT_MEMBER(Field, dex_field_index_));
+ ALWAYS_INLINE uint32_t GetArtFieldIndex() REQUIRES_SHARED(Locks::mutator_lock_) {
+ return GetField32(OFFSET_OF_OBJECT_MEMBER(Field, art_field_index_));
}
// Public for use by class redefinition code.
template<bool kTransactionActive>
- void SetDexFieldIndex(uint32_t idx) REQUIRES_SHARED(Locks::mutator_lock_) {
- SetField32<kTransactionActive>(OFFSET_OF_OBJECT_MEMBER(Field, dex_field_index_), idx);
+ void SetArtFieldIndex(uint32_t idx) REQUIRES_SHARED(Locks::mutator_lock_) {
+ SetField32<kTransactionActive>(OFFSET_OF_OBJECT_MEMBER(Field, art_field_index_), idx);
}
@@ -75,10 +75,7 @@
return GetField32(OFFSET_OF_OBJECT_MEMBER(Field, offset_));
}
- // Slow, try to use only for PrettyField and such. Set use-dex-cache to false to not utilize the
- // dex-cache when finding the art-field. This is useful for cases where the dex-cache might be
- // temporarally invalid.
- ArtField* GetArtField(bool use_dex_cache = true) REQUIRES_SHARED(Locks::mutator_lock_);
+ ArtField* GetArtField() REQUIRES_SHARED(Locks::mutator_lock_);
template <PointerSize kPointerSize, bool kTransactionActive = false>
static ObjPtr<mirror::Field> CreateFromArtField(Thread* self,
@@ -98,7 +95,7 @@
HeapReference<mirror::Class> declaring_class_;
HeapReference<mirror::Class> type_;
int32_t access_flags_;
- int32_t dex_field_index_;
+ int32_t art_field_index_;
int32_t offset_;
template<bool kTransactionActive>