RFR(S): 8243586: Optimize calls to SystemDictionaryShared::define_shared_package for classpath

David Holmes david.holmes at oracle.com
Mon Jun 29 03:04:25 UTC 2020


Hi Calvin,

Generally looks okay but a few comments/suggestions.

On 25/06/2020 8:53 am, Calvin Cheung wrote:
> JBS: https://bugs.openjdk.java.net/browse/JDK-8243586
> 
> webrev: http://cr.openjdk.java.net/~ccheung/jdk16/8243586/webrev.00/
> 
> The proposed change is to reduce the calls to 
> SystemDictionaryShared::define_shared_package which does a java call to 
> AppClassLoader.defineOrCheckPackage. Currently, define_shared_package is 
> called for every shared class but it is needed only for each package in 
> each jar specified in the classpath.

src/hotspot/share/classfile/packageEntry.hpp

+   volatile intptr_t _defined_in_class_path; // a Package java object 
has been define via CDS

The name and description of this field is somewhat lacking. It is a bit 
map indicating which CDS classpath entries have defined classes in this 
package.

+   bool is_defined_by_cds_in_class_path(intptr_t idx) const {
+   void set_defined_in_class_path(intptr_t idx) {

These names should be consistent i.e.

+   void set_defined_by_cds_in_class_path(intptr_t idx) {

I agree with Ioi that the idx should be range checked by an assert.

test/hotspot/jtreg/runtime/cds/appcds/PackageSealing.java

I don't really understand what this test was doing previously, but it is 
not obvious that it is exercising the new bitmap logic in any rigorous 
way. Is this test change really related to the code change?

    * @compile test-classes/C1.java
    * @compile test-classes/C2.java
+  * @compile test-classes/C3.java
    * @compile test-classes/PackageSealingTest.java
    * @compile test-classes/Hello.java

Unless there is a reason separate compilation is required, the number of 
@compile commands can be reduced - perhaps to one.

+         classList[1] = "sealed/pkg/C3"; // C3 is from a non-sealed package

The comment seems in contradiction to the path - is it sealed or not?

+         TestCommon.testDump(jars, TestCommon.list(classList2), 
"-XX:+TraceExceptions");

TraceExceptions is deprecated - use -Xlog:exceptions=info

Thanks,
David
-----

> zprint perf results summary:
> 
> instr delta = -104380126 -2.2487%
> time delta = -23.779 ms -3.3640%
> 
> Testing: running mach5 tier1 and 2 tests.
> 
> thanks,
> 
> Calvin
> 


More information about the hotspot-runtime-dev mailing list