That’s great. Anyone maintaining this file should see it.
> On Feb 20, 2018, at 7:46 AM, Claes Redestad <[email protected]> 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 <[email protected]> >>> 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" <[email protected]> >>>>>>> À: "core-libs-dev" <[email protected]> >>>>>>> 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 >
