RFR: 8307468: CDS Lambda Proxy classes are regenerated in dynamic dump

[Calvin Cheung]

On Fri, 1 Sep 2023 19:10:16 GMT, Ashutosh Mehra <duke at openjdk.org> wrote:

>> Please review this changeset for fixing the following issues:
>> 
>> 1. for lambda proxy classes already stored in the static archive, during CDS dynamic dump, those classes should not be regenerated and stored in the dynamic archive; (see changes in jvm.cpp)
>> 2. handle multiple lambda proxy classes with the same invoke dynamic info stored in both static and dynamic archives. (see changes in systemDictionaryShared.cpp)
>> 
>> Also added a test.
>> 
>> Passed tiers 1 - 4 testing.
>
> src/hotspot/share/prims/jvm.cpp line 3632:
> 
>> 3630:   InstanceKlass* shared_lambda = nullptr;
>> 3631:   if (caller_ik->is_shared()) {
>> 3632:     shared_lambda = SystemDictionaryShared::get_shared_lambda_proxy_class(caller_ik, interface_method_name, factory_type,
> 
> I think instead of doing a search here, if we first search in `InnerClassLambdaMetafactory.spinInnerClass`, we can avoid generating the class when we are dumping the dynamic archive and the lambda proxy class is already present in the static archive.
> I think the change proposed by @iklam [here](https://bugs.openjdk.org/browse/JDK-8307468?focusedCommentId=14579121&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14579121) is on the same line.

I've made the changes in `InnerClassLambdaMetafactory.java` and reverted the changes in `jvm.cpp`. I also removed the following check in `LambdaProxyClassArchive.java`:

if (CDS.isDumpingArchive())
            throw new IllegalStateException("cannot load class from CDS archive at dump time");

Will do more testing before posting an update.

> test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/UsedAllArchivedLambdas.java line 69:
> 
>> 67:             "-cp", appJar, mainClass, "run")
>> 68:             .assertNormalExit(output -> {
>> 69:                 output.shouldContain("Used all dynamic archived lambda proxy classes for: UsedAllArchivedLambdasApp run()Ljava/lang/Runnable;")
> 
> Probably worth checking the output for "Used all static archived lambda proxy classes for" as well?

The "Used all static archived lambda proxy classes for" log output isn't applicable to the `UsedAllArchivedLambdas.java` test because the lambda proxy classes are all in the dynamic archive. It could be applicable to the new test. I'll check.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15524#discussion_r1313644762
PR Review Comment: https://git.openjdk.org/jdk/pull/15524#discussion_r1313645465