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

Yumin Qi minqi at openjdk.java.net
Wed Oct 7 19:44:29 UTC 2020


On Tue, 6 Oct 2020 18:12:50 GMT, Mandy Chung <mchung at openjdk.org> wrote:

>> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Removed unused imports.
>
> src/java.base/share/classes/jdk/internal/misc/CDS.java line 83:
> 
>> 81:      * check if -XX:+DumpLoadedClassList and given file is open
>> 82:      */
>> 83:     public static boolean isDumpLoadedClassList() {
> 
> I agree with Ioi's suggestion to rename this to `isDumpingClassList` which describes what the VM is doing.

Done

> src/java.base/share/classes/java/lang/invoke/GenerateJLIClassesHelper.java line 74:
> 
>> 72:                 System.out.println(traceSP + (salvage != null ? " (salvaged)" : " (generated)"));
>> 73:             }
>> 74:             CDS.traceLambdaFormInvoker(traceSP);
> 
> I suggest leaving the existing code unchanged.  Instead, add the following:
>     if (CDS.isDumpingClassList()) {
>          CDS.traceSpeciesType(cn);
>     }
> 
> The above uses Ioi's suggested method name which reads better.

Done

> src/java.base/share/classes/java/lang/invoke/GenerateJLIClassesHelper.java line 63:
> 
>> 61:             if (TRACE_RESOLVE) {
>> 62:                 System.out.println(traceLF + (resolvedMember != null ? " (success)" : " (fail)"));
>> 63:             }
> 
> I suggest not to change the existing code.  Instead, have `CDS::traceLambdaFormInvoker`
> to take individual parameters `Class<?> holder, String name, String shortenSignature`
> (rather than the formatted string).   Something like:
> 
>     if (CDS.isDumpLoadedClassList()) {
>           CDS.traceLambdaFormInvoker(holder, name, shortenSignature(basicTypeSignature(type));
>     }
> 
> This also gives flexibility to CDS to decide on what format to write to the class list (like this case, you drop the
> text "success/fail")
> In addition, the conditional check on `CDS.isDumpLoadedClassList()` is hard to relate to why CDS traces these events.
> I see Ioi's comment on this method name too.   I agree with Ioi that `isDumpingClassList` makes more sense.

Done

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

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



More information about the build-dev mailing list