RFR: 8189748: More precise closures for WeakProcessor::weak_oops_do calls

[Per Liden]

Looks good.

/Per

On 2017-10-20 19:55, Stefan Karlsson wrote:
> Hi all,
>
> Please review this patch to use more precise closures for the
> WeakProcessor::weak_oops_do calls:
>
> http://cr.openjdk.java.net/~stefank/8189748/webrev.00
> https://bugs.openjdk.java.net/browse/JDK-8189748
>
> ------------------------------------------------------------------------
> WeakProcessor::weak_oops_do takes three closures:
>
> * is_alive - checks if the oop* points to an alive object
>
> * keep_alive - applied to all oop*s that point to live objects. It's
> typically used by the GC to update forwarded pointers to the to the new
> copy of object. The marking GCs pass in a closure that marks objects and
> pushes references to the marking stacks.
>
> * complete - the intention is to use this closure to scan objects copied
> by keep_alive, or mark through objects passed to the marking stacks.
>
> However, much of this work is unnecessary and actually misleading.
>
> The marking GCs should never find unmarked objects when the keep_alive
> closure is applied. So, there's actually no need to apply the keep_alive
> closure, and therefore also not necessary to apply the complete closure.
> The proposal is to change these calls to:
> WeakProcessor::weak_oops_do(is_alive, &do_nothing_cl);
>
> The copying young GCs should never find objects that have not been
> copied, only pointers that have not been forwarded to already copied
> objects. Therefore, the complete closure isn't needed for these calls
> either. As a first step we could change them to:
>  WeakProcessor::weak_oops_do(is_alive, keep_alive);
>  assert(verify that no objects where copied...)
>
> The keep_alive closure is still needed to forward the pointers to the
> copied objects, but later changes could change the keep_alive closure to
> be more explicit and only do the forwarding.
>
> ------------------------------------------------------------------------
> The patch uses and moves the DoNothingClosure to iterator.hpp.
>
> ------------------------------------------------------------------------
> For ParallelScavenge the PSKeepAliveClosure has been changed to
> PSScavengeRootsClosure. The latter is stricter and don't allow oop*s to
> be visited twice. This is the closure used by the StringTable cleaning.
>
> ------------------------------------------------------------------------
> Tested with hs-tier1, hs-tier2, hs-tier3.
>
> Thanks,
> StefanK