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

[Stefan Johansson]

Looks good,
StefanJ

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