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

[Mandy Chung]

On Fri, 2 Oct 2020 16:30:01 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> Following up on archiving lambda proxy classes in dynamic CDS archive
>> ([JDK-8198698](https://bugs.openjdk.java.net/browse/JDK-8198698)), this RFE adds the functionality of archiving of
>> lambda proxy classes in static CDS archive.
>> When the -XX:DumpLoadedClassList is enabled, the constant pool index related to LambdaMetafactory that are resolved
>> during application execution will be included in the classlist. The entry for a lambda proxy class in a class list will
>> be of the following format:
>> `@lambda-proxy: <classname> <cp index>`
>> 
>> e.g.
>> `@lambda-proxy: test/java/lang/invoke/MethodHandlesGeneralTest 233`
>> `@lambda-proxy: test/java/lang/invoke/MethodHandlesGeneralTest 355`
>> 
>> When dumping a CDS archive using the -Xshare:dump and -XX:ExtraSharedClassListFile options, when the above
>> `@lambda-proxy` entry is encountered while parsing the classlist, we will resolve the corresponding constant pool
>> indices (233 and 355 in the above example). As a result, lambda proxy classes will be generated for the constant pool
>> entries, and will be cached using a similar mechanism to JDK-8198698.  During dumping, there is check on the cp index
>> and on the created BootstrapInfo using the cp index. VM will exit with an error message if the check has failed.
>> During runtime when looking up a lambda proxy class, the lookup will be perform on the static CDS archive and if not
>> found, then lookup from the dynamic archive if one is specified.  (Only name change (IsDynamicDumpingEnabled ->
>> IsCDSDumpingEnabled) is involved in the core-libs code.)
>> Testing: tiers 1,2,3,4.
>> 
>> Performance results (javac on HelloWorld on linux-x64):
>> Results of " perf stat -r 40 bin/javac -J-Xshare:on -J-XX:SharedArchiveFile=javac2.jsa Bench_HelloWorld.java "
>>    1:   2228016795  2067752708 (-160264087)      -----    377.760   349.110 (-28.650)      -----
>>    2:   2223051476  2063016483 (-160034993)      -----    374.580   350.620 (-23.960)      ----
>>    3:   2225908334  2067673847 (-158234487)      -----    375.220   350.990 (-24.230)      ----
>>    4:   2225835999  2064596883 (-161239116)      -----    374.670   349.840 (-24.830)      ----
>>    5:   2226005510  2061694332 (-164311178)      -----    373.512   351.120 (-22.392)      ----
>>    6:   2225574949  2062657482 (-162917467)      -----    374.710   348.380 (-26.330)      -----
>>    7:   2224702424  2064634122 (-160068302)      -----    373.670   349.510 (-24.160)      ----
>>    8:   2226662277  2066301134 (-160361143)      -----    375.350   349.790 (-25.560)      ----
>>    9:   2226761470  2063162795 (-163598675)      -----    374.260   351.290 (-22.970)      ----
>>   10:   2230149089  2066203307 (-163945782)      -----    374.760   350.620 (-24.140)      ----
>> ============================================================
>>         2226266109  2064768307 (-161497801)      -----    374.848   350.126 (-24.722)      ----
>> instr delta =   -161497801    -7.2542%
>> time  delta =      -24.722 ms -6.5951%
>
> 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`?

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.

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/

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

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