RFR(S) 8240205: Avoid PackageEntry lookup when loading shared classes

[Calvin Cheung]

On 4/3/20 1:53 PM, Lois Foltan wrote:
> On 4/3/2020 1:23 PM, Calvin Cheung wrote:
>> Hi Lois,
>>
>> Thanks for your review!
>>
>> On 4/3/20 9:46 AM, Lois Foltan wrote:
>>> On 4/2/2020 8:58 PM, Calvin Cheung wrote:
>>>> RFE: https://bugs.openjdk.java.net/browse/JDK-8240205
>>>> webrev: http://cr.openjdk.java.net/~ccheung/jdk15/8240205/webrev.00/
>>>>
>>>> During loading of a shared class from a CDS archive, the 
>>>> PackageEntry is being looked up in several places within the same 
>>>> call stack. This RFE is to do the PackageEntry lookup at one place 
>>>> and then pass it along to the subsequently called functions which 
>>>> need it.
>>>>
>>>> Perf improvement on loading approx. 2000 classes on linux:
>>>>
>>>> instr delta = -17930620 -0.6155%
>>>> time delta = -6.311 ms -0.7516%
>>>>
>>>> Passed tiers1 - 4 testing.
>>>>
>>>> thanks,
>>>>
>>>> Calvin
>>>>
>>>>
>>> Hi Calvin,
>>>
>>> This looks good & promising on the numbers!  I have a slight concern 
>>> that you've introduced yet another "get package entry from name" 
>>> sort of function in SystemDictionaryShared. classfile/modules.cpp 
>>> already contains a static method get_package_entry_by_name() but it 
>>> operates on a package name not a class name.  Please consider moving 
>>> SystemDictionaryShared::get_package_entry_from_class_name into 
>>> ClassLoader near ClassLoader::get_package_from_class_name(). That 
>>> seems like a more consistent place to add it that various areas of 
>>> the JVM can take advantage of.
>>
>> The SystemDictionaryShared::get_package_entry_from_class_name 
>> function is currently being used only within the 
>> SharedDictionaryShared class. That's why it was declared private and 
>> I think that's the right place for it.
>>
>> There also another SystemDictionaryShared::get_package_entry() which 
>> operates on a package name and is very similar to the one in 
>> ClassLoader. I can file another RFE to consolidate those 2 functions.
>
> That would be great. Thanks!

I've pushed the change and filed the following RFE:

     https://bugs.openjdk.java.net/browse/JDK-8242134

thanks,

Calvin

> Lois
>
>>
>> thanks,
>>
>> Calvin
>>
>>>
>>> Thanks,
>>> Lois
>