Fwd: RFR: 8247536: Support pre-generated MethodHandle lambda forms in CDS
calvin.cheung at oracle.com
calvin.cheung at oracle.com
Wed Aug 12 18:54:27 UTC 2020
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