RFR (S) 8178336: Unnecessary SystemDictionary walk for Protection domain liveness

[coleen.phillimore at oracle.com]

On 4/11/17 2:35 AM, Ioi Lam wrote:
> Hi Coleen,
>
> Thanks for doing this clean up. I was guiltily of writing the original 
> code :-(
>
> A few questions:
>
> Why is this block of code moved and the comments dropped?
>
>  328 void Dictionary::oops_do(OopClosure* f) {
>  329   // Only the protection domain oops contain references into the 
> heap. Iterate
>  330   // over all of them.
>  331   _pd_cache_table->oops_do(f);
>  332 }
>  333
>
> It would be better to make the changes in-place.

I didn't have to change Dictionary::oops_do and moved it to be near 
always_strong_oops_do(), so I shouldn't have removed the comment.  I 
moved it back but I don't like that it's separated from the other GC 
functions.   With my other change, it'll be closer (I think I'm going to 
have a merge conflict with myself).
>
> Also, have you validated that (either with an explicit test, or inside 
> the debugger)
>
> [1] live protection domains in _pd_cache_table are properly relocated 
> during GC?
> [2] dead protection domains are removed after class unloading?

I ran with runThese (which has lots of class loading and unloading) with 
logging for both oops_do and unlink functions (removing protection 
domain entries)  but I didn't realize that always_strong_oops_do is 
never called, so I deleted this function.

New webrev (with dictionary::oops_do put back):

http://oklahoma.us.oracle.com/~cphillim/webrev/8178336.02/webrev/index.html

Thanks!
Coleen
>
> Thanks
> - Ioi
>
> On 4/11/17 4:18 AM, coleen.phillimore at oracle.com wrote:
>> Summary: remove system dictionary walk and pass strong closure for 
>> !ClassUnloading
>>
>> See bug for more details:
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8178336.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8178336
>>
>> Tested with nightly tier2-5 tests and jprt (runs all GCs) and 
>> runThese with -XX:-ClassUnloading.
>>
>> Thanks,
>> Coleen
>