RFR: 8254980: ZGC: ZHeapIterator visits armed nmethods with -XX:-ClassUnloading
Stefan Karlsson
stefank at openjdk.java.net
Thu Oct 22 09:48:19 UTC 2020
When class unloading is turned off (-XX:-ClassUnloading) we consider all nmethods in the code cache to be "concurrent roots":
void ZConcurrentRootsIterator::oops_do(ZRootsIteratorClosure* cl) {
...
if (!ClassUnloading) {
_code_cache.oops_do(cl);
}
}
This means that they are concurrently fixed up. See ZMarkConcurrentRootsTask and ZMarkConcurrentRootsIteratorClosure. Java threads that want to engage with an nmethod, needs to go through an nmethod entry barrier in case the concurrent roots have not been processed yet. When the ZHeapIterator is used, we rely on the fact that the vm operation the heap iteration is running in, has done a pre-step to fix all thread stacks (including "entering" the relevant nmethods"). See code using:
// You may override skip_thread_oop_barriers to return true if the operation
// does not access thread-private oops (including frames).
virtual bool skip_thread_oop_barriers() const { return false; }
So, this works when we don't use the entire code cache as root. However, when running with -XX:-ClassUnloading, there's no guarantee that the _code_cache.oops_do(cl) call above has run when the heap iteration is executing. So, the heap iteration code could end up reading oops from an nmethod that has not been entered. Currently, this is benign, since we do apply appropriate load barriers to handle this. However, it would be better if we had a clearer model that only entered/disarmed nmethods are visited.
The proposed patch extends the previous should_disarm_nmethods() bool, into an enum stating what "enter" operation ZNMethodToOopsDoClosure should perform.
I think the names of the values quite self-explanatory:
PreBarrier,
VerifyDisarmed,
None
The tricky one is `Disarm`, which is only used by the marking code. It's used to provide an optimized entry barrier for the marking code. However, since the marking code only provides an OopClosure, and not an NMethodClosuer, we have to put the disarming code here. I have some ideas on how to make all this simpler, but I first want some simplifications that it requires some other patches around our weak root handling to trickle in first.
-------------
Commit messages:
- 8254980: ZGC: ZHeapIterator visits armed nmethods with -XX:-ClassUnloading
Changes: https://git.openjdk.java.net/jdk/pull/801/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=801&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8254980
Stats: 68 lines in 8 files changed: 47 ins; 4 del; 17 mod
Patch: https://git.openjdk.java.net/jdk/pull/801.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/801/head:pull/801
PR: https://git.openjdk.java.net/jdk/pull/801
More information about the hotspot-gc-dev
mailing list