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.redes...@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.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 >>> >
