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

[Ioi Lam]

Hi Calvin,

[1]
I think the following name is too generic and also shouldn't be in the 
global scope. How about moving it into PackageEntry as:

class PackageEntry ... {
   static int max_index_for_defined_in_class_path() {
     return sizeof(intptr_t) * BitsPerByte;
   }
}

Also, I think it's better to use "int" as the type of "idx" for 
is_defined_by_cds_in_class_path and set_defined_in_class_path.

Both of the above two functions should assert(index < 
max_index_for_defined_in_class_path()).

  875       if (index_offset < MAX_BITMAP_BITS) {
  876         if (pkg_entry == NULL || 
!pkg_entry->is_defined_by_cds_in_class_path(index_offset)) {
   >>> add comments about the optimization?

[2] In the test case:

   66         // Test loading of two classes from the same package from 
different jars.
   67         // First loaded class is from a non-sealed package, the 
second loaded
   68         // class is from the same package but sealed.
   69         // The expected result is a SecurityException with a 
"sealing violation".

How about

   69         // The expected result is a SecurityException with a 
"sealing violation"
              // for the second class.

and then, check in the output that the first class was successfully loaded.

I think we should also have another test case for:

          output = TestCommon.exec(jars, "PackageSealingTest",
                                   "sealed/pkg/C1", "sealed", 
"sealed/pkg/C3", "notSealed");
          TestCommon.checkExec(output, ".....");

in this case, C3 should fail to load.

Thanks
- Ioi



On 6/24/20 3:53 PM, 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.
>
> 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
>