RFR: 8294961: Convert java.base/java.lang.reflect.ProxyGenerator to use the Classfile API to generate proxy classes [v6]

[liach]

On Fri, 10 Mar 2023 08:48:14 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> java.base java.lang.reflect.ProxyGenerator uses ASM to generate proxy classes and this patch converts it to use Classfile API.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 196 commits:
> 
>  - Merge branch 'master' into JDK-8294961-proxy
>  - Merge branch 'master' into JDK-8294961-proxy
>  - Merge branch 'JDK-8294982' into JDK-8294961
>  - removed obsolete javadoc from implementation classes
>  - minor fix in CodeBuilder and added test cases to LDCTest
>  - EntryMap::nextPowerOfTwo delegates to Long:numberOfLeadingZeros
>  - fixed CodeBuilder:constantInstruction for -0.0d and -0.0f values and added test
>  - Merge branch 'master' into JDK-8294982
>  - fixed new lines at end of file
>  - package jdk.internal.classfile.jdktypes moved to jdk.internal.classfile.java.lang.constant
>  - ... and 186 more: https://git.openjdk.org/jdk/compare/e26cc526...951b1bc7

src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 68:

> 66:             CD_InvocationHandler = ClassDesc.ofInternalName("java/lang/reflect/InvocationHandler"),
> 67:             CD_Method = ClassDesc.ofInternalName("java/lang/reflect/Method"),
> 68:             CD_MethodHandles$Lookup = ClassDesc.ofInternalName("java/lang/invoke/MethodHandles$Lookup"),

Can reuse CD_MethodHandles_Lookup from ConstantDescs:
Suggestion:

src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 85:

> 83:             MTD_ClassLoader = MethodTypeDesc.of(CD_ClassLoader),
> 84:             MTD_MethodHandles$Lookup = MethodTypeDesc.of(CD_MethodHandles$Lookup),
> 85:             MTD_MethodHandles$Lookup_MethodHandles$Lookup = MethodTypeDesc.of(CD_MethodHandles$Lookup, CD_MethodHandles$Lookup),

Suggestion:

            MTD_MethodHandles$Lookup = MethodTypeDesc.of(CD_MethodHandles_Lookup),
            MTD_MethodHandles$Lookup_MethodHandles$Lookup = MethodTypeDesc.of(CD_MethodHandles_Lookup, CD_MethodHandles_Lookup),

src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 426:

> 424:                  */
> 425:                 localCache.computeIfAbsent(classDesc, cd -> {
> 426:                     try {

This probably can be a factory method in ClassHierarchyResolver too, by examining the reflection API via a ClassLoader/Lookup. I'm inclined to submit a patch for improvement.

Also, why does ProxyGenerator come with a custom ClassHierarchyResolver while InnerClassLambdaMetafactory in 8294960 #12945 doesn't?

src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 602:

> 600:                             .block(blockBuilder -> blockBuilder
> 601:                                     .aload(cob.parameterSlot(0))
> 602:                                     .invokevirtual(CD_MethodHandles$Lookup, "lookupClass", MTD_Class)

Suggestion:

                                    .invokevirtual(CD_MethodHandles_Lookup, "lookupClass", MTD_Class)

src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 606:

> 604:                                     .if_acmpne(blockBuilder.breakLabel())
> 605:                                     .aload(cob.parameterSlot(0))
> 606:                                     .invokevirtual(CD_MethodHandles$Lookup, "hasFullPrivilegeAccess", MTD_boolean)

Suggestion:

                                    .invokevirtual(CD_MethodHandles_Lookup, "hasFullPrivilegeAccess", MTD_boolean)

src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 613:

> 611:                             .dup()
> 612:                             .aload(cob.parameterSlot(0))
> 613:                             .invokevirtual(CD_MethodHandles$Lookup, "toString", MTD_String)

Suggestion:

                            .invokevirtual(CD_MethodHandles_Lookup, "toString", MTD_String)

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

PR: https://git.openjdk.org/jdk/pull/10991