RFR: 8247666: Support Lambda proxy classes in static CDS archive

[Ioi Lam]

On Fri, 25 Sep 2020 17:52:24 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%

I have reviewed a small part of the changes. Here are my initial comments

src/hotspot/share/classfile/systemDictionaryShared.cpp line 336:

> 334:
> 335:   static unsigned int dumptime_hash(Symbol* sym)  {
> 336:     if (sym == NULL) {

How about adding a comment "_invoked_name may be NULL"?

src/hotspot/share/classfile/systemDictionaryShared.cpp line 2300:

> 2298:
> 2299: class ArchivedLambdaMirrorPatcher {
> 2300:   static void update(Klass* k) {

I think ArchivedLambdaMirrorPatcher can be subclass of ArchivedMirrorPatcher. That way you can share the same code for
ArchivedMirrorPatcher::update (you can make this a protected method).

src/hotspot/share/classfile/systemDictionaryShared.cpp line 2242:

> 2240:         st->print_cr("Dynamic Shared Lambda Dictionary");
> 2241:         SharedLambdaDictionaryPrinter ldp(st);
> 2242:         _dynamic_lambda_proxy_class_dictionary.iterate(&ldp);

I think this function can be refactored, something like:

SystemDictionaryShared::print_on(outputStream* st) {
   if (UseSharedSpaces) {
     print_on("", &_builtin_dictionary, &_unregistered_dictionary, &_lambda_proxy_class_dictionary);
     if (DynamicArchive::is_mapped()) {
       print_on("Dynamic ", &dynamic__builtin_dictionary, &_dynamic_unregistered_dictionary,
         &_dynamic_lambda_proxy_class_dictionary);
    }
  }
}

src/hotspot/share/classfile/systemDictionaryShared.cpp line 1984:

> 1982: int compare_runtime_lambda_proxy_class_info(const RunTimeLambdaProxyClassNode& r1,
> 1983:                                             const RunTimeLambdaProxyClassNode& r2) {
> 1984:   ResourceMark rm;

Is this function being used?

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

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