Hi Stefan,
The fix looks good to me.
Testing the tiers 1-7 for different GC's has to be good enough.
Thanks,
Serguei
On 3/12/19 8:19 AM, Stefan Karlsson wrote:
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
