I have a stack of fourteen patches to clean-up and optimise Promise built-ins. Some performance numbers after the patches: // Promise.race // 2: 75ms -> 50ms // 4: 120ms -> 70ms // 10: 260ms -> 145ms // // Promise.all // 2: 150ms -> 75ms // 4: 220ms -> 110ms // 10: 415ms -> 220ms function testAllOrRace() { var t = 0; for (var i = 0; i < 40000; ++i) { var a = Array(2).fill(null).map(() => newPromiseCapability()); var xs = a.map(c => c.promise); var u = dateNow() Promise.race(cx); // Promise.all(xs); t += dateNow() - u; a.forEach(c => c.resolve()); drainJobQueue(); } print(t); } // Promise.then // 125ms -> 95ms // // Promise.catch // 140ms -> 95ms function testThenOrCatch() { var t = 0; for (var i = 0; i < 200000; ++i) { var c = newPromiseCapability(); var u = dateNow() c.promise.then(); // c.promise.catch(); t += dateNow() - u; c.resolve(); drainJobQueue(); } print(t); }

Changes the parameter type for async-generator code from JSObject* to PromiseObject* in preparation for the next patch.

We don't need to go through |RejectMaybeWrappedPromise| when the promise object is definitely not a wrapped promise object. I've named the new helper method "RejectPromiseInternal", so it matches the already existing "ResolvePromiseInternal".

We don't need to root already rooted values. Drive-by fix: Add a missing |ReportAccessDenied| call in |PromiseObject::create|.

Changes variables from size_t to uint32_t when used for slots/dense indices, because slot and dense elements methods all expect uint32_t values. Drive-by fix: Correct indentation in PromiseReactionRecord.

Use more efficient functions to initialize dense elements. EnqueuePromiseResolveThenableJob: JSCompartment::wrap(...) can GC, so if we move both wrap() calls before the call to NewDenseFullyAllocatedArray(), we can safely use NativeObject::setDenseInitializedLength() and NativeObject::initDenseElement() to fill the array. GetWaitForAllPromise: We can use NativeObject::ensureDenseInitializedLength() to directly initialize all dense elements in an empty array. This is more efficient than calling NativeObject::ensureDenseElements(). AddPromiseReaction: Similar to EnqueuePromiseResolveThenableJob, we can also use NativeObject::setDenseInitializedLength() and NativeObject::initDenseElement() to fill the first two elements. And change the NativeObject::ensureDenseElements() call to use the "correct" invocation, so we fall into this fast path [1]. [1] https://searchfox.org/mozilla-central/rev/448f792f9652d29daebdad21bf50b03405e40a45/js/src/vm/NativeObject-inl.h#393-404

Remove unnecessary rooting where either no GC can happen or the value is directly stored in another rooted location. Also moves rooted variables outside of loops in Promise.all and Promise.race for efficiency reasons.

Avoid duplicate UncheckedUnwrap() calls and change some JSCompartment::wrap() calls to access the compartment through the context instead of using an object. (The latter change is just for consistency with other calls to wrap() in this file and probably also other parts of the engine.) Also change |JSAutoRealm| to |AutoRealm| in PerformPromiseAll to avoid going through JSAPI for engine internal code. And I've changed PromiseAllResolveElementFunction to use |IsProxy| instead of |IsWrapper| to match other code in this file. (I assume Promise code uses IsProxy instead of IsWrapper in some places for performance reasons only.)

Add a PromiseCapability struct to avoid passing the PromiseCapability tuple ([[Promise]], [[Resolve]], and [[Reject]]) as three separate arguments in multiple functions. This change also allows us to make the code a bit more similar to the spec. For example we can have a AbruptRejectPromise function which accepts a PromiseCapability <https://tc39.github.io/ecma262/#sec-ifabruptrejectpromise>. The "resultCapability" resp. "promiseCapability" variable names were chosen to match the names used in the spec algorithms.

