RFR: 8254980: ZGC: ZHeapIterator visits armed nmethods with -XX:-ClassUnloading [v3]

[Stefan Karlsson]

> 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.

Stefan Karlsson has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:

 - Merge remote-tracking branch 'origin/master' into 8254980_zheapiterator_classunloading
 - Review 2
 - Review 1
 - 8254980: ZGC: ZHeapIterator visits armed nmethods with -XX:-ClassUnloading

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/801/files
  - new: https://git.openjdk.java.net/jdk/pull/801/files/c6a1879a..9de772e1

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=801&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=801&range=01-02

  Stats: 4279 lines in 235 files changed: 2022 ins; 1513 del; 744 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