On Mon, 28 Sep 2020 13:47:04 GMT, Alan Bateman <al...@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
