On Fri, 10 Mar 2023 08:48:14 GMT, Adam Sotona <asot...@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
