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

[Calvin Cheung]

Hi Mandy,

Thanks for taking another look.

On 6/3/20 2:07 PM, Mandy Chung wrote:
>
>
> 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.
I've removed it.
>
> 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?

I've added a check in JVM_RegisterLambdaProxyClassForArchiving. If the 
caller class is hidden or vm anonymous, it will return.

I also added 2 test cases to test the above. If the caller class is a 
hidden class, the test makes sure the corresponding lambda proxy class 
is not being archived. Currently, it doesn't seem possible to have a vm 
anonymous class to be the caller class of a lambda proxy class. I've 
added a test anyway so if things change later, we'll notice it.

>
>> 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.
Instead of putting the post loading code in the 
JVM_LookupLambdaProxyClassFromArchive function which would require 
changing some of the functions from private to public, I've renamed 
SystemDictionaryShared::load_shared_lambda_proxy_class to 
SystemDictionaryShared::prepare_shared_lambda_proxy class and moved the 
code there.
>
>> 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?
I've made the change.

Updated webrevs:

inc: http://cr.openjdk.java.net/~ccheung/jdk15/8198698/webrev_delta.02/

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

thanks,

Calvin