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