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

Reply via email to