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

[Mandy Chung]

On 6/3/20 12:34 PM, Calvin Cheung wrote:
>
> 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");
>

I don't think this assert is needed.  caller_ik can be a hidden class 
and so this assert is not correct then.

Is there any issue to archive lambda proxy class whose caller is a 
hidden class?  Is there any assumption in the CDS implementation that 
the caller class is always a normal class?

> 1691   assert(nest_host == shared_nest_host, "mismatched nest host");
>

This is good.

>
> In SystemDictionary::load_shared_lambda_proxy_class, it checks the flag:
>
> 1422   if (initialize) {
> 1423     loaded_ik->initialize(CHECK_NULL);
> 1424   }
>


I think JVM_LookupLambdaProxyClassFromArchive is a more appropriate 
place to link and initialize the class before return.   I expect 
load_shared_lambda_proxy_class does loading only and linking and 
initialization should be separate from loading.

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

Harold and Lois can chime in here.  I think ik->link_class may be for 
unsafe anonymous class to prepare for constant pool patching.

> Currently, the strong vs weak hidden class isn't recorded in the archive.
>
> :
>
> -----
>
> 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.
>

An assert is good.


3752   if (invokedName == NULL || invokedType == NULL || methodType == 
NULL ||
3753       implMethodMember == NULL || instantiatedMethodType == NULL) {
3754     return NULL;
3755   }


Should this throw NPE instead?

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

Actually the Java side checks if CDS is enabled before it calls the VM.  
So it's good.

Mandy