RFR: 6983726: Reimplement MethodHandleProxies.asInterfaceInstance [v16]
Mandy Chung
mchung at openjdk.org
Fri May 26 19:02:11 UTC 2023
On Sun, 7 May 2023 13:34:54 GMT, Chen Liang <liach at openjdk.org> wrote:
>> As John Rose has pointed out in this issue, the current j.l.r.Proxy based implementation of MethodHandleProxies.asInterface has a few issues:
>> 1. Exposes too much information via Proxy supertype (and WrapperInstance interface)
>> 2. Does not allow future expansion to support SAM[^1] abstract classes
>> 3. Slow (in fact, very slow)
>>
>> This patch addresses all 3 problems:
>> 1. It updates the WrapperInstance methods to take an `Empty` to avoid method clashes
>> 2. This patch obtains already generated classes from a ClassValue by the requested interface type; the ClassValue can later be updated to compute implementation generation for abstract classes as well.
>> 3. This patch's faster than old implementation in general.
>>
>>
>> Benchmark Mode Cnt Score Error Units
>> MethodHandleProxiesAsIFInstance.baselineAllocCompute avgt 15 1.483 ± 0.025 ns/op
>> MethodHandleProxiesAsIFInstance.baselineCompute avgt 15 0.264 ± 0.006 ns/op
>> MethodHandleProxiesAsIFInstance.testCall avgt 15 1.773 ± 0.040 ns/op
>> MethodHandleProxiesAsIFInstance.testCreate avgt 15 16.754 ± 0.411 ns/op
>> MethodHandleProxiesAsIFInstance.testCreateCall avgt 15 19.609 ± 1.598 ns/op
>> MethodHandleProxiesAsIFInstanceCall.callDoable avgt 15 0.424 ± 0.024 ns/op
>> MethodHandleProxiesAsIFInstanceCall.callHandle avgt 15 1.936 ± 0.008 ns/op
>> MethodHandleProxiesAsIFInstanceCall.callInterfaceInstance avgt 15 1.766 ± 0.014 ns/op
>> MethodHandleProxiesAsIFInstanceCall.callLambda avgt 15 0.414 ± 0.005 ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantDoable avgt 15 0.271 ± 0.006 ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantHandle avgt 15 0.263 ± 0.004 ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantInterfaceInstance avgt 15 0.266 ± 0.005 ns/op
>> MethodHandleProxiesAsIFInstanceCall.constantLambda avgt 15 0.262 ± 0.003 ns/op
>> MethodHandleProxiesAsIFInstanceCall.direct avgt 15 0.264 ± 0.005 ns/op
>> MethodHandleProxiesAsIFInstanceCreate.createCallInterfaceInstance avgt 15 18.000 ± 0.181 ns/op
>> MethodHandleProxiesAsIFInstanceCreat...
>
> Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
>
> Remove assertion, no longer true with teleport definition in MHP
src/hotspot/share/prims/jvm.cpp line 1019:
> 1017: }
> 1018: }
> 1019: assert(Reflection::is_same_class_package(lookup_k, ik),
I use the Lookup of the proxy interface to define a hidden class in a dynamic module as the dynamic module has no class yet and it's defined to the class loader of the proxy interface.
We should keep the same package check if the defined class is a normal class or a hidden nestmate class. It only allows the lookup class be in a different package for defining a hidden non-nestmate class. This is only internal capability. `Lookup::defineHiddenClass` API will throw IAE if the given bytes denotes a class in a different package than the lookup class.
+ if ((!is_hidden || is_nestmate) && !Reflection::is_same_class_package(lookup_k, ik)) {
+ // non-hidden class or nestmate class must be in the same package as the Lookup class
+ THROW_MSG_0(vmSymbols::java_lang_IllegalArgumentException(), "Lookup class and defined class are in different packages");
+ }
+
src/java.base/share/classes/java/lang/ClassLoader.java line 685:
> 683: final SecurityManager sm = System.getSecurityManager();
> 684: if (sm != null) {
> 685: if (ReflectUtil.isNonPublicProxyClass(cls) || ReflectUtil.isMethodHandleProxiesClass(cls)) {
Why do you need this? `Proxy` allows non-public interface whereas `MethodHandleProxies` only allows public interface.
src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line 209:
> 207: if (intfc.isHidden())
> 208: throw newIllegalArgumentException("a hidden interface", intfc.getName());
> 209: if (!VM.isModuleSystemInited())
I don't expect this is needed. I assume you are thinking for LMF to use this API?
src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line 245:
> 243: private record LocalMethodInfo(MethodTypeDesc desc, List<ClassDesc> thrown, String fieldName) {}
> 244:
> 245: private record ProxyClassInfo(MethodHandle constructor, Lookup originalLookup) {}
`ProxyClassInfo` for interface `I` will be stored in the class value of `I`. Hence it keeps a strong reference to the generated proxy class for `I` until `I` is unloaded. The hidden class implicitly becomes a strong link to the defining class loader. Need to look into keeping it in a weak reference to allow the hidden class to be unloaded independent of the lifetime of the class loader.
src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line 433:
> 431:
> 432: private static WrapperInstance asWrapperInstance(Object x) {
> 433: if (x instanceof WrapperInstance wrapperInstance)
In the previous version, `WrapperInstance` was not needed. Instead it checks if the class is a MHProxy class. Did you run into any issue that you resurrect `WrapperInstance`? Is it just because of accessing `ensureOriginalLookup`?
test/jdk/java/lang/reflect/Proxy/ProxyForMethodHandle.java line 42:
> 40: * @run testng ProxyForMethodHandle
> 41: */
> 42: public class ProxyForMethodHandle {
This test seems to be useful. Can this be modified and moved to `test/jdk/java/lang/invoke/MethodHandleProxies`?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1207189226
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1207185214
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1207191315
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1207194444
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1207208803
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1207190411
More information about the core-libs-dev
mailing list