ResolvePromiseInternal gets "then" property from the thenable object, and passes the value to job, but in most case the "then" property is Promise.prototype.then. I think, we could skip the following: * get "then" property in some case (if the shape is same every time, see also bug 1410695) * store "then" callable object to slot (this can avoid post barrier) * call on the "then" function with Call+CallArgs

(In reply to Tooru Fujisawa [:arai] from comment #0) > ResolvePromiseInternal gets "then" property from the thenable object, and > passes the value to job, but in most case the "then" property is > Promise.prototype.then. if "then" property exists. (sorry I forgot to think about other cases)

Stopped storing "then" property when it's the original Promise#then from the same compartment, and instead directly call Promise#then implementation for such case. performance comparison on bug 1393712's case: before: 4745 [ms] after: 4545 [ms]

Comment on attachment 8988413 [details] [diff] [review] Do not store the then property into thenable job if it is the original Promise.prototype.then. Review of attachment 8988413 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, looks reasonable! ::: js/src/builtin/Promise.cpp @@ +27,5 @@ > #include "vm/NativeObject-inl.h" > > using namespace js; > > +using mozilla::Maybe; Please also add |#include "mozilla/Maybe.h"|. Hmm wait, the local style in Promise.cpp seems to be using the full qualified name when accessing mozilla::Maybe, so maybe stick with that here too instead of adding a |using| declaration? @@ +40,5 @@ > enum PromiseHandler { > PromiseHandlerIdentity = 0, > PromiseHandlerThrower, > > + // Originall Promise.prototype.then which comes from the same compartment. Typo: Originall -> Original Do we need "same compartment" or "same realm"? (Because of bug 1357862.) @@ +606,5 @@ > if (!IsCallable(thenVal)) > return FulfillMaybeWrappedPromise(cx, promise, resolutionVal); > > + // If the `then` property is the original Promise.prototype.then from the > + // same compartment, we skip storing/calling it. With bug 1357862, this may need to change to check the current realm, but it doesn't seem like :jandem already started tackling the various "if the object is not a wrapper, then it's from the current realm" checks we use all over the place. @@ +1315,5 @@ > > // Step 1. > RootedObject resolveFn(cx); > RootedObject rejectFn(cx); > if (!CreateResolvingFunctions(cx, promise, &resolveFn, &rejectFn)) This makes me wonder if we can avoid creating the resolving functions, when we know |then| is the original `Promise.prototype.then`. But I'm not sure about it, I'd need to dig deeper into the Promise spec again to say for sure... @@ +1333,5 @@ > + RootedValue resolveVal(cx, ObjectValue(*resolveFn)); > + RootedValue rejectVal(cx, ObjectValue(*rejectFn)); > + > + // Same as above, we return immediately on success. > + if (Promise_then_impl(cx, thenable, resolveVal, rejectVal, &rval, true)) Is |rvalUsed = true| necessary here, because the returned value |rval| is never used, is it?

Thank you for reviewing :D (In reply to André Bargull [:anba] from comment #3) > Do we need "same compartment" or "same realm"? (Because of bug 1357862.) Yeah, it needs same realm (same global object). fixed the comment. > @@ +1333,5 @@ > > + RootedValue resolveVal(cx, ObjectValue(*resolveFn)); > > + RootedValue rejectVal(cx, ObjectValue(*rejectFn)); > > + > > + // Same as above, we return immediately on success. > > + if (Promise_then_impl(cx, thenable, resolveVal, rejectVal, &rval, true)) > > Is |rvalUsed = true| necessary here, because the returned value |rval| is > never used, is it? Good catch. changed to false.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9877b8bc201ccdba981383b536bfb017cebb76ff Bug 1412200 - Do not store the then property into thenable job if it is the original Promise.prototype.then. r=anba

Are there any numbers on how faster it is now?

(In reply to Jens Hausdorf from comment #7) > Are there any numbers on how faster it is now? comment #2