RFR 8206955 MethodHandleProxies.asInterfaceInstance does not support default methods

[forax at univ-mlv.fr]

----- Mail original -----
> De: "Peter Levart" <peter.levart at gmail.com>
> À: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "Paul Sandoz" <paul.sandoz at oracle.com>, "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Mercredi 11 Juillet 2018 18:32:13
> Objet: Re: RFR 8206955 MethodHandleProxies.asInterfaceInstance does not support default methods

> Sorry Paul for hijacking the thread, just answering to Remi ...
> 
> On 07/11/2018 05:31 PM, Remi Forax wrote:
>> ----- Mail original -----
>>> De: "Peter Levart" <peter.levart at gmail.com>
>>> À: "Paul Sandoz" <paul.sandoz at oracle.com>, "core-libs-dev"
>>> <core-libs-dev at openjdk.java.net>
>>> Envoyé: Mercredi 11 Juillet 2018 17:15:09
>>> Objet: Re: RFR 8206955 MethodHandleProxies.asInterfaceInstance does not support
>>> default methods
>>> Hi Paul,
>>>
>>> The patch looks ok. I hope IMPL_LOOKUP has access to all methods (even
>>> if located in package-private interfaces and/or in concealed packages of
>>> modules)?
>>>
>>> Just a thought... Would it be possible to implement this API in terms of
>>> LambdaMetafactory ?
>>>
>>> Regards, Peter
>> Hi Peter,
>> not with the current LambdaMetaFactory, the LambdaMetaFactory only accept some
>> kind of method handles (constant method calls) not all kind of method handles.
>>
>> That said the current implementation of MethodHandleProxies is very raw and not
>> very efficient, we should use the same idea as the lambda meta factory i.e spin
>> an anonymous class and use the mechanism of constant patching offer by
>> unsafe.defineAnonymousClass to inject the method handle into proxy so it will
>> work with any method handle.
>>
>> For each interface, you should cache the bytecode of the anonymous class you
>> want to load and use defineAnonymousClass with the method handle each time
>> asInterfaceInstance is called.
> 
> If the generated class used invokeExact on the method handle, bytecode
> should be generated specifically for each tuple (interface type, method
> handle type), as the needed conversions of arguments/return values would
> be specific for each distinct combination of the two types.
> 
> ...which would still mean that you would define new anonymous class for
> each method handle instance, just the bytecodes would be generated once
> per (interface type, method handle type) combination.

I was thinking to still collect the arguments in an array as this is done now, to only have one anonymous class by interface type.

> 
> The method handle could then be constant-folded in the generated class,
> but selection of the underlying proxy class would still be governed by
> the proxy instance which would be invoked via the interface method on
> the functional interface. If the proxy instance could not be
> constant-folded (i.e. was not assigned to static final field and used
> from it), the combined invocation performance would still not be the
> same as using invokeExact on the constant method handle, would it?
> 
> So perhaps for this API it is more suitable to:
> 
> - define the specific proxy class once per (interface type, method
> handle type) combination (and cache the class itself, not just bytecode)
> - have that proxy class implement a constructor taking the method handle
> and assign it to a @Stable instance field
> - implement the single interface method as parameter/return value
> conversions around invokeExact on the method handle taken from @Stable
> instance field
> 
> If such proxy instance was constant-folded, so would be the @Stable
> method handle field, right?

If you use an anonymous class, you do not need @Stable because final fields of an anonymous class are trusted by the VM.

The problem with storing the mh in a field is that proxies are rarely used as constant, so the mh will rarely be fully optimized, that why i've proposed to inject the mh as a constant pool constant.

> 
> What do you think of this strategy?

The problem is code sharing vs performance, if you share the proxy class, you end up with a non constant-foldable mh, if you don't share the proxy class, you delay the time to steady state.
Currently the lambda metafactory do not do code sharing and it seems the right choice at least for Java programs, hence my proposal to consider the mh as a constant pool constant.

> 
> Regards, Peter

regards,
Rémi

> 
>>
>> cheers,
>> Rémi
>>
>>>
>>> On 07/11/2018 12:43 AM, Paul Sandoz wrote:
>>>> Hi,
>>>>
>>>> Please review this fix to MethodHandleProxies.asInterfaceInstance to support
>>>> default methods:
>>>>
>>>>     http://cr.openjdk.java.net/~psandoz/jdk/JDK-8206955-mh-func-iface-proxy-default-methods/webrev/
>>>>     <http://cr.openjdk.java.net/~psandoz/jdk/JDK-8206955-mh-func-iface-proxy-default-methods/webrev/>
>>>>
>>>> It probably requires a CSR, which i shall do after this review.
>>>>
>>>> Thanks,
> >>> Paul.