RFR: 8202813: Move vm_weak processing from SystemDictionary::do_unloading to WeakProcessor

[Stefan Johansson]

Hi Kim,

On 2018-05-08 21:34, Kim Barrett wrote:
> Please review this change to move the clearing of dead entries in
> vm_weak_oop_storage from SystemDictionary::do_unloading to preceeding
> calls to WeakProcessor::weak_oops_do.
> 
> This involves conditionalizing WeakProcessor invocations, to allow
> different calls to process different subsets of the native weak
> references.  For example, vm_weak_oop_storage is treated as strong
> rather than weak when ClassUnloading is disabled.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8202813
> 
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8202813/open.00/
> 
Thanks for doing this work to unify weak root processing.

I might be missing the bigger picture, so before starting to commenting 
the code I have a couple of questions:
1. With this patch SystemDictionary::vm_weak_oop_storage is now 
processed both by the WeakProcessor in weak_oops_do and the 
SystemDictionary in oops_do() and roots_oops_do(). Shouldn't all 
processing be moved to the WeakProcessor to make it clear who is 
responsible for doing the processing? Or why do we want to treat 
do_unloading special?

2. For this patch I don't see the need for the PhaseSet class and the 
serial and parallel phases distinction. Couldn't we just pass in a bool 
to weak_oops_do saying if we should handle the vm_weak_oop_storage or 
not? I understand that for upcoming patches this might be needed, but 
then I think this should be added in that change.

Thanks,
Stefan

> Testing:
> Mach5 jdk-tier{1-2}, hs-tier{1-5}
> Locally runthese30m.
>