Clean up after obsolete methods update fix.
This is a follow-up after
https://android-review.googlesource.com/942485 ,
addressing most of the late comments.
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing --interpreter
Bug: 73333076
Change-Id: I52c0a3cbf81e97474dc46846486263946379416a
diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc
index 975843c..2474b02 100644
--- a/openjdkjvmti/ti_redefine.cc
+++ b/openjdkjvmti/ti_redefine.cc
@@ -143,11 +143,18 @@
art::ArtMethod* obsolete_method;
};
- class ObsoleteMapIter : public std::iterator<std::forward_iterator_tag, ObsoleteMethodPair> {
+ class ObsoleteMapIter {
public:
+ using iterator_category = std::forward_iterator_tag;
+ using value_type = ObsoleteMethodPair;
+ using difference_type = ptrdiff_t;
+ using pointer = void; // Unsupported.
+ using reference = void; // Unsupported.
+
ObsoleteMethodPair operator*() const
REQUIRES(art::Locks::mutator_lock_, art::Roles::uninterruptible_) {
- art::ArtMethod* obsolete = map_->FindObsoleteVersion(iter_->first);
+ art::ArtMethod* obsolete = map_->obsolete_methods_->GetElementPtrSize<art::ArtMethod*>(
+ iter_->second, art::kRuntimePointerSize);
DCHECK(obsolete != nullptr);
return { iter_->first, obsolete };
}
@@ -160,13 +167,13 @@
return !(*this == other);
}
- ObsoleteMapIter operator++(int) const {
+ ObsoleteMapIter operator++(int) {
ObsoleteMapIter retval = *this;
++(*this);
return retval;
}
- ObsoleteMapIter operator++() const {
+ ObsoleteMapIter operator++() {
++iter_;
return *this;
}
@@ -177,7 +184,7 @@
: map_(map), iter_(iter) {}
const ObsoleteMap* map_;
- mutable std::unordered_map<art::ArtMethod*, int32_t>::const_iterator iter_;
+ std::unordered_map<art::ArtMethod*, int32_t>::const_iterator iter_;
friend class ObsoleteMap;
};
@@ -637,7 +644,8 @@
art::MutexLock mu(driver_->self_, *art::Locks::thread_list_lock_);
art::ThreadList* list = art::Runtime::Current()->GetThreadList();
list->ForEach(DoAllocateObsoleteMethodsCallback, static_cast<void*>(&ctx));
- // Update JIT Data structures to point to the new method.
+ // After we've done walking all threads' stacks and updating method pointers on them,
+ // update JIT data structures (used by the stack walk above) to point to the new methods.
art::jit::Jit* jit = art::Runtime::Current()->GetJit();
if (jit != nullptr) {
for (const ObsoleteMap::ObsoleteMethodPair& it : *ctx.obsolete_map) {
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index fee5bec..47a8b47 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -1821,17 +1821,9 @@
}
if (kIsDebugBuild && method != nullptr && !method->IsNative()) {
- // When we are walking the stack to redefine classes and creating obsolete methods it is
- // possible that we might have updated the method_code_map by making this method obsolete in a
- // previous frame. Therefore we should just check that the non-obsolete version of this method
- // is the one we expect. We change to the non-obsolete versions in the error message since the
- // obsolete version of the method might not be fully initialized yet. This situation can only
- // occur when we are in the process of allocating and setting up obsolete methods. Otherwise
- // method and it->second should be identical. (See openjdkjvmti/ti_redefine.cc for more
- // information.)
- DCHECK_EQ(found_method->GetNonObsoleteMethod(), method->GetNonObsoleteMethod())
- << ArtMethod::PrettyMethod(method->GetNonObsoleteMethod()) << " "
- << ArtMethod::PrettyMethod(found_method->GetNonObsoleteMethod()) << " "
+ DCHECK_EQ(found_method, method)
+ << ArtMethod::PrettyMethod(method) << " "
+ << ArtMethod::PrettyMethod(found_method) << " "
<< std::hex << pc;
}
return method_header;
diff --git a/test/1960-obsolete-jit-multithread-native/src/Main.java b/test/1960-obsolete-jit-multithread-native/src/Main.java
index 1ae5fb7..dbe11a5 100644
--- a/test/1960-obsolete-jit-multithread-native/src/Main.java
+++ b/test/1960-obsolete-jit-multithread-native/src/Main.java
@@ -31,7 +31,7 @@
// class Transform {
// public native void nativeSayHi(Consumer<Consumer<String>> r, Consumer<String> rep);
// public void sayHi(Consumer<Consumer<String>> r, Consumer<String> reporter) {
- // reporter.accept("goodbye - Start method sayHi");
+ // reporter.accept("goodbye - Start method sayHi");
// r.accept(reporter);
// reporter.accept("goodbye - End method sayHi");
// }
@@ -156,8 +156,6 @@
}
public void run() {
- // Figure out if we can even JIT at all.
- final boolean has_jit = hasJit();
try {
this.arrivalLatch.await();
maybePrint("REDEFINITION THREAD: redefining something!");
@@ -201,7 +199,5 @@
}
}
- private static native boolean hasJit();
-
private static native void ensureJitCompiled(Class c, String name);
}
diff --git a/test/1961-obsolete-jit-multithread/src/Main.java b/test/1961-obsolete-jit-multithread/src/Main.java
index 81bb936..390a9de 100644
--- a/test/1961-obsolete-jit-multithread/src/Main.java
+++ b/test/1961-obsolete-jit-multithread/src/Main.java
@@ -30,7 +30,7 @@
//
// class Transform {
// public void sayHi(Consumer<Consumer<String>> r, Consumer<String> reporter) {
- // reporter.accept("goodbye - Start method sayHi");
+ // reporter.accept("goodbye - Start method sayHi");
// r.accept(reporter);
// reporter.accept("goodbye - End method sayHi");
// }
@@ -154,8 +154,6 @@
}
public void run() {
- // Figure out if we can even JIT at all.
- final boolean has_jit = hasJit();
try {
this.arrivalLatch.await();
maybePrint("REDEFINITION THREAD: redefining something!");
@@ -199,7 +197,5 @@
}
}
- private static native boolean hasJit();
-
private static native void ensureJitCompiled(Class c, String name);
}