Personally I think it would be cleaner to just explicitly pass the extra argument to the builtin. The the overhead of these few extra bytes are minimal compared to the rest of the await machinery.
On 12 July 2017 at 21:35, 'Jakob Gruber' via v8-dev <[email protected] > wrote: > The (admittedly minor) difference is that in option 1 passes > `is_predicted_as_caught` when calling the TFJ AsyncFunctionAwait builtin > from bytecode. > > I suggested keeping the two separate TFJ builtins > (AsyncFunctionAwaitCaught & AsyncFunctionAwaitUncaught), but moving the > currently inlined implementation (generated by > AsyncBuiltinsAssembler::Await) into a single TFS builtin that is called > from AsyncFunctionAwait*. `is_predicted_as_caught` would be passed in from > each TFJ builtin. This solution should be simple to implement, the bytecode > wouldn't need to change, and code size overhead should be low as the TFJ > builtins only need to fetch arguments and call the TFS builtin. Downsides > are that we'd now have 3 builtins instead of 2 & the additional call > overhead. > > On Wed, Jul 12, 2017 at 6:41 PM, Caitlin Potter <[email protected]> > wrote: > >> Isn't that the same thing as option 1? I'm not sure I understand the >> difference >> >> On Wednesday, 12 July 2017 12:33:34 UTC-4, Jakob Gruber wrote: >>> >>> If it's just about saving code size, the simplest option might be to add >>> another internal TFS builtin that is called by both Uncaught and Caught >>> variants and which additionally takes a `is_predicted_as_caught` parameter. >>> You'd get the roughly 50% size reduction without having to change much >>> outside of src/builtins. >>> >>> That said, option 1 also sounds good to me. >>> >>> On Wed, Jul 12, 2017 at 5:49 PM, 'Ross McIlroy' via v8-dev < >>> [email protected]> wrote: >>> >>>> Hi Caitlin, >>>> >>>> I'd definitively like to avoid the need for these extra copies of the >>>> async functions to specify the catch prediction, thanks for looking into >>>> this. >>>> >>>> Option (1) seems by far the most preferable to me. I think it would be >>>> slightly more than 1 extra byte per await operation (I think you would want >>>> an LdaSmi + Star bytecode combo to load the prediction into a register and >>>> pass that as an argument to the InvokeIntrinsic bytecode that calls the >>>> await builtin, so ~4 bytes). However, even so, the cost is pretty minimal >>>> compared to all the other bytecode that needs to be generated for async >>>> functions. >>>> >>>> While option (2) would only require extra bytes on try/catches rather >>>> than awaits, each try/catch would require more bytecodes than option (1) >>>> (since setting the prediction in the generator object would require a >>>> LdaSmi + Star + InvokeIntrisic to actually set the field in the generator >>>> object, so ~7 bytes). It's not clear that there would actually be savings >>>> in real world code, and even if there were, I don't think the extra >>>> complexity would justify it. >>>> >>>> Option (3) would be really complicated (particularly as the builtin >>>> would need to deal with both interpreted or optimized code calling it), and >>>> I don't think it's worth the complexity. >>>> >>>> Cheers, >>>> Ross >>>> >>>> On 12 July 2017 at 16:19, <[email protected]> wrote: >>>> >>>>> Hi, >>>>> >>>>> Currently there are 4 different builtin functions used for Await >>>>> operations, 2 for Async Functions and 2 for Async Generators. >>>>> The reason there are 2 versions of each is so that a Promise bitfield >>>>> can be conditionally updated to indicate that promise >>>>> rejection is handled by a catch handler in an async function. The >>>>> operation `promise.[[flags]] = promise.[[flags]] & <constant>` ends >>>>> up costing a little less than 4-5kb in snapshot size, which is more >>>>> than it ought to need. >>>>> >>>>> It's not the most critical thing, but it might be a simple way to >>>>> streamline these builtins a bit and gain a small amt of savings for >>>>> mobile devices. I'd appreciate feedback on those ideas before spending >>>>> time trying to do any of it. >>>>> >>>>> I think reducing snapshot size is appreciated, so a few ideas came to >>>>> mind, sorted from least to most "exotic": >>>>> >>>>> 1) pass the current catch prediction to the Await builtin explicitly >>>>> as a parameter to the builtin (increase the bytecode cost of each Await >>>>> operation by ~1byte). >>>>> >>>>> 2) save catch prediction in flags field of JSGeneratorObject on entry >>>>> and exit from a top-level Try block in body of an async function body (in >>>>> BytecodeBenerator::VisitTryStatement), >>>>> and load this flag from the AsyncFunctionAwait and AsyncGeneratorAwait >>>>> builtins (very minor increase in bytecode size on average, a few bytes per >>>>> top-level try-block in async function body). >>>>> >>>>> 3) refactor these builtins so that they have access to the caller's >>>>> PC, and calculate availability of a catch handler from that (my least >>>>> preferred option, unknown cost in terms of bytecode size). >>>>> >>>>> -- >>>>> -- >>>>> v8-dev mailing list >>>>> [email protected] >>>>> http://groups.google.com/group/v8-dev >>>>> --- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "v8-dev" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to [email protected]. >>>>> For more options, visit https://groups.google.com/d/optout. >>>>> >>>> >>>> -- >>>> -- >>>> v8-dev mailing list >>>> [email protected] >>>> http://groups.google.com/group/v8-dev >>>> --- >>>> You received this message because you are subscribed to the Google >>>> Groups "v8-dev" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> >>> -- >> -- >> v8-dev mailing list >> [email protected] >> http://groups.google.com/group/v8-dev >> --- >> You received this message because you are subscribed to the Google Groups >> "v8-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> For more options, visit https://groups.google.com/d/optout. >> > > -- > -- > v8-dev mailing list > [email protected] > http://groups.google.com/group/v8-dev > --- > You received this message because you are subscribed to the Google Groups > "v8-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
