Separated from bug 1314055. currently await is creating functions for onFulfilled/onRejected each time, but Promise implementation supports default functions, and we could integrate await handling there.
draft patch is ready, I'm about to land this at the same time as bug 1314055.
Created attachment 8808005 [details] [diff] [review] Do not allocate onFulfilled/onRejected function for await. Added PROMISE_HANDLER_AWAIT_FULFILLED and PROMISE_HANDLER_AWAIT_REJECTED special handler, like PROMISE_HANDLER_IDENTITY. when handler is PROMISE_HANDLER_AWAIT_FULFILLED, it calls AsyncFunctionAwaitedFulfilled. when handler is PROMISE_HANDLER_AWAIT_REJECTED, it calls AsyncFunctionAwaitedRejected. Added PromiseSlot_AwaitGenerator slot value that equals to PromiseSlot_RejectFunction. Promise with PROMISE_FLAG_AWAIT flag stores generator value of async function to the slot. the generator value is used by PROMISE_HANDLER_AWAIT_FULFILLED and PROMISE_HANDLER_AWAIT_REJECTED. also, * Separate NewPromiseCapability's optimized path to CreatePromiseObjectWithDefaultResolution to call from AsyncFunctionAwait * Separated PerformPromiseThen's Step 7- to PerformPromiseThenImpl, to call from AsyncFunctionAwait with special handlers * Fixed bug 1314055 part 1's issue that it gets "constructor" property from passed promise
Attachment #8808005 - Flags: review?(till)
Comment on attachment 8808005 [details] [diff] [review] Do not allocate onFulfilled/onRejected function for await. Review of attachment 8808005 [details] [diff] [review]: ----------------------------------------------------------------- While this looks great, I'm a little concerned with the added complexity in the Promise implementation. To some extent that is of course unavoidable - I just wonder if we can factor it out better. One thing that should be possible without too much effort is to directly use EnqueuePromiseReactionJob in AsyncFunctionAwait. Instead of creating the resultPromise in step 7, you directly create a ReactionRecord using NewReactionRecord, with the generatorVal, converted to an object, stored in the ReactionRecordSlot_Promise. Depending on whether the promise created in step 2 is still pending (i.e., if the `value` it was resolved to is a thenable) or not, you use AddPromiseReactionJob or EnqueuePromiseReactionJob. Then in PromiseReactionJob, you can check if the reaction has the (newly-added) REACTION_FLAG_AWAIT set, and treat it accordingly. Maybe even factor that out of PromiseReactionJob into a function such as ExecuteAwaitReaction(cx, reaction, handlerNum, argument). Now none of this is less complicated than what you have now, but I feel it's easier to factor out (no need to split up PerformPromiseThen), and avoids creation of the throwawayPromise instance. I realize that this isn't a trivial refactoring, so I'm perfectly fine with landing this as-is - we can always do further improvements later on. ::: js/src/builtin/Promise.h @@ +30,5 @@ > #define PROMISE_FLAG_HANDLED 0x4 > #define PROMISE_FLAG_REPORTED 0x8 > #define PROMISE_FLAG_DEFAULT_RESOLVE_FUNCTION 0x10 > #define PROMISE_FLAG_DEFAULT_REJECT_FUNCTION 0x20 > +#define PROMISE_FLAG_AWAIT 0x40 nit: right-align 0x40 with 0x8 above
Attachment #8808005 - Flags: review?(till) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f38bd73f5bd95cfd150e9552bd499c220f07816 Bug 1315559 - Do not allocate onFulfilled/onRejected function for await. r=till
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.