RFR(XS): 8242134: Consolidate the get_package_entry() in SystemDictionaryShared and ClassLoader

Calvin Cheung calvin.cheung at oracle.com
Tue Apr 7 21:58:40 UTC 2020


Thanks for your review, Lois.

Calvin

On 4/7/20 2:37 PM, Lois Foltan wrote:
> Latest webrev looks good Calvin!
> Lois
>
> On 4/7/2020 5:06 PM, Calvin Cheung wrote:
>> Hi Claes,
>>
>> Thanks for your review!
>>
>> On 4/7/20 1:21 PM, Claes Redestad wrote:
>>> Hi,
>>>
>>> one suggestion is that I think we should try to keep the invariant
>>> that loader_data can't be NULL in ClassLoader::get_package_entry and
>>> avoid adding a NULL check there.
>>>
>>> The only call site that needs to be fixed appears to be the one in
>>> systemDictionaryShared. Maybe something like this:
>>>
>>>        ClassLoaderData* platform_loader = 
>>> ClassLoaderData::class_loader_data_or_null(SystemDictionary::java_platform_loader()); 
>>> // can be NULL during bootstrap
>>>        if ((platform_loader == NULL ||
>>>             ClassLoader::get_package_entry(pkg_name, 
>>> platform_loader) == NULL) &&
>>>             ClassLoader::get_package_entry(pkg_name, 
>>> ClassLoaderData::the_null_class_loader_data()) == NULL) {
>>>
>>> What do you think?
>>
>> I've tried your suggestions and it passed all appcds locally on 
>> linux-x64.
>>
>> Updated webrev:
>>     http://cr.openjdk.java.net/~ccheung/jdk15/8242134/webrev.01/
>>
>> thanks,
>>
>> Calvin
>>>
>>> /Claes
>>>
>>> On 2020-04-07 19:39, Calvin Cheung wrote:
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8242134
>>>> webrev: http://cr.openjdk.java.net/~ccheung/jdk15/8242134/webrev.00/
>>>>
>>>> The above proposed change removes get_package_entry() from 
>>>> SystemDictionaryShared.
>>>> The ClassLoader::get_package_entry() is being modified slightly so 
>>>> that it could be used in SystemDictionaryShared.
>>>>
>>>> Passed tier 1 - 4 tests.
>>>>
>>>> thanks,
>>>> Calvin
>


More information about the hotspot-runtime-dev mailing list