RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive

Sun Aug 16 01:19:55 UTC 2020

To better capture what we're trying to do in this RFE, I've changed the 
RFE title (and the subject of this email thread) to

https://bugs.openjdk.java.net/browse/JDK-8247536
Support for pre-generated java.lang.invoke classes in CDS static archive

I also update the RFE Description to give an overview of the problem and 
the solution.

The design document is also updated to reflect Yumin's latest implementation

https://wiki.openjdk.java.net/display/HotSpot/JDK-8247536+Support+pre-generated+MethodHandle+LambdaForms+in+CDS

Thanks
- Ioi

On 8/12/20 11:54 AM, calvin.cheung at oracle.com wrote:
> Hi Yumin,
>
> I reviewed mostly the native code. Below are my comments:
>
> 1) classListParser.hpp
>
> 71   bool                _lambda_format;
>
> The above name is too generic. How about _lambda_form or _is_lambda_form?
> If you rename the above, please also rename the function which returns 
> the above bool.
>
> 2) jvm.cpp
>
> 3850 JVM_ENTRY(void, JVM_CDSTraceResolve(JNIEnv* env, jclass ignore, 
> jstring line))
>
> ignore -> ignored
>
> 3) jvm.hpp
>
> 210 JVM_CDSTraceResolve(JNIEnv* env, jclass ignore, jstring line);
>
> Same comment as for jvm.cpp
>
> 4) metaspaceShared.cpp
>
> 2017   size_t i = 0;
> 2018   while (i < size) {
> 2019     full_name[i++] = *start++;
> 2020   }
>
> Could the above be simplified to the following?
>
>     strncpy(full_name, start, size - 1);
>
> 2029   char* class_name = get_full_class_name(path_name);
>
> Should os::free(class_name) be called before the function returns?
>
> 1870 static GrowableArray<char *>* lambda_list = NULL;
>
> The name lambda_list is a bit generic, how about lambda_form_list?
>
> 2112       lambda_list->append(parser.current_line());
>
> Since parser.current_line() does a strdup, do those buffer need to be 
> freed after its use? (i.e. after the call to 
> regenerate_holder_classes()?)
>
> In MetaspaceShared::regenerate_holder_classes, before calling up to 
> java, I think it's better to strip the prefix "@lambda-form-invoker" 
> from the strings. So that the java 
> InvokerBytecodeGeneratorHelper.readTraceConfig method doesn't need to 
> handle it:
>
>  143                     case "[LF_RESOLVE]":
>  144                     case InvokerBytecodeGenerator.CDS_LOG_PREFIX:
>
> 5) DumpSymbolAndStringTable.java
>
> 37     private static final String my_string = 
> "DumpSymbolAndStringTable";
>
> Unused variable?
>
> thanks,
>
> Calvin
>
> On 8/11/20 10:36 AM, Yumin Qi wrote:
>> Forget to send to @core-lib-dev, the patch changed jdk code.
>>
>>
>> Thanks
>>
>> Yumin
>>
>>
>>
>> -------- Forwarded Message --------
>> Subject:     RFR: 8247536: Support pre-generated MethodHandle lambda 
>> forms in CDS
>> Date:     Tue, 11 Aug 2020 07:44:34 -0700
>> From:     Yumin Qi <yumin.qi at oracle.com>
>> To:     hotspot-runtime-dev at openjdk.java.net
>>
>>
>>
>> Hi, Please reivew
>>
>>   bug: https://bugs.openjdk.java.net/browse/JDK-8247536
>>   Webrev: http://cr.openjdk.java.net/~minqi/2020/8247536/webrev-01/
>>
>>   Summary: CDS does not archive pre-generated lambda form classes for 
>> method handles:
>>
>> [0.142s][info][class,load] 
>> java.lang.invoke.LambdaForm$MH/0x0000000800066c40 source: 
>> __JVM_LookupDefineClass__
>>
>> The method handle resolution if not found in the holder class, a 
>> class with the method will be generated and loaded as vm internal 
>> created class and not archived like above. Those class names are 
>> mangled as combination of the class name and the class instance address.
>>
>> In this patch, collect those method holder class names and their 
>> associated methods' signatures when user specify DumpLoadedClassList, 
>> and record them in the log file in a format similar to the output 
>> format from traced by 
>> -Djava.lang.invoke.MethodHandle.TRACE_RESOLVE=true, but here use 
>> another name for CDS: 
>> -Djava.lang.invoke.MethodHandle.CDS_TRACE_RESOLVE=true instead. The 
>> line prefix also changed from "[LF_RESOLVE]" to 
>> "@lambda-invoke-handle".  At dump time, regenerate the holder class 
>> with those methods and replace the existing holder class and archived 
>> it. At runtime, the resolution of the holder class which contains 
>> those methods are loaded from CDS archive so avoid regenerating them 
>> again. The patch reorganized the code for Jlink plugin, so the new 
>> added InvokerBytecodeGeneratorHelper can be shared both by JLink 
>> plugin and CDS code. Also change made to the mangled generated class 
>> name at static dump, the class name combining the class name and a 
>> sequentially ordered number instead of the class instance address, 
>> which looks like a random.
>>
>>
>> To use this feature, one can do the dump/run just like done for a 
>> static dump/run, so no extra steps required.
>>
>>   1) java -XX:DumpLoadedClassList=myclasslist JavaApp
>>
>>       DumpLoadedClassList will turn on 
>> -Djava.lang.invoke.MethodHandle.CDS_TRACE_RESOLVE=true
>>
>>   2) java -Xshare:dump -XX:SharedClassListFile=myclasslist 
>> -XX:SharedArchiveFile=my.jsa JavaApp
>>
>>   3) java -Xshare:on -XX:SharedArchiveFile=my.jsa JavaApp
>>
>>   The performance on javac HelloWorld.java (javac-benchmark), no 
>> patch vs patch:
>>
>>    1:   2689285002  2641821474 (-47463528)      ----     391.720 
>> 382.990 ( -8.730)      ----
>>    2:   2687495085  2632969688 (-54525397)      ----     391.030 
>> 381.480 ( -9.550)      -----
>>    3:   2694664066  2636523114 (-58140952)      -----    390.610 
>> 382.550 ( -8.060)      ----
>>    4:   2686554164  2639355233 (-47198931)      ----     390.700 
>> 383.390 ( -7.310)      ---
>>    5:   2691072338  2633016687 (-58055651)      -----    388.990 
>> 382.360 ( -6.630)      ---
>>    6:   2684448174  2644191854 (-40256320)      ---      389.450 
>> 382.990 ( -6.460)      ---
>>    7:   2694921227  2630505090 (-64416137)      -----    389.300 
>> 383.160 ( -6.140)      ---
>>    8:   2685209712  2639334320 (-45875392)      ----     388.370 
>> 381.060 ( -7.310)      ---
>>    9:   2695885942  2640618655 (-55267287)      ----     389.560 
>> 381.100 ( -8.460)      ----
>>   10:   2689162942  2635658943 (-53503999)      ----     389.690 
>> 379.110 (-10.580)      -----
>> ============================================================
>>         2689866989  2637396210 (-52470778)      ----     389.941 
>> 382.017 ( -7.924)      ----
>>
>> instr delta =    -52470778    -1.9507%
>>
>> time  delta =       -7.924 ms -2.0321%
>>
>>   It will show much more improvement if compare with the default 
>> archive:
>>
>>                  no patch                        patch
>>
>> instr         3,996,847,605        3,320,928,995
>>
>> time(s).     0.686835162          0.474933546
>>
>>
>>   Tests:
>>
>> 1)jtreg on jdk/tools and hotspot/runtime local.
>>
>> 2) mach5 ter1,2
>>
>>
>> Thanks
>>
>> Yumin
>>
>>   `