RFR: 8294961: Convert java.base/java.lang.reflect.ProxyGenerator to use the Classfile API to generate proxy classes [v6]
liach
duke at openjdk.org
Sat Mar 11 16:24:35 UTC 2023
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
More information about the core-libs-dev
mailing list