RFR(XL) 8198698: Support Lambda proxy classes in dynamic CDS archive

Calvin Cheung calvin.cheung at oracle.com
Wed Jun 3 19:34:10 UTC 2020


Hi Mandy,

Thanks for your review.

On 6/2/20 11:38 AM, Mandy Chung wrote:
> Hi Calvin,
>
> To recap an offline discussion, I think we should use JDK bootcycle 
> build to test this feature.  For example generating a boot JDK with 
> archived lambda proxy classes for java.base and javac to build itself 
> and also use it to run JDK tier 1-3 tests. This would serve a good 
> test bed.   I also suggest to experiment how JDK can benefit from this 
> feature (e.g.  any java.base and javac performance gain).  The module 
> system initialization had to avoid using lambdas at startup time and 
> we could re-examine this again.

This RFE supports archiving the lambda proxy classes in a dynamic 
archive. In order to exercise this feature during JDK build time, we 
probably need to include the lambda proxy classes in the default CDS 
archive. Or have another dynamic CDS archive on top of the default CDS 
archive. The dynamic archive may initially contain the classes required 
by javac to compile a java program, like a HelloWorld. I think this 
should be explored as a separate RFE.

I can get some perf numbers on running javac on HelloWorld later.

>
> On 5/29/20 2:29 PM, Calvin Cheung wrote:
>> RFE: https://bugs.openjdk.java.net/browse/JDK-8198698
>>
>> webrev: http://cr.openjdk.java.net/~ccheung/jdk15/8198698/webrev.00/
>>
>
> Hidden classes have the following properties:
> 1. not discoverable by name.  It's not registered in the system 
> dictionary.
> 2. It can be a dynamic nest member of a nest host.  For lambda proxy 
> class, it has the same nest host as the caller class (same runtime 
> package as the caller class)
> 3. strong or weak link with the class loader.  a hidden class may be 
> unloaded even its defining class loader is alive.
>
> For the archived lambda proxy classes, do you re-assign a new class 
> name to the shared lambda proxy class?
No, I didn't assign a new class name to a shared lambda proxy class.
> I suspect not.  It's very rare that it will conflict with the name of 
> other hidden classes.  It's an option to patch the name to indicate 
> it's from shared archive if it helps diagnosability.

In the class load trace (-Xlog:class+load), one will see the following 
if the lambda proxy class is loaded from a dynamic archive:

[0.428s][info ][class,load   ] 
BasicLambdaApp$$Lambda$1/0x0000000800bd5258 source: shared objects file 
(top)

