RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive
Yumin Qi
yumin.qi at oracle.com
Mon Aug 17 19:37:31 UTC 2020
Hi, Ioi
Thanks for review/suggestion. I have updated the webrev at the following link:
http://cr.openjdk.java.net/~minqi/2020/8247536/webrev-02/
Following changes done in this patch:
1) move regenerating holder classes into new added file: lambdaFormInvokers.[ch]pp
"@lambda-form-invoker" tag only observed at jvm side.
2) remove InvokerBytecodeGeneratorHelper.java, move related functions to existing class, GenerateJLIClassesHelper.java
3) add tracing for SPECIES_RESOLVE, so archive more classes.
4) remove InvokerGenerateBytesException.java, using RuntimeException instead.
5) Changed make file to exclude the new added lamdaFormInvokers.cpp from the non-cds building list.
6) There is a bug in previous patch, jobject (typeArrayOop) should not be used directly for loading class since GC will move the java object. Fixed by copying the bytes from handle to C heap.
7) Tested with tier1,2
Thanks
Yumin
On 8/15/20 6:27 PM, Ioi Lam wrote:
> On 8/15/20 6:19 PM, Ioi Lam wrote:
>> 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
>>
>
> Oops, sent too quickly. The design doc's title is also updated:
>
> https://wiki.openjdk.java.net/display/HotSpot/Support+for+pre-generated+java.lang.invoke+classes+in+CDS+archive
>
>> 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
>>>>
>>>> `
>>
>
More information about the hotspot-runtime-dev
mailing list