RFR (trivial) 8203953: Rename SystemDictionary::load_shared_class(Symbol*, Handle, TRAPS) to load_shared_boot_class()

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Mon Oct 29 15:51:30 UTC 2018


Yes, this looks good!
Coleen

On 10/26/18 4:55 PM, Jiangli Zhou wrote:
> Hi Coleen,
>
> Thanks for the review! Removed the class_loader argument and also 
> cleaned up the comment as suggested.
>
> http://cr.openjdk.java.net/~jiangli/8203953/webrev.01/
>
> Thanks,
> Jiangli
>
> On 10/26/18 1:23 PM, coleen.phillimore at oracle.com wrote:
>>
>> Yes, it looks trivial.  You could even remove the class_loader 
>> argument from this newly renamed function.
>>
>> 1192 // Load a class for boot loader from the shared spaces (found 
>> through
>> 1193 // the shared system dictionary). Force the superclass and all 
>> interfaces
>> 1194 // to be loaded. Update the class definition to include sibling 
>> classes
>> 1195 // and no subclasses (yet). [Classes in the shared space are not 
>> part of
>> 1196 // the object hierarchy until loaded.]
>>
>> Can you add a space between super and class?
>>
>> This code does update the Klass::_subklass and Klass::_next_sibling 
>> lists, and then clears them at remove_unshareable_info.  The comment 
>> seems to say differently (or who knows what it says...)
>>
>> I think the comment starting at "Update" doesn't make any sense to 
>> me, isn't entirely accurate, and isn't interesting or helpful at this 
>> point.  I would remove it.
>>
>> thanks,
>> Coleen
>>
>> On 10/26/18 3:55 PM, Jiangli Zhou wrote:
>>> Please review this trivial change that renames 
>>> SystemDictionary::load_shared_class(Symbol*, Handle, TRAPS) to 
>>> load_shared_boot_class(), since it is only called for the boot 
>>> loader. As part of the change, I also removed the 
>>> 'class_loader.is_null()' check and made it as an assert.
>>>
>>> webrev: http://cr.openjdk.java.net/~jiangli/8203953/webrev.00/
>>>
>>> RFE: https://bugs.openjdk.java.net/browse/JDK-8203953
>>>
>>> Tested build on linux-x64 and ran appcds tests locally. Will run 
>>> tier1-tier3 after mach5 is back online.
>>>
>>> Thanks,
>>>
>>> Jiangli
>>>
>>
>



More information about the hotspot-runtime-dev mailing list