>
> My comments below are related to the above.
>
> 1374 InstanceKlass* 
> SystemDictionary::load_shared_lambda_proxy_class(InstanceKlass* ik,
> 1375 Handle class_loader,
> 1376 Handle protection_domain,
> 1377 PackageEntry* pkg_entry,
> 1378 TRAPS) {
> 1379 InstanceKlass* shared_n_h = 
> SystemDictionaryShared::get_shared_nest_host(ik);
> 1380 assert(shared_n_h->is_shared(), "nest host must be in CDS archive");
> 1381 Symbol* cn = shared_n_h->name();
> 1382 Klass *s = resolve_or_fail(cn, class_loader, protection_domain, 
> true, CHECK_NULL);
> 1383 if (s != shared_n_h) {
> 1384 // The dynamically resolved nest_host is not the same as the one 
> we used during dump time,
> 1385 // so we cannot use ik.
> 1386 return NULL;The above resolve_or_fail is to ensure that H is the 
> same class loaded by the given class loader.  Another validation that you should do is to check
> the nest host and nest member must be in the same runtime package -
> maybeik->set_nest_host(shared_n_h, THREAD) validates the runtime package??

I've added an assert:

1398   InstanceKlass* loaded_ik = load_shared_class(ik, class_loader, 
protection_domain, NULL, pkg_entry, CHECK_NULL);
1399
1400   assert(shared_nest_host->is_same_class_package(ik),
1401          "lambda proxy class and its nest host must be in the same 
package");

The assert needs to be after load_shared_class so that the class loader 
and PackageEntry have been setup.

>
>
> SystemDictionaryShared::load_shared_lambda_proxy_class
>   
> 1679   InstanceKlass* shared_n_h = get_shared_nest_host(lambda_ik);
> 1680   assert(shared_n_h != NULL, "unexpected NULL _nest_host");
> 1681
> 1682   InstanceKlass* loaded_lambda =
> 1683     SystemDictionary::load_shared_lambda_proxy_class(lambda_ik, class_loader, protection_domain, pkg_entry, CHECK_NULL);
> 1684
> 1685   InstanceKlass* n_h = caller_ik->nest_host(THREAD);
> 1686   assert(n_h->has_nest_member(caller_ik, THREAD) ||
> 1687          n_h == caller_ik, "mismatched nest host");
>
> I think you meant to check n_h->has_nest_member(loaded_lambda, THREAD).

I saw David has commented on this. So I'll leave the assert as before 
and I've added another assert (see line 1691):

1687   // The following ensures that the caller's nest host is the same 
as the lambda proxy's
1688   // nest host recorded at dump time.
1689   assert(nest_host->has_nest_member(caller_ik, THREAD) ||
1690          nest_host == caller_ik, "caller_ik failed nest member check");
1691   assert(nest_host == shared_nest_host, "mismatched nest host");

>
> It'd be good to add a comment to say that this ensures thatthe caller's nest host must be the same as the lambda proxy's nest 
> host recorded at dump time.
Done.
>
> Nit: the variable name "n_h" or "_n_h" would be clearer if it's named
> "nest_host" or "_nest_host".
I've renamed n_h to nest_host.
>   
> 1689   SystemDictionary::set_shared_class_init_state(lambda_ik, InstanceKlass::loaded);
It turns just setting the lambda_ik to the InstanceKlass::loaded state 
is not enough. We need to do some of the stuff which are currently being 
done in parse_stream. Please refer to the change in 
SystemDictionary::load_shared_lambda_proxy_class.
>
> A lambda proxy class typically is eagerly initialized (unless a flag is given).
> Your change dumps the hidden class with no knowledge if `Lookup::defineHiddenClass`
> is called with "initialize" parameter is true or not.  This may affect the runtime
> performance (for example MethodHandle will have class init barrier if the class
> is not yet initialized).
> This may have performance implication. I think we should consider 
> passing the initialize parameter to LambdaProxyClassArchive::find such 
> that if the class is found from CDS archive, it will invoke <clinit> 
> before it returns to the library.

In InnerClassLambdaMetafactory, the !disableEagerInitialization will be 
passed to LambdaProxyClassArchive.find which will pass down to vm.

  271         Class<?> innerClass = 
LambdaProxyClassArchive.find(targetClass,
  272 samMethodName,
  273 invokedType,
  274 samMethodType,
  275 implMethod,
  276 instantiatedMethodType,
  277 isSerializable,
  278 markerInterfaces,
  279 additionalBridges,
  280 !disableEagerInitialization);

In SystemDictionary::load_shared_lambda_proxy_class, it checks the flag:

1422   if (initialize) {
1423     loaded_ik->initialize(CHECK_NULL);
1424   }

On a related note, in the existing jvm_lookup_define_class in jvm.cpp:

   if (init) {
     ik->initialize(CHECK_NULL);
   } else {
     ik->link_class(CHECK_NULL);
   }

I don't think the else is necessary as the ik->link_class(CHECK_NULL) 
has been done within the SystemDictionary::parse_stream.

> One other point I brought up - strong vs weak hidden class (i.e. is it 
> using the primary CLD or the per-mirror CLD?). I have lost myself 
> where you do this in the implementation. Can you point that out? Is it 
> recorded in the shared archive and make sure it uses the exact CLD 
> when it's loaded?

Currently, the strong vs weak hidden class isn't recorded in the archive.

Because in InnerClassLambdaMetafactory.generateInnerClass, there's the 
following.

Lookup lookup = caller.defineHiddenClass(classBytes, 
!disableEagerInitialization, NESTMATE, STRONG);

Also, the above line of code is after the call to 
LambdaProxyClassArchive.register. (Refer to call stack below.)

  1 - frame( sp=0x00007ffff7fc2730, unextended_sp=0x00007ffff7fc2730, 
fp=0x00007ffff7fc27c8, pc=0x00007fffd8d3daca)
java.lang.ClassLoader.defineClass0(Native Method)
  2 - frame( sp=0x00007ffff7fc27d8, unextended_sp=0x00007ffff7fc27e8, 
fp=0x00007ffff7fc2880, pc=0x00007fffd8d34e00)
java.lang.System$2.defineClass(System.java:2193)
  3 - frame( sp=0x00007ffff7fc2890, unextended_sp=0x00007ffff7fc2890, 
fp=0x00007ffff7fc2920, pc=0x00007fffd8d34ee3)
java.lang.invoke.MethodHandles$Lookup$ClassDefiner.defineClass(MethodHandles.java:2181)
  4 - frame( sp=0x00007ffff7fc2930, unextended_sp=0x00007ffff7fc2950, 
fp=0x00007ffff7fc29b0, pc=0x00007fffd8d34e00)
java.lang.invoke.MethodHandles$Lookup$ClassDefiner.defineClassAsLookup(MethodHandles.java:2163)
  5 - frame( sp=0x00007ffff7fc29c0, unextended_sp=0x00007ffff7fc29c8, 
fp=0x00007ffff7fc2a20, pc=0x00007fffd8d34e00)
java.lang.invoke.MethodHandles$Lookup.defineHiddenClass(MethodHandles.java:1961)
  6 - frame( sp=0x00007ffff7fc2a30, unextended_sp=0x00007ffff7fc2a30, 
fp=0x00007ffff7fc2a98, pc=0x00007fffd8d34e00)
java.lang.invoke.InnerClassLambdaMetafactory.generateInnerClass(InnerClassLambdaMetafactory.java:379)
  7 - frame( sp=0x00007ffff7fc2aa8, unextended_sp=0x00007ffff7fc2ae8, 
fp=0x00007ffff7fc2b38, pc=0x00007fffd8d34e00)
java.lang.invoke.InnerClassLambdaMetafactory.spinInnerClass(InnerClassLambdaMetafactory.java:280)
  8 - frame( sp=0x00007ffff7fc2b48, unextended_sp=0x00007ffff7fc2b50, 
fp=0x00007ffff7fc2ba0, pc=0x00007fffd8d34e00)
java.lang.invoke.InnerClassLambdaMetafactory.buildCallSite(InnerClassLambdaMetafactory.java:209)
  9 - frame( sp=0x00007ffff7fc2bb0, unextended_sp=0x00007ffff7fc2bc8, 
fp=0x00007ffff7fc2c18, pc=0x00007fffd8d34e00)
java.lang.invoke.LambdaMetafactory.metafactory(LambdaMetafactory.java:328)
10 - frame( sp=0x00007ffff7fc2c28, unextended_sp=0x00007ffff7fc2c28, 
fp=0x00007ffff7fc2ca8, pc=0x00007fffd8d34e00)
java.lang.invoke.DirectMethodHandle$Holder.invokeStatic
11 - frame( sp=0x00007ffff7fc2cb8, unextended_sp=0x00007ffff7fc2cc0, 
fp=0x00007ffff7fc2d40, pc=0x00007fffd8d34e00)
java.lang.invoke.Invokers$Holder.invokeExact_MT
12 - frame( sp=0x00007ffff7fc2d50, unextended_sp=0x00007ffff7fc2d50, 
fp=0x00007ffff7fc2dd8, pc=0x00007fffd8d34e00)
java.lang.invoke.BootstrapMethodInvoker.invoke(BootstrapMethodInvoker.java:127)
13 - frame( sp=0x00007ffff7fc2de8, unextended_sp=0x00007ffff7fc2e30, 
fp=0x00007ffff7fc2ea8, pc=0x00007fffd8d34e00)
java.lang.invoke.CallSite.makeSite(CallSite.java:315)
14 - frame( sp=0x00007ffff7fc2eb8, unextended_sp=0x00007ffff7fc2ec8, 
fp=0x00007ffff7fc2f38, pc=0x00007fffd8d34e00)
java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:270)
15 - frame( sp=0x00007ffff7fc2f48, unextended_sp=0x00007ffff7fc2f50, 
fp=0x00007ffff7fc2fc8, pc=0x00007fffd8d34e00)
java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:260)
C frame (sp=0x00007ffff7fc2fd8 unextended sp=0x00007ffff7fc2ff8, 
fp=0x00007ffff7fc3090, real_fp=0x00007ffff7fc3090, pc=0x00007fffd8d2b9ca)
(~Stub::call_stub)
      BufferBlob (0x00007fffd8d2b810) used for StubRoutines (1)

16 - frame( sp=0x00007ffff7fc3880, unextended_sp=0x00007ffff7fc3880, 
fp=0x00007ffff7fc38c8, pc=0x00007fffd8d542d3)
LambHello.main(LambHello.java:3)

-----

For now, I've added an assert in 
JVM_RegisterLambdaProxyClassForArchiving to make sure the hidden class 
is strong so that when things changed later, we'll notice it.

I also added a check in 
SystemDictionaryShared::is_registered_lambda_proxy_class to make sure 
the hidden class is strong.

As for the CLD, I didn't do anything special. In 
SystemDictionary::load_shared_class:

1470   ClassLoaderData* loader_data = 
ClassLoaderData::class_loader_data(class_loader());

It is consistent with what is being done in parse_stream for a strong 
hidden class.

>
>
> JVM_RegisterLambdaProxyClassForArchiving - we can add an assert to 
> check if lambdaProxyClass is a hidden class.
Added.
> JVM_IsCDSDumpingEnabled and JVM_IsCDSSharingEnabled - an alternative 
> library/VM interface is to use internal system properties that 
> jdk.internal.misc.VM::saveProperties reads and removes these flags.
I'd prefer to leave it as is as there's no perf issue if CDS is not 
enabled. (More perf data below.)
> InnerClassLambdaMetafactory 247 /** Use the LambdaProxyClassArchive 
> for including the lambda proxy class 248 * in CDS archive and 
> retrieving the class from the archive. If the class 249 * is not found 
> in the archive, it calls generatesInnerClass to create 250 * the 
> class. 251 */ What about simpler comment: /* * Spins the lambda proxy 
> class. * * This first checks if a lambda proxy class can be loaded 
> from CDS archive. * Otherwise, generate the lambda proxy class. */

I've updated the comment.


> This adds a VM call for every lambda proxy class creation. Do you have 
> any the performance measurement when CDS is not on? Any performance 
> regression?

Here's the perf data with CDS disabled. The data is for loading 200 
lambda proxy classes.

Results of " perf stat -r 50 bin/java -Xshare:off -cp TestD.jar TestD 200 "
    1:    507523520   506743227 ( -780293)      --       156.992 156.453 
( -0.539)      --
    2:    506280039   507494909 ( 1214870)   +++         155.799 156.510 
(  0.711)    ++
    3:    506600572   507544573 (  944001)    ++         155.100 156.878 
(  1.778) +++++
    4:    506240025   508590609 ( 2350584) +++++         155.191 155.745 
(  0.554)    ++
    5:    507113653   506861905 ( -251748)      -        155.089 156.126 
(  1.037)   +++
    6:    506331554   507727117 ( 1395563)   +++         155.588 155.452 
( -0.136)
    7:    506309111   506724285 (  415174)     +         156.407 156.914 
(  0.507)     +
    8:    507805169   506926943 ( -878226)      --       155.796 155.922 
(  0.126)
    9:    506970117   507097707 (  127590)               155.538 156.244 
(  0.706)    ++
   10:    506951104   506754063 ( -197041)               155.429 156.747 
(  1.318)  ++++
============================================================
          506812213   507246215 (  434002)     +         155.692 156.298 
(  0.607)    ++
instr delta =       434002     0.0856%
time  delta =        0.607 ms  0.3896%

Updated webrevs:

delta: http://cr.openjdk.java.net/~ccheung/jdk15/8198698/webrev_delta.01/

full: http://cr.openjdk.java.net/~ccheung/jdk15/8198698/webrev.01/

thanks,

Calvin



More information about the hotspot-runtime-dev mailing list