Another small change to improve filling the values array in PerformPromiseAll: - Avoid the slowish JSCompartment::wrap() call when the promise object is not wrapped. - Use NewDenseEmptyArray(cx) instead of NewDenseFullyAllocatedArray(cx, 0), to skip a few extra checks in NewDenseFullyAllocatedArray. - Call NewbornArrayPush instead of DefineDataProperty to initialize the values array with undefined-values. We can use NewbornArrayPush here, because the values array is only exposed to user code when we've finished the iteration. - Remove the no longer necessary magic-value check in NativeObject::initDenseElementWithType() so it matches NativeObject::setDenseElementWithType(). And also avoid calling AddTypePropertyId() when it's not necessary, similar to the optimisation in setDenseElementWithType().

Combine the loops from PerformPromiseAll and PerformPromiseRace into a single helper function CommonPerformPromiseAllRace. This change is in preparation for the next patch, which adds more code into the loop and by having only a single implementation of this loop we can avoid quite a bit of code duplication. And also inline BlockOnPromise() into the new CommonPerformPromiseAllRace(), because (a) BlockOnPromise() is now only called from CommonPerformPromiseAllRace() and (b) this'll also make the next patch easier to integrate. Combining everything into a single function requires quite a bit of rooted values and because we normally try to define rooted variables outside of loop statements (in performance sensitive code), I've tried to reuse rooted variables where possible to improve readability and for a tiny bit of additional performance. When a rooted variable is reused, the variable is accordingly named to reflect all use sites, e.g. |RootedObject thenSpeciesOrBlockedPromise| for a rooted object which is first used for |Rooted thenSpecies| and then for |RootedObject blockedPromise|.

Add a property lookup cache similar to the existing lookup cache for Array[@@species]. Because Promise operations are not as performance sensitive as Array[@@species] code, the lookup doesn't store pointers to the |Promise| and |Promise.prototype|. This also reduces the sizeof(PromiseLookup), so it should still be acceptable to directly store PromiseLookup as a member of js::Realm instead of malloc'ing it dynamically. Apart from that the cache matches more or less ArraySpeciesLookup. The new PromiseLookup class only supports Promise object without own properties and whose prototype is |Promise.prototype|. I hope this covers the majority of current use cases for Promise objects, but if we ever want to optimise Promise sub-classes, we'll need to revisit this decision. ResolvePromiseInternal - It looks like |promise| and |resolutionVal| are always from the same compartment, which enables us to remove some of the compartment checks I've recently added for the EnqueuePromiseResolveThenableBuiltinJob fast path. (But also asserts this to be sure that this is really the case.) - Use the new promise lookup cache to try to avoid the GetProperty() call. CommonPerformPromiseAllRace - Uses the lookup cache to skip up to five GetProperty() calls per each iteration ([[Get]] for "resolve", in |Promise.resolve| the "constructor" lookup, then getting the "then" property, in |Promise.prototype.then| another lookup for "constructor" and also for @@species). - Additionally inspect the JS::ForOfIterator (through the new PromiseForOfIterator subclass) to detect the case when the iteration itself doesn't produce any side-effects. This allows us to skip revalidating the promise lookup cache for each iteration. Promise_catch_impl and Promise_then_impl: - Use the lookup cache to directly call into PerformPromiseThen.

We don't need to keep both handlers alive in PromiseReactionRecord when we've determined the target state. Instead we can reuse the slot of the no longer used handler to store the handler-argument. This change has two benefits: 1. It makes the no longer used handler object eligible for GC. 2. It reduces the slot size of PromiseReactionRecord by one, which means PromiseReactionRecord now has "only" eights slots, so it fits into the OBJECT8 allocation kind.

Another clean-up patch to use the Call() functions from Interpreter.h, so we don't need to create FixedInvokeArgs objects ourselves.

Clear the "IncumbentGlobalObject" slot when it's no longer needed to avoid an unnecessary GC thing reference in PromiseReactionRecord objects. (I don't know if this change makes a real difference for the GC, but at least theoretically it should be better to remove unnecessary references to GC things.)

