[11] RFR: 8198418: Invoke LambdaMetafactory::metafactory exactly from the BootstrapMethodInvoker

[Claes Redestad]

You also pointed out that if the params or return types doesn't match, 
we'd get a CCE sooner or later, making the return and argument checks 
superfluous. This all simplifies into this, then:

http://cr.openjdk.java.net/~redestad/8198418/jdk.02/

Thanks!

/Claes


On 2018-02-20 13:20, Vladimir Ivanov wrote:
> No need in MT.equals. Pointer comparison should work as well: 
> MethodType instances are interned and all exact type checks on 
> MethodHandles are implemented using == on their MTs.
>
> Best regards,
> Vladimir Ivanov
>
> On 2/20/18 3:07 PM, Claes Redestad wrote:
>> Hi Rémi,
>>
>> sure, MethodType.equals will do a fast == check, but then checks the 
>> param types. It sure looks cleaner, though:
>>
>> http://cr.openjdk.java.net/~redestad/8198418/jdk.01/
>>
>> Thanks!
>>
>> /Claes
>>
>>
>> On 2018-02-20 12:38, Remi Forax wrote:
>>> Hi Claes,
>>> instead of checking each parameter of the bsmType(), why not 
>>> allocating the corresponding MethodType and storing it in a static 
>>> final, so you can check if the MethodType are equals using equals 
>>> (as far as i remember MethodType.equals is a == in the OpenJDK 
>>> implementation).
>>>
>>> in term of name why not isLambdaMetafactoryIndyBootstrapMethod and 
>>> isLambdaMetafactoryCondyBoostrapMethod instead of 
>>> isLambdaMetafactoryCallSite and isLambdaMetafactoryFunction ?
>>>
>>> and you can remove <T> in the signature of 
>>> isLambdaMetafactoryCallSite() and replace Class<T> by Class<?>.
>>>
>>> cheers,
>>> Rémi
>>>
>>> ----- Mail original -----
>>>> De: "Claes Redestad" <claes.redestad at oracle.com>
>>>> À: "core-libs-dev" <core-libs-dev at openjdk.java.net>
>>>> Envoyé: Mardi 20 Février 2018 11:51:15
>>>> Objet: [11] RFR: 8198418: Invoke LambdaMetafactory::metafactory 
>>>> exactly from the BootstrapMethodInvoker
>>>> Hi,
>>>>
>>>> a small regression to lambda bootstrapping came in with the recent
>>>> condy merge, and it took me a while to figure out why.
>>>>
>>>> Before condy, the first three parameters of calls from the BSM invoker
>>>> to the six parameter LambdaMetafactory::metafactory were statically
>>>> known, so only the fourth through sixth param were dynamically bound
>>>> to enforce runtime type checks (MH.invoke -> MH.checkGenericInvoker
>>>> -> MH.asType(MT) -> MHI.makePairwiseConvertByEditor -> generates a
>>>> slew of filterArguments, rebinds, casting MHs etc).
>>>>
>>>> With condy, the third parameter is now an Object (in reality either a
>>>> Class or a MethodType), thus not statically known. This means the
>>>> MethodType sent to checkGenericInvoker will have to add a cast for
>>>> this param too, thus in makePairwiseConvertByEditor we see an
>>>> additional rebind, some additional time spent spinning classes etc.
>>>> Effectively increasing the cost of first lambda initialization by a 
>>>> small
>>>> amount (a couple of ms).
>>>>
>>>> Here came the realization that much of the static overhead of the
>>>> lambda bootstrapping could be avoided altogether since we can
>>>> determine and cast arguments statically for the special-but-common
>>>> case of LambdaMetafactory::metafactory. By using exact type
>>>> information, and even bootstrapMethod.invokeExact, no dynamic
>>>> runtime checking is needed, so the time spent in
>>>> makePairwiseConvertByEditor is avoided entirely.
>>>>
>>>> This might be a hack, but a hack that removes a large chunk of the
>>>> code executed (~75% less bytecode) for the initial lambda bootstrap.
>>>> Startup tests exercising lambdas show a 10-15ms improvement - the
>>>> static overhead of using lambdas is now just a few milliseconds in 
>>>> total.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~redestad/8198418/jdk.00/
>>>> RFE: https://bugs.openjdk.java.net/browse/JDK-8198418
>>>>
>>>> The patch includes a test for an experimental new metafactory method
>>>> that exists only in the amber condy-folding branch. I can easily 
>>>> break it
>>>> out and push that directly to amber once this patch syncs up there, 
>>>> but
>>>> have tested that keeping it in here does no harm.
>>>>
>>>> Thanks!
>>>>
>>>> /Claes
>>