RFR: 6983726: Reimplement MethodHandleProxies.asInterfaceInstance [v24]
Chen Liang
liach at openjdk.org
Tue Jul 11 18:34:33 UTC 2023
On Tue, 11 Jul 2023 17:36:33 GMT, Mandy Chung <mchung at openjdk.org> wrote:
>> Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Fix the lazy test, thanks Jorn Vernee!
>
> test/jdk/java/lang/invoke/MethodHandleProxies/ProxiesImplementationTest.java line 198:
>
>> 196: System.gc();
>> 197: var c2 = asInterfaceInstance(ifaceClass, mh);
>> 198: assertTrue(cl.refersTo(c2.getClass()), "MHP should reuse implementation class when available");
>
> We can simplify the test a little bit. Just to check if the class of `c1` and `c2` is the same first.
>
> Suggestion:
>
> var c2 = asInterfaceInstance(ifaceClass, mh);
> assertTrue(c1.getClass() == c2.getClass(), "MHP should reuse implementation class when available");
I will keep the gc, which ensures a scenario like that incorrectly weakref'd Lookup doesn't happpen again.
> test/jdk/java/lang/invoke/MethodHandleProxies/ProxiesInterfaceTest.java line 143:
>
>> 141: }
>> 142:
>> 143: //<editor-fold desc="Infrastructure">
>
> this comment can be deleted.
It was a leftover from when I tried to organize the tests with IDE editor folds. No longer necessary indeed.
> test/micro/org/openjdk/bench/java/lang/invoke/MethodHandleProxiesAsIFInstanceCall.java line 49:
>
>> 47: /**
>> 48: * Benchmark evaluates the call performance of MethodHandleProxies.asInterfaceInstance
>> 49: * return value, compared to
>
> incomplete comment?
On a side note, these benchmarks were added for the class-per-MH implementation; I probably need to reevaluate if these 2 benchmarks are needed, or if the original `MethodHandleProxiesAsIFInstance.java` suffices.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1260124340
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1260116789
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1260116076
More information about the core-libs-dev
mailing list