On Wed, 21 Feb 2024 19:10:16 GMT, Calvin Cheung <cche...@openjdk.org> wrote:

> To avoid the CDS dump error message, a fix is during dumping a classlist, 
> check if an invoker can be archived. 
> If not, don't write the invoker info into the classlist, i.e. don't call 
> `logLambdaFormInvoker()`. While generating holder classes (in 
> `generateHolderClasses()`), don't add the `MethodType` to the `invokerTypes` 
> if will fail the check in the `build()` method which would result in a 
> `RuntimeException`.
> 
> Also updated the `MethodHandlesInvokersTest.java` under 
> `appcds/methodHandles` and `appcds/dynamicArchive/methodHandles` to check 
> that the "Failed to generate LambdaForm holder classes" error is not in the 
> output;
> 
> Passed tiers 1 - 3 testing.

src/java.base/share/classes/java/lang/invoke/GenerateJLIClassesHelper.java line 
333:

> 331:                                                     .warning("Invalid 
> LF_RESOLVE " + parts[1] + " " + parts[2] + " " + parts[3]);
> 332:                                         }
> 333:                                     }

The underlying cause of the failure in CDS is 
[JDK-8327499](https://bugs.openjdk.org/browse/JDK-8327499) -- 
MethodHandleStatics.traceLambdaForm() includes methods that cannot be handle by 
GenerateJLIClassesHelper.java.

Therefore, I think the warning should always be printed (e.g., when 
jdk.tools.jlink.internal.plugins.GenerateJLIClassesPlugin is used during the 
JDK build).

Also, maybe add a comment above line 326: 


// Work around JDK-8327499

src/java.base/share/classes/jdk/internal/misc/CDS.java line 127:

> 125:      */
> 126:     public static void traceLambdaFormInvoker(String prefix, String 
> holder, String name, String type, MethodType mt) {
> 127:         if (isDumpingClassList && isInvokerArchivable(mt, holder)) {

Is this check still needed after you added the filtering in 
GenerateJLIClassesHelper.java?

Since the logic here is not 100% the same as the logic in 
GenerateJLIClassesHelper.java, we might be filtering more things than necessary.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17953#discussion_r1515211752
PR Review Comment: https://git.openjdk.org/jdk/pull/17953#discussion_r1515214286

Reply via email to