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

Tue Feb 20 16:46:28 UTC 2018

Thanks! Pushed.

/Claes

On 2018-02-20 17:06, Brian Goetz wrote:
> That’s great.  Anyone maintaining this file should see it.
>
>
>> On Feb 20, 2018, at 7:46 AM, Claes Redestad <claes.redestad at oracle.com> wrote:
>>
>> Would injecting this before the LMF::metafactory do? Or should this be an @implNote?
>>
>> diff -r d8e1eab41853 src/java.base/share/classes/java/lang/invoke/LambdaMetafactory.java
>> --- a/src/java.base/share/classes/java/lang/invoke/LambdaMetafactory.java Tue Feb 20 14:40:53 2018 +0100
>> +++ b/src/java.base/share/classes/java/lang/invoke/LambdaMetafactory.java Tue Feb 20 16:50:27 2018 +0100
>> @@ -242,6 +242,12 @@
>>       private static final Class<?>[] EMPTY_CLASS_ARRAY = new Class<?>[0];
>>       private static final MethodType[] EMPTY_MT_ARRAY = new MethodType[0];
>>
>> +    // LambdaMetafactory bootstrap methods are startup sensitive, and may be
>> +    // special cased in java.lang.invokeBootstrapMethodInvoker to ensure
>> +    // methods are invoked with exact type information to avoid generating
>> +    // code for runtime checks. Take care any changes or additions here are
>> +    // reflected there as appropriate.
>> +
>>       /**
>>        * Facilitates the creation of simple "function objects" that implement one
>>        * or more interfaces by delegation to a provided {@link MethodHandle},
>>
>> /Claes
>>
>> On 2018-02-20 16:05, Brian Goetz wrote:
>>> Add a comment to LMF to remember to update the hack if additional sigs are added.
>>>
>>> Sent from my iPad
>>>
>>>> On Feb 20, 2018, at 4:27 AM, Claes Redestad <claes.redestad at oracle.com> wrote:
>>>>
>>>> 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