Comment on attachment 8992071 [details] [diff] [review] bug1475678-part6-unnecessary-rooting.patch Review of attachment 8992071 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/builtin/Promise.cpp @@ +628,5 @@ > args.rval().setUndefined(); > return true; > } > > + RootedObject promise(cx, &promiseVal.toObject()); this needs some comment about the ordering. iiuc, this is done before ClearResolutionFunctionSlots in order to keep the following reference graph, right? args.callee() -> reject -> promiseVal -> promise

Comment on attachment 8992074 [details] [diff] [review] bug1475678-part8-promise-capability-struct.patch Review of attachment 8992074 [details] [diff] [review]: ----------------------------------------------------------------- Nice cleanup :D

Comment on attachment 8992077 [details] [diff] [review] bug1475678-part9-promise-all-elem-array-init.patch Review of attachment 8992077 [details] [diff] [review]: ----------------------------------------------------------------- I'll review remaining patches this afternoon.

Comment on attachment 8992081 [details] [diff] [review] bug1475678-part10-combine-race-all-loop.patch Review of attachment 8992081 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/builtin/Promise.cpp @@ +2283,5 @@ > +IsPromiseSpecies(JSContext* cx, JSFunction* species); > + > +template <typename T> > +static MOZ_MUST_USE bool > +CommonPerformPromiseAllRace(JSContext *cx, JS::ForOfIterator& iterator, HandleObject C, can you put the following comment above this definition? // ES2018 PerformPromiseAll 25.6.4.1.1 step 6. // ES2018 PerformPromiseRace 25.6.4.3.1 step 3. so that "step a" etc make sense. (or ES2016 25.4.4.x for each) @@ +2296,5 @@ > + HandleObject resultPromise = resultCapability.promise(); > + RootedValue resolveFunVal(cx); > + RootedValue rejectFunVal(cx, ObjectValue(*resultCapability.reject())); > + > + RootedValue nextValue(cx); nextValueOrNextPromise ? @@ +2300,5 @@ > + RootedValue nextValue(cx); > + RootedObject nextPromiseObj(cx); > + > + RootedValue resolveOrThen(cx); > + RootedObject thenSpeciesOrBlockedPromise(cx); they need a comment about reuse/optimization. @@ +2317,5 @@ > + // Step d. > + if (*done) > + return true; > + > + // Step i. it's different step for each function. // 25.6.4.1.1 step 6.i. // 25.6.4.3.1 step 3.h. @@ +2330,5 @@ > + if (!Call(cx, staticResolve, CVal, nextValue, &nextPromise)) > + return false; > + > + // Get the resolve function for this iteration. > + // Steps j-q. this step exists only in PerformPromiseAll. // 25.6.4.1.1 steps 6.j-q. @@ +2345,5 @@ > + // |Promise| constructor to create the resulting promise, we skip the > + // call to |nextPromise.then| and thus creating a new promise that > + // would not be observable by content. > + > + // Step r. // 25.6.4.1.1 step 6.r. // 25.6.4.3.1 step 3.i. @@ +2362,5 @@ > + if (nextPromiseObj->is<PromiseObject>() && IsNativeFunction(thenVal, Promise_then)) { > + // |nextPromise| is an unwrapped Promise, and |then| is the > + // original |Promise.prototype.then|, inline it here. > + > + // 25.4.5.3., step 3. if you use ES 2018 above, "25.6.5.4 step 3". @@ +2535,1 @@ > // Step h. // Step 6.h. same for other steps in this lambda, since isn't inside a block with "// Step 6." comment. @@ +2570,5 @@ > + // Step 6. > + if (!CommonPerformPromiseAllRace(cx, iterator, C, resultCapability, done, true, getResolve)) > + return false; > + > + // Step d.ii. // Step 6.d.ii. same for the following, since this is not inside a block which has "// Step 6."

Comment on attachment 8992090 [details] [diff] [review] bug1475678-part11-property-lookup-cache.patch Review of attachment 8992090 [details] [diff] [review]: ----------------------------------------------------------------- I have some concern about GC related part. :pbone, can you review GC part of this patch? especially weak pointer of shape. ::: js/src/builtin/Promise.cpp @@ +783,5 @@ > + // through the resolving functions. In that case the normal bookkeeping to > + // ensure only pending promises can be resolved doesn't apply and we need > + // to manually check for already settled promises. The exception is simply > + // dropped when this case happens. > + if (IsSettledMaybeWrappedPromise(promise)) shouldn't we keep this for built-in "then"? @@ +2316,5 @@ > + // during the iteration. > + bool iterationMayHaveSideEffects = !iterator.isOptimizedDenseArrayIteration(); > + > + // Only try to optimize when C is the builtin Promise constructor. > + bool isDefaultPromiseState = C == promiseCtor; can you add comment that explains this is updated to the correct value inside the loop? because, "C == promiseCtor" itself doesn't mean "default promise state", but it's just a requirement. @@ +2366,5 @@ > + > + if (nextValuePromise && > + promiseLookup.isDefaultInstanceWhenPromiseStateIsSane(cx, nextValuePromise)) > + { > + // BlockOnPromiseBuiltinThen doesn't produce any side-effects, BlockOnPromiseBuiltinThen doesn't exist I think? (or perhaps it's in other bug's patch I overlooked/forgot ?) @@ +4173,5 @@ > + getter->as<JSFunction>().realm() == cx->realm(); > +} > + > +void > +js::PromiseLookup::initialize(JSContext* cx) can you enumerate the short summary of requirement at the top? like https://searchfox.org/mozilla-central/rev/6f86cc3479f80ace97f62634e2c82a483d1ede40/js/src/vm/RegExpShared.h#304-312 maybe with index, and refer index in initialize and isPromiseStateStillSane methods, so that it's clear we're not missing some requirement, and also when some other requirement is added in the future, we won't miss it. @@ +4257,5 @@ > + > +void > +js::PromiseLookup::reset() > +{ > + JS_POISON(this, 0xBB, sizeof(this), MemCheckKind::MakeUndefined); now 0xBB is used in 2 places, and I think it's better adding it to something like following (please ask review from GC people), and maybe teach it to mrgiggles :) https://searchfox.org/mozilla-central/rev/6f86cc3479f80ace97f62634e2c82a483d1ede40/js/src/jsutil.h#254-269 https://searchfox.org/mozilla-central/rev/6f86cc3479f80ace97f62634e2c82a483d1ede40/js/src/gc/Marking.cpp#131-143 @@ +4371,5 @@ > + // Ensure |promise| doesn't define any own properties. This serves as a > + // quick check to make sure |promise| doesn't define an own "constructor" > + // or "then" property which may shadow Promise.prototype.constructor or > + // Promise.prototype.then. > + return promise->lastProperty()->isEmptyShape(); it's lucky that Promise object doesn't have property by default :) (otherwise we should cache its shape, like RegExp) ::: js/src/builtin/Promise.h @@ +209,5 @@ > + > + // Shape of matching Promise, slot containing the @@species property. > + MOZ_INIT_OUTSIDE_CTOR Shape* promiseConstructorShape_; > +#ifdef DEBUG > + MOZ_INIT_OUTSIDE_CTOR Shape* promiseSpeciesShape_; it would be better pointing the comment in isPromiseStateStillSane to clarify why this is debug-only, which a bit contradicts with the explanation above. @@ +213,5 @@ > + MOZ_INIT_OUTSIDE_CTOR Shape* promiseSpeciesShape_; > +#endif > + > + // Shape of matching Promise.prototype object. > + MOZ_INIT_OUTSIDE_CTOR Shape* promiseProtoShape_; given it's weak pointer, don't we have to use ReadBarriered<Shape*> instead ? RegExp's cache uses it https://searchfox.org/mozilla-central/rev/6f86cc3479f80ace97f62634e2c82a483d1ede40/js/src/vm/RegExpShared.h#314 I'd forward the review. @@ +250,5 @@ > + using CheckStateFn = decltype(&PromiseLookup::isPromiseStateStillSane); > + > + enum class Reinitialize : bool { > + Allowed, > + Disallowed can you add some comment what they mean, especially, reinitialization of what? iiuc, this allows/disallows reinitializing the lookup cache when failure, right? @@ +267,5 @@ > + bool ensureThenInitialized(JSContext* cx) { > + return ensureInitialized<&PromiseLookup::isPromiseThenStateStillSane>(cx, Reinitialize::Allowed); > + } > + > + // Return true if the prototype is Promise.prototype and the object if the prototype *of the given Promise object* is

Comment on attachment 8992092 [details] [diff] [review] bug1475678-part12-reaction-record-alloc-size.patch Review of attachment 8992092 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/builtin/Promise.cpp @@ +476,1 @@ > ReactionRecordSlot_OnFulfilled, can we rename this to OnFulfilledHandlerOrOnRejectedArg or define one more enum entry like this? ReactionRecordSlot_OnRejectedArg = ReactionRecordSlot_OnFulfilled, same for OnRejected. or add explicit comment in handlerArgSlot method to clarify the name mismatch is intentional.

Comment on attachment 8992090 [details] [diff] [review] bug1475678-part11-property-lookup-cache.patch Review of attachment 8992090 [details] [diff] [review]: ----------------------------------------------------------------- r+ pending fixes. ::: js/src/builtin/Promise.cpp @@ +4257,5 @@ > + > +void > +js::PromiseLookup::reset() > +{ > + JS_POISON(this, 0xBB, sizeof(this), MemCheckKind::MakeUndefined); Arai is right, this should be a new arbitrary bit pattern and added to the lists on those two places. ::: js/src/builtin/Promise.h @@ +213,5 @@ > + MOZ_INIT_OUTSIDE_CTOR Shape* promiseSpeciesShape_; > +#endif > + > + // Shape of matching Promise.prototype object. > + MOZ_INIT_OUTSIDE_CTOR Shape* promiseProtoShape_; Yes. but I don't see where it says it's a weak ref. If it is perhaps use WeakRef<Shape*> instead?

(In reply to Tooru Fujisawa [:arai] from comment #19) > ::: js/src/builtin/Promise.cpp > @@ +783,5 @@ > > + // through the resolving functions. In that case the normal bookkeeping to > > + // ensure only pending promises can be resolved doesn't apply and we need > > + // to manually check for already settled promises. The exception is simply > > + // dropped when this case happens. > > + if (IsSettledMaybeWrappedPromise(promise)) > > shouldn't we keep this for built-in "then"? I'll just remove the change for this function for now, because all it saves is a single property lookup, which currently may improve performance in µ-benchmarks by 1%, if at all. > @@ +2366,5 @@ > > + > > + if (nextValuePromise && > > + promiseLookup.isDefaultInstanceWhenPromiseStateIsSane(cx, nextValuePromise)) > > + { > > + // BlockOnPromiseBuiltinThen doesn't produce any side-effects, > > BlockOnPromiseBuiltinThen doesn't exist I think? > (or perhaps it's in other bug's patch I overlooked/forgot ?) Oh, that's just a leftover from a previous version of these patches. > @@ +4257,5 @@ > > + > > +void > > +js::PromiseLookup::reset() > > +{ > > + JS_POISON(this, 0xBB, sizeof(this), MemCheckKind::MakeUndefined); > > now 0xBB is used in 2 places, and I think it's better adding it to something > like following (please ask review from GC people), and maybe teach it to > mrgiggles :) > I don't really know the rules when to add patterns to the list in jsutil.h. jsutil.h refers to IsThingPoisoned, which makes it sound like only things which are GC-things should be in jsutil.h? Furthermore there are other callers to JS_POISON which directly pass a hex-value [1], including a reuse for 0xCC (NativeIterator::NativeIterator and TrailingNamesArray). [1] https://searchfox.org/mozilla-central/search?q=JS_POISON%5C(%5B%5E%2C%5D%2B%2C%5Cs%2B0&case=true&regexp=true&path= > @@ +213,5 @@ > > + MOZ_INIT_OUTSIDE_CTOR Shape* promiseSpeciesShape_; > > +#endif > > + > > + // Shape of matching Promise.prototype object. > > + MOZ_INIT_OUTSIDE_CTOR Shape* promiseProtoShape_; > > given it's weak pointer, don't we have to use ReadBarriered<Shape*> instead ? > RegExp's cache uses it > https://searchfox.org/mozilla-central/rev/ > 6f86cc3479f80ace97f62634e2c82a483d1ede40/js/src/vm/RegExpShared.h#314 > > I'd forward the review. > The complete cache is purged on GC (see bug 1376572, comment #3 and bug 1376572, comment #7 for the design), so unless things have changed, ReadBarriered shouldn't be necessary here.

(In reply to André Bargull [:anba] from comment #22) > > @@ +4257,5 @@ > > > + > > > +void > > > +js::PromiseLookup::reset() > > > +{ > > > + JS_POISON(this, 0xBB, sizeof(this), MemCheckKind::MakeUndefined); > > > > now 0xBB is used in 2 places, and I think it's better adding it to something > > like following (please ask review from GC people), and maybe teach it to > > mrgiggles :) > > > > I don't really know the rules when to add patterns to the list in jsutil.h. > jsutil.h refers to IsThingPoisoned, which makes it sound like only things > which are GC-things should be in jsutil.h? Furthermore there are other > callers to JS_POISON which directly pass a hex-value [1], including a reuse > for 0xCC (NativeIterator::NativeIterator and TrailingNamesArray). > > [1] > https://searchfox.org/mozilla-central/ > search?q=JS_POISON%5C(%5B%5E%2C%5D%2B%2C%5Cs%2B0&case=true&regexp=true&path= IMO everything should be listed in single place there, in order to give name and unique value :) (so, we should fix those callsites as well) > The complete cache is purged on GC (see bug 1376572, comment #3 and bug > 1376572, comment #7 for the design), so unless things have changed, > ReadBarriered shouldn't be necessary here. oh, indeed. I overlooked the difference of the requirements between RegExp's cache and this cache. https://searchfox.org/mozilla-central/rev/88199de427d3c5762b7f3c2a4860c10734abd867/js/src/vm/Realm.cpp#433-437

> > @@ +4257,5 @@ > > > + > > > +void > > > +js::PromiseLookup::reset() > > > +{ > > > + JS_POISON(this, 0xBB, sizeof(this), MemCheckKind::MakeUndefined); > > > > now 0xBB is used in 2 places, and I think it's better adding it to something > > like following (please ask review from GC people), and maybe teach it to > > mrgiggles :) > > > > I don't really know the rules when to add patterns to the list in jsutil.h. > jsutil.h refers to IsThingPoisoned, which makes it sound like only things > which are GC-things should be in jsutil.h? Furthermore there are other > callers to JS_POISON which directly pass a hex-value [1], including a reuse > for 0xCC (NativeIterator::NativeIterator and TrailingNamesArray). > > [1] > https://searchfox.org/mozilla-central/ > search?q=JS_POISON%5C(%5B%5E%2C%5D%2B%2C%5Cs%2B0&case=true&regexp=true&path= > I don't know the rules either, but it seems like a good idea. If you're still not sure then leave it ass is and we'll fix it in Bug 1476845.

(In reply to Tooru Fujisawa [:arai] from comment #23) > > The complete cache is purged on GC (see bug 1376572, comment #3 and bug > > 1376572, comment #7 for the design), so unless things have changed, > > ReadBarriered shouldn't be necessary here. > > oh, indeed. > I overlooked the difference of the requirements between RegExp's cache and > this cache. > https://searchfox.org/mozilla-central/rev/ > 88199de427d3c5762b7f3c2a4860c10734abd867/js/src/vm/Realm.cpp#433-437 Hmm, the code comment in |Realm::sweepRegExps()| seems to be out of date since quite some time. Filed bug 1476903. (In reply to Paul Bone [:pbone] from comment #24) > I don't know the rules either, but it seems like a good idea. If you're > still not sure then leave it ass is and we'll fix it in Bug 1476845. Thanks for filing that bug. I think I'll leave the patch in this bug as is, pending the resolution for bug 1476845.

Update part 6 per review comments, carrying r+.

Update part 10 per review comments, carrying r+.

Update part 11 per review comments, carrying r+.

Update part 12 per review comments, carrying r+.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=638d715e1948366cfb4b8ffa2bffa271b881efaf

Tried to land this an got: applying bug1475678-part7-unwrapping-calls.patch patching file js/src/builtin/Promise.cpp Hunk #3 FAILED at 1151 Hunk #4 FAILED at 2247 2 out of 6 hunks FAILED -- saving rejects to file js/src/builtin/Promise.cpp.rej patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and qrefresh bug1475678-part7-unwrapping-calls.patch

The error in comment #31 happens when part 7 is applied after part 5, but part 6 hasn't yet been applied. Thanks, André

Pushed by apavel@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f8d04aa5f853 Part 1: Use PromiseObject instead JSObject as the parameter type for async generators. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/e1ca4196b325 Part 2: Skip RejectMaybeWrappedPromise when the promise is definitely not wrapped. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/ef8a735fbc88 Part 3: Remove additional rooting of already rooted values. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/9999014fea3e Part 4: Change size_t variables to uint32_t when the underlying type is int32/uint32. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/3f16b5db5feb Part 5: Improve dense array initialization for Promise code. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/297e8205c591 Part 6: Remove unnecessary rooting in Promise code. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/3dae42009b52 Part 7: Avoid duplicate UncheckedUnwrap calls and use AutoRealm instead of JSAutoRealm. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/04f122f832e9 Part 8: Add a PromiseCapability struct. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/c7a3253ffecd Part 9: Directly initialize dense elements in PerformPromiseAll using NewbornArrayPush. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/7404656e431f Part 10: Create a shared helper for PerformPromiseAll and PerformPromiseRace. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/9d5851d5a423 Part 11: Add cache for Promise property lookups. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/e064d391ad24 Part 12: Reduce slot size of PromiseReactionRecord to fit into OBJECT8 alloc kind. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/1d65a2e8d382 Part 13: Use the Call() helper from Interpreter.h to reduce code duplication. r=arai

i just used the default order. pushed now. Thanks

(In reply to Andreea Pavel [:apavel] from comment #34) > i just used the default order. pushed now. > > Thanks Cool, thanks! Part 14 seems to be missing, though. :-)

Pushed by apavel@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c1af3f6ba686 Part 14: Clear the incumbent global slot when it's no longer used. r=arai

:anba sorry about that.

No worries! And again thanks for taking care of pushing my patches! :-)

https://hg.mozilla.org/mozilla-central/rev/f8d04aa5f853 https://hg.mozilla.org/mozilla-central/rev/e1ca4196b325 https://hg.mozilla.org/mozilla-central/rev/ef8a735fbc88 https://hg.mozilla.org/mozilla-central/rev/9999014fea3e https://hg.mozilla.org/mozilla-central/rev/3f16b5db5feb https://hg.mozilla.org/mozilla-central/rev/297e8205c591 https://hg.mozilla.org/mozilla-central/rev/3dae42009b52 https://hg.mozilla.org/mozilla-central/rev/04f122f832e9 https://hg.mozilla.org/mozilla-central/rev/c7a3253ffecd https://hg.mozilla.org/mozilla-central/rev/7404656e431f https://hg.mozilla.org/mozilla-central/rev/9d5851d5a423 https://hg.mozilla.org/mozilla-central/rev/e064d391ad24 https://hg.mozilla.org/mozilla-central/rev/1d65a2e8d382 https://hg.mozilla.org/mozilla-central/rev/c1af3f6ba686