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