RFR/RFC: 8220342: Remove scavenge_root_nmethods_do from VM_HeapWalkOperation::collect_simple_roots
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Tue Mar 12 21:50:05 UTC 2019
The fix looks good to me.
Testing the tiers 1-7 for different GC's has to be good enough.
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 is used to implement the
> following JVMTI functionality:
> "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:
> And there's one set of roots that is present in collect_simple_roots,
> that is not visited by our unloading GCs:
> 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.
> 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:
> "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
> 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
> // CMSCollector uses this to do intermediate-strength collections.
> // We scan the entire code cache, since CodeCache::do_unloading
> is not called.
More information about the hotspot-gc-dev