8254980: ZGC: ZHeapIterator visits armed nmethods with -XX:-ClassUnloading
Reviewed-by: eosterlund, pliden
diff --git a/src/hotspot/share/gc/z/zHeapIterator.cpp b/src/hotspot/share/gc/z/zHeapIterator.cpp
index c5f3970..367e276 100644
--- a/src/hotspot/share/gc/z/zHeapIterator.cpp
+++ b/src/hotspot/share/gc/z/zHeapIterator.cpp
@@ -126,6 +126,17 @@
CodeBlobToOopClosure code_cl(this, false /* fix_oop_relocations */);
thread->oops_do(this, &code_cl);
}
+
+ virtual ZNMethodEntry nmethod_entry() const {
+ if (ClassUnloading) {
+ // All encountered nmethods should have been "entered" during stack walking
+ return ZNMethodEntry::VerifyDisarmed;
+ } else {
+ // All nmethods are considered roots and will be visited.
+ // Make sure that the unvisited gets fixed and disarmed before proceeding.
+ return ZNMethodEntry::PreBarrier;
+ }
+ }
};
template <bool VisitReferents>
diff --git a/src/hotspot/share/gc/z/zMark.cpp b/src/hotspot/share/gc/z/zMark.cpp
index 421dc28..df47f69 100644
--- a/src/hotspot/share/gc/z/zMark.cpp
+++ b/src/hotspot/share/gc/z/zMark.cpp
@@ -582,8 +582,9 @@
ZThreadLocalAllocBuffer::publish_statistics();
}
- virtual bool should_disarm_nmethods() const {
- return true;
+ virtual ZNMethodEntry nmethod_entry() const {
+ // Only apply closure to armed nmethods, and then disarm them.
+ return ZNMethodEntry::Disarm;
}
virtual void do_thread(Thread* thread) {
diff --git a/src/hotspot/share/gc/z/zNMethod.cpp b/src/hotspot/share/gc/z/zNMethod.cpp
index 16c0ef1..dfbf38d 100644
--- a/src/hotspot/share/gc/z/zNMethod.cpp
+++ b/src/hotspot/share/gc/z/zNMethod.cpp
@@ -236,28 +236,42 @@
class ZNMethodToOopsDoClosure : public NMethodClosure {
private:
- OopClosure* const _cl;
- const bool _should_disarm_nmethods;
+ OopClosure* const _cl;
+ const ZNMethodEntry _entry;
+ BarrierSetNMethod* const _bs_nm;
public:
- ZNMethodToOopsDoClosure(OopClosure* cl, bool should_disarm_nmethods) :
+ ZNMethodToOopsDoClosure(OopClosure* cl, ZNMethodEntry entry) :
_cl(cl),
- _should_disarm_nmethods(should_disarm_nmethods) {}
+ _entry(entry),
+ _bs_nm(BarrierSet::barrier_set()->barrier_set_nmethod()) {}
virtual void do_nmethod(nmethod* nm) {
+ if (_entry == ZNMethodEntry::PreBarrier) {
+ // Apply entry barrier before proceeding with closure
+ _bs_nm->nmethod_entry_barrier(nm);
+ }
+
ZLocker<ZReentrantLock> locker(ZNMethod::lock_for_nmethod(nm));
if (!nm->is_alive()) {
return;
}
- if (_should_disarm_nmethods) {
+ if (_entry == ZNMethodEntry::Disarm) {
+ // Apply closure and disarm only armed nmethods
if (ZNMethod::is_armed(nm)) {
ZNMethod::nmethod_oops_do(nm, _cl);
ZNMethod::disarm(nm);
}
- } else {
- ZNMethod::nmethod_oops_do(nm, _cl);
+ return;
}
+
+ if (_entry == ZNMethodEntry::VerifyDisarmed) {
+ // Only verify
+ assert(!ZNMethod::is_armed(nm), "Must be disarmed");
+ }
+
+ ZNMethod::nmethod_oops_do(nm, _cl);
}
};
@@ -269,8 +283,8 @@
ZNMethodTable::nmethods_do_end();
}
-void ZNMethod::oops_do(OopClosure* cl, bool should_disarm_nmethods) {
- ZNMethodToOopsDoClosure nmethod_cl(cl, should_disarm_nmethods);
+void ZNMethod::oops_do(OopClosure* cl, ZNMethodEntry entry) {
+ ZNMethodToOopsDoClosure nmethod_cl(cl, entry);
ZNMethodTable::nmethods_do(&nmethod_cl);
}
diff --git a/src/hotspot/share/gc/z/zNMethod.hpp b/src/hotspot/share/gc/z/zNMethod.hpp
index 1b6117b..66914f1 100644
--- a/src/hotspot/share/gc/z/zNMethod.hpp
+++ b/src/hotspot/share/gc/z/zNMethod.hpp
@@ -31,6 +31,13 @@
class ZReentrantLock;
class ZWorkers;
+enum class ZNMethodEntry {
+ PreBarrier,
+ Disarm,
+ VerifyDisarmed,
+ None
+};
+
class ZNMethod : public AllStatic {
private:
static void attach_gc_data(nmethod* nm);
@@ -52,7 +59,7 @@
static void oops_do_begin();
static void oops_do_end();
- static void oops_do(OopClosure* cl, bool should_disarm_nmethods);
+ static void oops_do(OopClosure* cl, ZNMethodEntry entry);
static ZReentrantLock* lock_for_nmethod(nmethod* nm);
diff --git a/src/hotspot/share/gc/z/zOopClosures.hpp b/src/hotspot/share/gc/z/zOopClosures.hpp
index 25984b6c..93aede7 100644
--- a/src/hotspot/share/gc/z/zOopClosures.hpp
+++ b/src/hotspot/share/gc/z/zOopClosures.hpp
@@ -57,12 +57,16 @@
public:
virtual void do_oop(oop* p);
virtual void do_oop(narrowOop* p);
+
+ virtual ZNMethodEntry nmethod_entry() const;
};
class ZPhantomCleanOopClosure : public ZRootsIteratorClosure {
public:
virtual void do_oop(oop* p);
virtual void do_oop(narrowOop* p);
+
+ virtual ZNMethodEntry nmethod_entry() const;
};
#endif // SHARE_GC_Z_ZOOPCLOSURES_HPP
diff --git a/src/hotspot/share/gc/z/zOopClosures.inline.hpp b/src/hotspot/share/gc/z/zOopClosures.inline.hpp
index da8f22f..eb9c7ad 100644
--- a/src/hotspot/share/gc/z/zOopClosures.inline.hpp
+++ b/src/hotspot/share/gc/z/zOopClosures.inline.hpp
@@ -80,6 +80,11 @@
ZBarrier::keep_alive_barrier_on_phantom_oop_field(p);
}
+inline ZNMethodEntry ZPhantomKeepAliveOopClosure::nmethod_entry() const {
+ ShouldNotReachHere();
+ return ZNMethodEntry::None;
+}
+
inline void ZPhantomKeepAliveOopClosure::do_oop(narrowOop* p) {
ShouldNotReachHere();
}
@@ -104,4 +109,9 @@
ShouldNotReachHere();
}
+inline ZNMethodEntry ZPhantomCleanOopClosure::nmethod_entry() const {
+ ShouldNotReachHere();
+ return ZNMethodEntry::None;
+}
+
#endif // SHARE_GC_Z_ZOOPCLOSURES_INLINE_HPP
diff --git a/src/hotspot/share/gc/z/zRootsIterator.cpp b/src/hotspot/share/gc/z/zRootsIterator.cpp
index 5d5e719..3ddda27 100644
--- a/src/hotspot/share/gc/z/zRootsIterator.cpp
+++ b/src/hotspot/share/gc/z/zRootsIterator.cpp
@@ -122,7 +122,7 @@
void ZConcurrentRootsIterator::do_code_cache(ZRootsIteratorClosure* cl) {
ZStatTimer timer(ZSubPhaseConcurrentRootsCodeCache);
- ZNMethod::oops_do(cl, cl->should_disarm_nmethods());
+ ZNMethod::oops_do(cl, cl->nmethod_entry());
}
class ZConcurrentRootsIteratorThreadClosure : public ThreadClosure {
diff --git a/src/hotspot/share/gc/z/zRootsIterator.hpp b/src/hotspot/share/gc/z/zRootsIterator.hpp
index 87626b0..49e87cd 100644
--- a/src/hotspot/share/gc/z/zRootsIterator.hpp
+++ b/src/hotspot/share/gc/z/zRootsIterator.hpp
@@ -26,6 +26,7 @@
#include "classfile/classLoaderDataGraph.hpp"
#include "gc/shared/oopStorageSetParState.hpp"
+#include "gc/z/zNMethod.hpp"
#include "memory/allocation.hpp"
#include "memory/iterator.hpp"
#include "runtime/threadSMR.hpp"
@@ -61,9 +62,7 @@
public:
virtual void do_thread(Thread* thread) {}
- virtual bool should_disarm_nmethods() const {
- return false;
- }
+ virtual ZNMethodEntry nmethod_entry() const = 0;
};
class ZJavaThreadsIterator {
diff --git a/src/hotspot/share/gc/z/zVerify.cpp b/src/hotspot/share/gc/z/zVerify.cpp
index 841cd4c..a10d64c 100644
--- a/src/hotspot/share/gc/z/zVerify.cpp
+++ b/src/hotspot/share/gc/z/zVerify.cpp
@@ -94,6 +94,11 @@
bool verify_fixed() const {
return _verify_fixed;
}
+
+ virtual ZNMethodEntry nmethod_entry() const {
+ // Verification performs its own verification
+ return ZNMethodEntry::None;
+ }
};
class ZVerifyCodeBlobClosure : public CodeBlobToOopClosure {
@@ -188,7 +193,7 @@
verify_stack.verify_frames();
}
-class ZVerifyOopClosure : public ClaimMetadataVisitingOopIterateClosure, public ZRootsIteratorClosure {
+class ZVerifyOopClosure : public ClaimMetadataVisitingOopIterateClosure {
private:
const bool _verify_weaks;
