On Sun, 18 Oct 2020 15:09:31 GMT, Peter Levart <plev...@openjdk.org> wrote:
>> Hi Peter, >> >> This seems an attracting idea to keep the possibility of using lambdas. >> However, the full privileged lookup of the >> proxy class will be leaked to `InvocationHandlerWithLookup` implementation >> which imposes security concerns. This >> full-privileged lookup has access to other proxy classes in that module >> including private members that we don't want to >> hand out to user code. Other than this security concern, we will also need >> to consider the inconsistency having >> `java.lang.reflect` API to reference `Lookup`. > > Hi Mandy, > > You're right. I haven't thought of what one can do with a Lookup for one > proxy class when other proxy classes are > co-located in the same module and/or package. So instead of a Lookup, we > could use some other more targeted > "capability". Here's alternative API no.2 that uses special SuperInvoker > callback interface passed as argument to > invocation handler: https://github.com/mlchung/jdk/pull/4 > Each proxy class has its own SuperInvoker instance, assigned to static final > field of generated proxy class like Method > objects, which is passed to invocation handler in the same way as the > selected Method object. Invocation handler can > use this SuperInvoker instance to invoke the super default method on the > proxy. SuperInvoker can verify that the > passed-in proxy is of the correct class very quickly. Also the cache of > transformed method handles per Method is > embedded in the SuperInvoker itself instead of using ClassValue to hold it, > so overhead for super invocations is > further reduced: Mandy's original: 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 > > > Peter's performance patch: > > 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 > > > Static method moved to InvocationHandler + added access check > > Benchmark Mode Cnt Score Error Units > ProxyBench.implClass avgt 5 3.740 ± 0.004 ns/op > ProxyBench.implProxy avgt 5 34.226 ± 0.125 ns/op > ProxyBench.ppImplClass avgt 5 3.780 ± 0.004 ns/op > ProxyBench.ppImplProxy avgt 5 147.318 ± 1.422 ns/op > > > Alternative API #1 with access check in newProxyInstance and Lookup parameter > > Benchmark Mode Cnt Score Error Units > ProxyBench.implClass avgt 5 3.782 ± 0.013 ns/op > ProxyBench.implProxy avgt 5 32.493 ± 0.192 ns/op > ProxyBench.ppImplClass avgt 5 3.749 ± 0.002 ns/op > ProxyBench.ppImplProxy avgt 5 30.565 ± 0.190 ns/op > > > Alternative API #2 with SuperInvoker parameter > > Benchmark Mode Cnt Score Error Units > ProxyBench.implClass avgt 5 3.777 ± 0.003 ns/op > ProxyBench.implProxy avgt 5 20.282 ± 0.585 ns/op > ProxyBench.ppImplClass avgt 5 3.752 ± 0.002 ns/op > ProxyBench.ppImplProxy avgt 5 19.790 ± 0.335 ns/op > > So what do you think about this one? Hi Peter, thanks for coming with these alternatives. The need for new `InvocationHandler2` and `InvocationHandler2.SuperInvoker` APIs and the complexity of `plevart:proxy-default-method-alt-api2` look unpleasing in order to keep the invocation of default methods in lambdas. I prefer the `DelegatingInvocationHandler` abstract class approach without caller-sensitive method. ------------- PR: https://git.openjdk.java.net/jdk/pull/313