RFR: 8247666: Support Lambda proxy classes in static CDS archive [v3]

[Calvin Cheung]

On Fri, 2 Oct 2020 22:47:43 GMT, Mandy Chung <mchung at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev
>> excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since
>> the last revision:
>>  - Merge branch 'master' into 8247666
>>  - exclude archived lambda proxy classes in the classlist
>>  - updated systemDictionaryShared.[c|h]pp based on suggestions from Ioi
>>  - fix extraneous whitespace
>>  - 8247666 (initial commit)
>
>> @lambda-proxy: test/java/lang/invoke/MethodHandlesGeneralTest 233
> @lambda-proxy: test/java/lang/invoke/MethodHandlesGeneralTest 355
> 
> It seems very fragile to require listing the CP index to `invokedynamic` entries of a class file.   Have you considered
> a simpler usage model without CP indices and default to all `invokedynamic` to `LambdaMetaFactory`?

Hi Mandy,
Thanks for your review.
We've tried the approach of archiving all the lambda proxy classes of a given class but we feel that there will be too
many unused lambda proxy classes in the archive and it will also increase the CDS archive size. So we opted for a
little more complicated approach by storing the symbolic encoding lambda proxy resolution in the classlist. During
dumping, the same info will be retrieved from the constant pool entry and compared with the one from the classlist. An
example of the entry in the classlist is: `@lambda-proxy: LambHello run ()Ljava/lang/Runnable; ()V 6 LambHello
lambda$main$0 ()V ()V` With the above approach, the number of lambda proxy classes the default CDS archive is 67. If we
archive all the lambda proxy classes of a given class, the number is 279. Webrev 03 contains the above change and also
addresses your other comments. Thanks! Calvin

> src/java.base/share/classes/jdk/internal/misc/CDS.java line 56:
> 
>> 54:      * Check if CDS dumping is enabled via the DynamicDumpSharedSpaces or the DumpSharedSpaces flag.
>> 55:      */
>> 56:     public static native boolean isCDSDumpingEnabled();
> 
> I suggest to rename `CDS::isCDSDumpingEnabled` to `CDS:isDumpingEnabled` as this method is a static method in `CDS`
> case and the word `CDS` in the method name is just noise.
> JVM entry point `JVM_IsCDSDumpingEnabled`  is good.

Fixed in webrev 03.

> src/hotspot/share/prims/jvm.cpp line 3834:
> 
>> 3832:
>> 3833: JVM_ENTRY(jboolean, JVM_IsCDSDumpingEnabled(JNIEnv* env))
>> 3834:     JVMWrapper("JVM_IsCDSDumpingEnable");
> 
> typo: missing `d` s/Enable/Enabled/

Fixed in webrev 03.

-------------

PR: https://git.openjdk.java.net/jdk/pull/364