RFR: 8159746: (proxy) Support for default methods

[Peter Levart]

On Fri, 16 Oct 2020 18:13:08 GMT, Mandy Chung <mchung at openjdk.org> wrote:

>> Hi Mandy,
>> 
>> I read your concerns and have an alternative API proposition here. Instread of an abstract class with protected final
>> `invokeDefaultMethod` method, we can keep the public static `invokeDefaultMethod` and add a `Lookup` parameter to it.
>> The Lookup instance to be passed to the method should be able to `findSpecial` the default method - it has to be the
>> full privilege lookup with proxy class as lookup class. Such instance is provided in a call to the alternative
>> invocation handler: `InvocationHandlerWithLookup` which is a super-interface of plain `InvocationHandler` that just
>> forwards the invocation with the additional Lookup parameter to the abstract method without parameter (in order to keep
>> backwards-compatibility). The generated proxy class invokes this new method in new interface and passes its own lookup
>> to it. There's also an overloaded Proxy::newProxyInstance method that takes this new InvocationHandlerWithLookup
>> parameter instead of plain InvocationHandler. This allows specifying lambdas for both overloaded methods and the right
>> one is selected according to the number of lambda parameters. invoking the constructor of the proxy class taking plain
>> InvocationHandler is still allowed but only if the handler class does not use the Lookup parameter. The overloaded
>> Proxy.newProxyInstance throws IllegalAccessException in case the passed-in handler uses the provided lookup but some
>> interfaces of the proxy are not accessible.  Performance with this implementation is restored and is not worse when
>> dealing with inaccessible interfaces:  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
>> 
>> 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 with access check in newProxyInstance
>> 
>> 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
>> 
>> The code is in this pull request against your last commit in your branch: https://github.com/mlchung/jdk/pull/2
>
> 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?

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

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