RFR/RFC: 8220342: Remove scavenge_root_nmethods_do from VM_HeapWalkOperation::collect_simple_roots

[Stefan Karlsson]

Hi all,

Please review and/or comment on this change to remove 
CodeCache::scavenge_root_nmehods_do from 
VM_HeapWalkOperation::collect_simple_roots.

http://cr.openjdk.java.net/~stefank/8220342/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-8220342

VM_HeapWalkOperation::collect_simple_roots is used to implement the 
following JVMTI functionality:
  IterateOverReachableObjects
  IterateOverObjectsReachableFromObject
  FollowReferences

From:
https://docs.oracle.com/en/java/javase/11/docs/specs/jvmti.html#FollowReferences

"This function initiates a traversal over the objects that are directly 
and indirectly reachable from the specified object or, if initial_object 
is not specified, all objects reachable from the heap roots. The heap 
root are the set of system classes, JNI globals, references from thread 
stacks, and other objects used as roots for the purposes of garbage 
collection."

The set of roots in collect_simple_roots matches this, and mostly visits 
the set of roots that one of our class unloading enabled GCs would visit.

There are some roots missing. For example:
  Management::oops_do
  JvmtiExport::oops_do
  AOTLoader::oops_do

And there's one set of roots that is present in collect_simple_roots, 
that is not visited by our unloading GCs:
  CodeCache::scavenge_root_nmethods_do

As an example, in PSMarkSweep we have the following comment in the root 
scanning code:
   // Do not treat nmethods as strong roots for mark/sweep, since we can 
unload them.
   //CodeCache::scavenge_root_nmethods_do(...);

The CodeCache::scavenge_root_nmethods_do is only used by Serial, 
Parallel, and CMS, to scan pointers into young gen. Other GCs don't use 
it at all, and if we run with G1 this call does nothing at all.

CodeCache::scavenge_root_nmethods_do is an GC implementation detail that 
I want to confine with the following RFE:
  https://bugs.openjdk.java.net/browse/JDK-8220343
  "Move scavenge_root_nmethods from shared code"

and this is the only external usage of it.

Note also that the only effect of this code is that it adds a set of 
roots that point into the young gen, but only for some of our GCs. There 
are other roots that also point into the young gen that we don't visit. 
For example, the non-system classes. See how collect_simple_roots use 
ClassLoaderDataGraph::always_strong_cld_do instead of 
ClassLoaderDataGraph::cld_do.

I've run through tier1-7 with this removal, without any problems.

I'd be interested in hearing if others have a justification for having 
this code in collect_simple_roots. Or a test-case showing why this is 
needed.

There has been some brief, internal discussions that maybe we want to 
visit all sets of roots in the vm, both strong and weak. A quick 
implementation of that causes problem in testing when objects tagged by 
JVMTI, and JNI weak global handles, gets reported as roots. Because of 
that, such change requires more investigation and work than simply 
extending the set of roots. However, if one were to go that route the 
above call to CodeCache::scavenge_root_nmethods_do would be replaced 
with CodeCache::blobs_do, the function used when we turn off class 
unloading and use our weak roots as strong roots. As an example, see 
GenCollectedHeap::process_roots:

       // CMSCollector uses this to do intermediate-strength collections.
       // We scan the entire code cache, since CodeCache::do_unloading 
is not called.
       CodeCache::blobs_do(code_roots);

Thanks,
StefanK