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

[Claes Redestad]

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  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  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  in the signature of isLambdaMetafactoryCallSite() and replace 
Class by Class.

cheers,
Rémi

- Mail original -

De: "Claes Redestad" 
À: "core-libs-dev" 
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 

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

[Brian Goetz]

That’s great.  Anyone maintaining this file should see it.


> On Feb 20, 2018, at 7:46 AM, Claes Redestad  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  
>>> 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  in the signature of isLambdaMetafactoryCallSite() 
>> and replace Class by Class.
>> 
>> cheers,
>> Rémi
>> 
>> - Mail original -
>>> De: "Claes Redestad" 
>>> À: "core-libs-dev" 
>>> 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

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

[Claes Redestad]

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  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  in the signature of isLambdaMetafactoryCallSite() and replace 
Class by Class.

cheers,
Rémi

- Mail original -

De: "Claes Redestad" 
À: "core-libs-dev" 
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 

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

[Brian Goetz]

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  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  in the signature of isLambdaMetafactoryCallSite() 
 and replace Class by Class.
 
 cheers,
 Rémi
 
 - Mail original -
> De: "Claes Redestad" 
> À: "core-libs-dev" 
> 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
>>> 
> 

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

[forax]

Looks good !

Rémi

- Mail original -
> De: "Claes Redestad" <claes.redes...@oracle.com>
> À: "Vladimir Ivanov" <vladimir.x.iva...@oracle.com>, "Remi Forax" 
> <fo...@univ-mlv.fr>
> Cc: "core-libs-dev" <core-libs-dev@openjdk.java.net>
> Envoyé: Mardi 20 Février 2018 13:27:43
> Objet: Re: [11] RFR: 8198418: Invoke LambdaMetafactory::metafactory exactly 
> from the BootstrapMethodInvoker

> 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  in the signature of
>>>> isLambdaMetafactoryCallSite() and replace Class by Class.
>>>>
>>>> cheers,
>>>> Rémi
>>>>
>>>> - Mail original -
>>>>> De: "Claes Redestad" <claes.redes...@oracle.com>
>>>>> À: "core-libs-dev" <core-libs-dev@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

Re: [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  in the signature of 
isLambdaMetafactoryCallSite() and replace Class by Class.


cheers,
Rémi

- Mail original -

De: "Claes Redestad" 
À: "core-libs-dev" 
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

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

[Vladimir Ivanov]

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  in the signature of 
isLambdaMetafactoryCallSite() and replace Class by Class.


cheers,
Rémi

- Mail original -

De: "Claes Redestad" 
À: "core-libs-dev" 
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

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

[Claes Redestad]

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  in the signature of isLambdaMetafactoryCallSite() and replace 
Class by Class.

cheers,
Rémi

- Mail original -

De: "Claes Redestad" 
À: "core-libs-dev" 
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

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

[Remi Forax]

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  in the signature of isLambdaMetafactoryCallSite() and 
replace Class by Class.

cheers,
Rémi

- Mail original -
> De: "Claes Redestad" 
> À: "core-libs-dev" 
> 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