RFR: 8159746: (proxy) Support for default methods

[Peter Levart]

On Mon, 28 Sep 2020 13:47:04 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Hi Mandy,
>> In general this looks good but one thing. The Proxy::invokeDefaultMethod does a lot of checks (creating derived method
>> handles just to check parameters etc.) for every invocation and that means performance is not great. I created a
>> benchmark to see how it compares with bytecode equivalent of invoking super default method:
>> https://gist.github.com/plevart/eee13c50a91bdb61a79cf18a57a0af13 ... The results are not great: Benchmark
>> Mode  Cnt    Score    Error  Units ProxyBench.implClass  avgt    5    3.709 ±  0.026  ns/op ProxyBench.implProxy
>> avgt    5  926.215 ± 11.835  ns/op ...so I thought why not re-doing that part differently - do as many checks as
>> possible just once and delegate checking of passed-in arguments to method handle chain itself instead of doing that in
>> code with reflection. My attempt is expressed in the following commit on top of your PR:
>> https://github.com/plevart/jdk/commit/9b13be1064ec92ce8a25bda7e75f906e2e0b2978 (I couldn't create a PR against your
>> forked jdk repo - I don't know why but Github doesn't list your fork in the popup to choose from, so I'm just pointing
>> to commit in my fork). With this patch on top, the benchmark results are much better: Benchmark             Mode  Cnt
>> Score   Error  Units ProxyBench.implClass  avgt    5   3.777 ± 0.005  ns/op ProxyBench.implProxy  avgt    5  27.579 ±
>> 0.250  ns/op This patch also changes a little the specification of thrown exception types. Since this is a new method,
>> we could follow what method handles do. The choice among IllegalArgumentException, NullPointerException and
>> ClassCastException seems pretty logical to me that way. In case you don't agree, there is a possibility to do it
>> differently for the price of more complicated method handle chain but not necessarily slower performance. So what do
>> you think? Regards, Peter
>
> This is java.lang.reflect so throwing CCE for the mismatching parameter case would make it consistent with
> Method.invoke and maybe other methods. So the performance work is great and maybe see how far it can be brought by
> means of other MH combinators.

@AlanBateman you meant to say throwing CCE for mismatching parameter case would make it **inconsistent** with
Method.invoke, right? That's easy to change. We just catch CCE (and NPE?) and re-throw them wrapped in IAE. We don't
need to do that via MH combinators. Just directly in code. Performance will remain the same.

-------------

PR: https://git.openjdk.java.net/jdk/pull/313