1315559 - Do no allocate onFulfilled/onRejected functions in await.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

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.

Comment 1

[Tooru Fujisawa [:arai]]

draft patch is ready, I'm about to land this at the same time as bug 1314055.

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: 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

Comment 3

[Till Schneidereit [: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

Comment 4

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f38bd73f5bd95cfd150e9552bd499c220f07816
Bug 1315559 - Do not allocate onFulfilled/onRejected function for await. r=till

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1f38bd73f5bd