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

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Tue Apr 11 15:27:13 UTC 2017



On 4/11/17 9:03 AM, coleen.phillimore at oracle.com wrote:
>
>
> 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 :-(

I should point out that I reviewed it :)
Coleen

>>
>> 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
>>
>



More information about the hotspot-dev mailing list