RFR (M) 8198313: Wrap holder object for ClassLoaderData in a WeakHandle

Robbin Ehn robbin.ehn at oracle.com
Mon Apr 9 15:05:21 UTC 2018


On 2018-04-09 15:10, coleen.phillimore at oracle.com wrote:
> 
> I had an extensive conversation offline with Stefan, Robbin and Kim and have 
> reworked this change to walk the CLDG weak roots with the SystemDictionary.  
> Also, I moved the WeakHandle holder creation into the ClassLoaderData 
> constructor, and made WeakHandle a template class so that multiple weak roots 
> are supported.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/8198313.04/webrev

Thanks, looks good.

/Robbin

> 
> This passes tier1-5 tests on linux-x64 and windows-x64.  I've also done another 
> performance test aka. dev-submit with no significant difference.
> 
> The additional change to make _class_loader oop into an OopHandle or otherwise 
> remove from ClassLoaderData will be done as a separate change because it breaks 
> JFR.
> 
> Thanks,
> Coleen
> 
> On 4/2/18 3:22 PM, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 3/31/18 2:40 PM, Kim Barrett wrote:
>>>> On Mar 30, 2018, at 1:53 PM, coleen.phillimore at oracle.com wrote:
>>>>
>>>>
>>>> I have an incremental and full .02 version with the changes discussed here.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8198313.02.incr/webrev
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8198313.02/webrev
>>>>
>>>> These have been retested on x86, all hotspot jtreg tests.
>>>> thanks,
>>>> Coleen
>>> Looks good.
>>>
>>> In InstanceKlass::klass_holder_phantom, the klass_ prefix seems
>>> unnecessary. I don't need a new webrev if you decide to change the
>>> name.
>>
>> Since it's pre-existing, I don't think I'll address it with this change.
>> Thank you for the review.
>> Coleen
>>
> 


More information about the hotspot-dev mailing list