On Wed, 5 Apr 2023 13:22:30 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> Chen Liang has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 13 additional >> commits since the last revision: >> >> - Use a dumper for interface instances, other cleanup >> - Merge branch 'master' into explore/mhp-iface >> - Merge branch 'master' into explore/mhp-iface >> - Remove unused JavaLangReflectReflectAccess.invokeDefault >> - rethrow error >> - Benchmark compareing to lamda metafactory, various minor updates >> - Cache the spinned class, loading is still slow >> - Wrapper instance type is already available >> - mark records as private >> - Merge branch 'master' into explore/mhp-iface >> - ... and 3 more: https://git.openjdk.org/jdk/compare/118045b8...0eec507d > > src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line > 213: > >> 211: .defineClassAsLookup(true, List.of(mhs)); >> 212: proxy = lookup.findConstructor(lookup.lookupClass(), >> methodType(void.class)) >> 213: .asType(methodType(Object.class)).invokeExact(); > > This can use `invoke` instead of an explicit `asType` and `invokeExact`. It > is more or less the same. > Suggestion: > > proxy = lookup.findConstructor(lookup.lookupClass(), > methodType(void.class)).invoke(); Fyi the original code was an emulation of how LambdaMetafactory initializes no-arg lambda instances. > src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line > 225: > >> 223: private record LocalMethodInfo(MethodTypeDesc desc, List<ClassDesc> >> thrown) {} >> 224: >> 225: private record InterfaceInfo(@Stable MethodType[] types, Lookup >> lookup, @Stable byte[] template) {} > > Use of `@Stable` here is not needed. We don't constant fold through > `InterfaceInfo` instances. > Suggestion: > > private record InterfaceInfo(MethodType[] types, Lookup lookup, byte[] > template) {} Hmm, I thought records are constant-folded. https://github.com/openjdk/jdk/blob/44f33ad1a9617fc23864c9ba5f063b3fc2f1e18c/src/hotspot/share/ci/ciField.cpp#L240 Per https://github.com/openjdk/jdk/pull/9143#discussion_r912239501, array cloning sees 10% less time per operation with the annotation. I will try with and without these annotations and report the creation benchmark results to see if they are worth it. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1158852847 PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1158872512
