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

[Mandy Chung]

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.

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?  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.

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??


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).

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.
Nit: the variable name "n_h" or "_n_h" would be clearer if it's named
"nest_host" or "_nest_host".
  
1689   SystemDictionary::set_shared_class_init_state(lambda_ik, InstanceKlass::loaded);

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. 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?

JVM_RegisterLambdaProxyClassForArchiving - we can add an assert to check 
if lambdaProxyClass is a hidden class. 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. 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. */ 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? Mandy