RFR(S): 8240204: Optimize package handling for archived classes

Yumin Qi yumin.qi at oracle.com
Sat Apr 18 04:04:52 UTC 2020


HI, Harold and all

Updated webrev: http://cr.openjdk.java.net/~minqi/8240204/webrev-02/
Please check embedded description below.

On 4/17/20 10:54 AM, Yumin Qi wrote:
> Hi, Harold
>
>   Thanks for the review.
>
> On 4/17/20 8:30 AM, Harold Seigel wrote:
>> Hi Yumin,
>>
>> The changes look good.  Just one question.
>>
>> InstanceKlass::set_classpath_index() returns a value. That value is 
>> only checked in one of the places from where it's called.  Is it 
>> worth checking the return value in the other places and asserting if 
>> it is false?
>>
> Let me investigate this issue first. If so, we should put the assert 
> inside set_classpath_index and change return type to 'void'.
>
After checked the code, came up with a new version. The package entry is 
generated by its name and there would be no dup in PackageEntryTable for 
same name. The new version for set_classpath_index(s2 classpath_index, 
TRAPS):

+void InstanceKlass::set_classpath_index(s2 path_index, TRAPS) {
+  if (_package_entry != NULL) {
+    DEBUG_ONLY(PackageEntryTable* pkg_entry_tbl = 
ClassLoaderData::the_null_class_loader_data()->packages();)
+    assert(pkg_entry_tbl->lookup_only(_package_entry->name()) == 
_package_entry, "Should be same");
+    assert(path_index != -1, "Unexpected classpath_index");
+    _package_entry->set_classpath_index(path_index);
+  }
+}

If class has no package, its _package_entry is NULL, and we don't need 
care about setting classpath_index for it. We save time in product for 
not looking up entry from PackageEntryTable with locking on Moudle_lock.
The assert split into two lines, since it will be too long fit into one 
line.

Also update copyright for files: classLoader.hpp, klassFactory.cpp.

Re-tested mach5 hs-tier1-4. Passed.
Local test using slowdebug on runtime. No assertion caught.
Performance on javac and zprint showed a little more improvement.

Thanks
Yumin

> Thanks
> Yumin
>> Thanks, Harold
>>
>> On 4/17/2020 1:56 AM, Yumin Qi wrote:
>>> Hi,
>>>
>>>   Please review:
>>>   bug: 8240204: https://bugs.openjdk.java.net/browse/JDK-8240204
>>>   webrev: http://cr.openjdk.java.net/~minqi/8240204/webrev-01/
>>>
>>>   Summary: Move ClassLoader::add_package to InstanceKlass since what 
>>> it does is calling pkg->set_classpath_index. Also a minor 
>>> optimization for InstanceKlass::set_package, for shared class, avoid 
>>> call check_prohibited_package since CDS does not archive prohibited 
>>> classes.
>>>
>>>   Tests: hs-tier1-4
>>>   Performance data: javac and zprint showed a little improvement.
>>>
>>>   Thanks
>>>   Yumin
>



More information about the hotspot-runtime-dev mailing list