Bug 1289318 - Part 7: Store the Promise reactions list in the same slot as the result. r?efaust draft
authorTill Schneidereit <till@tillschneidereit.net>
Sat, 06 Aug 2016 02:39:32 +0200
changeset 400700 4ba2419ce2ab2a609fc7d5f20ce785b44fcf7859
parent 400699 2d88eea27b4b4e6575a5be0e5f53913de55bfa15
child 400701 a7b0f6d3a055a4f4a94171cebb306b9db84e9b2c
push id26245
push userbmo:till@tillschneidereit.net
push dateMon, 15 Aug 2016 14:02:13 +0000
reviewersefaust
bugs1289318
milestone51.0a1
Bug 1289318 - Part 7: Store the Promise reactions list in the same slot as the result. r?efaust A Promise can only have either a list of reactions, if it's pending, or a result/reason, if it's resolved. This gets us down to 3 non-debug-info slots. If we now move the debug info into its own object and only allocate that when the debugger is open, Promise instances only need 4 slots. MozReview-Commit-ID: FuLAwhmFTBe
js/src/builtin/Promise.cpp
js/src/builtin/Promise.h
js/src/builtin/Promise.js
js/src/builtin/SelfHostingDefines.h
js/src/builtin/TestingFunctions.cpp
--- a/js/src/builtin/Promise.cpp
+++ b/js/src/builtin/Promise.cpp
@@ -142,23 +142,22 @@ ResolvePromise(JSContext* cx, Handle<Pro
     MOZ_ASSERT(promise->state() == JS::PromiseState::Pending);
     MOZ_ASSERT(state == JS::PromiseState::Fulfilled || state == JS::PromiseState::Rejected);
 
     // Step 2.
     // We only have one list of reactions for both resolution types. So
     // instead of getting the right list of reactions, we determine the
     // resolution type to retrieve the right information from the
     // reaction records.
-    RootedValue reactionsVal(cx, promise->getFixedSlot(PROMISE_REACTIONS_SLOT));
+    RootedValue reactionsVal(cx, promise->getFixedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT));
 
-    // Step 3.
-    promise->setFixedSlot(PROMISE_RESULT_SLOT, valueOrReason);
-
-    // Steps 4-5.
-    promise->setFixedSlot(PROMISE_REACTIONS_SLOT, UndefinedValue());
+    // Step 3-5.
+    // The same slot is used for the reactions list and the result, so setting
+    // the result also removes the reactions list.
+    promise->setFixedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT, valueOrReason);
 
     // Step 6.
     promise->setFixedSlot(PROMISE_STATE_SLOT, Int32Value(int32_t(state)));
 
     // Also null out the resolve/reject functions so they can be GC'd.
     promise->setFixedSlot(PROMISE_RESOLVE_FUNCTION_SLOT, UndefinedValue());
 
     // Now that everything else is done, do the things the debugger needs.
@@ -544,17 +543,20 @@ PromiseObject::getID()
  * Per spec, we should have separate lists of reaction records for the
  * fulfill and reject cases. As an optimization, we have only one of those,
  * containing the required data for both cases. So we just walk that list
  * and extract the dependent promises from all reaction records.
  */
 bool
 PromiseObject::dependentPromises(JSContext* cx, MutableHandle<GCVector<Value>> values)
 {
-    RootedValue reactionsVal(cx, getReservedSlot(PROMISE_REACTIONS_SLOT));
+    if (state() != JS::PromiseState::Pending)
+        return true;
+
+    RootedValue reactionsVal(cx, getReservedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT));
     if (reactionsVal.isNullOrUndefined())
         return true;
     RootedObject reactions(cx, &reactionsVal.toObject());
 
     AutoIdVector keys(cx);
     if (!GetPropertyKeys(cx, reactions, JSITER_OWNONLY, &keys))
         return false;
 
--- a/js/src/builtin/Promise.h
+++ b/js/src/builtin/Promise.h
@@ -25,21 +25,21 @@ class PromiseObject : public NativeObjec
 
     JS::PromiseState state() {
         int32_t state = getFixedSlot(PROMISE_STATE_SLOT).toInt32();
         MOZ_ASSERT(state >= 0 && state <= int32_t(JS::PromiseState::Rejected));
         return JS::PromiseState(state);
     }
     Value value()  {
         MOZ_ASSERT(state() == JS::PromiseState::Fulfilled);
-        return getFixedSlot(PROMISE_RESULT_SLOT);
+        return getFixedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT);
     }
     Value reason() {
         MOZ_ASSERT(state() == JS::PromiseState::Rejected);
-        return getFixedSlot(PROMISE_RESULT_SLOT);
+        return getFixedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT);
     }
 
     MOZ_MUST_USE bool resolve(JSContext* cx, HandleValue resolutionValue);
     MOZ_MUST_USE bool reject(JSContext* cx, HandleValue rejectionValue);
 
     void onSettled(JSContext* cx);
 
     double allocationTime() { return getFixedSlot(PROMISE_ALLOCATION_TIME_SLOT).toNumber(); }
--- a/js/src/builtin/Promise.js
+++ b/js/src/builtin/Promise.js
@@ -475,29 +475,29 @@ function BlockOnPromise(promise, blocked
  * they're all set to NullFunction.
  *
  * The reason for the target Promise to be passed as the receiver is so the
  * same function can be used for wrapped and unwrapped Promise objects.
  */
 function AddDependentPromise(dependentPromise) {
     assert(IsPromise(this), "AddDependentPromise expects an unwrapped Promise as the receiver");
 
+    if (GetPromiseState(this) !== PROMISE_STATE_PENDING)
+        return;
+
     let reaction = new PromiseReactionRecord(dependentPromise, NullFunction, NullFunction,
                                              NullFunction, NullFunction, null);
 
-    let reactions = UnsafeGetReservedSlot(this, PROMISE_REACTIONS_SLOT);
+    let reactions = UnsafeGetReservedSlot(this, PROMISE_REACTIONS_OR_RESULT_SLOT);
 
-    // The reactions list might not have been allocated yet or been reset
-    // because the Promise was resolved.
-    if (!reactions) {
-        if (GetPromiseState(this) === PROMISE_STATE_PENDING)
-            UnsafeSetReservedSlot(promise, PROMISE_REACTIONS_SLOT, [reaction]);
-        return;
-    }
-    _DefineDataProperty(reactions, reactions.length, reaction);
+    // The reactions list might not have been allocated yet.
+    if (!reactions)
+        UnsafeSetReservedSlot(dependentPromise, PROMISE_REACTIONS_OR_RESULT_SLOT, [reaction]);
+    else
+        _DefineDataProperty(reactions, reactions.length, reaction);
 }
 
 // ES6, 25.4.4.4.
 function Promise_static_reject(r) {
     // Step 1.
     let C = this;
 
     // Step 2.
@@ -734,39 +734,39 @@ function PerformPromiseThen(promise, onF
                                              onRejected,
                                              incumbentGlobal);
 
     // Step 7.
     let state = GetPromiseState(promise);
     if (state === PROMISE_STATE_PENDING) {
         // Steps 7.a,b.
         // We only have a single list for fulfill and reject reactions.
-        let reactions = UnsafeGetReservedSlot(promise, PROMISE_REACTIONS_SLOT);
+        let reactions = UnsafeGetReservedSlot(promise, PROMISE_REACTIONS_OR_RESULT_SLOT);
         if (!reactions)
-            UnsafeSetReservedSlot(promise, PROMISE_REACTIONS_SLOT, [reaction]);
+            UnsafeSetReservedSlot(promise, PROMISE_REACTIONS_OR_RESULT_SLOT, [reaction]);
         else
             _DefineDataProperty(reactions, reactions.length, reaction);
     }
 
     // Step 8.
     else if (state === PROMISE_STATE_FULFILLED) {
         // Step 8.a.
-        let value = UnsafeGetReservedSlot(promise, PROMISE_RESULT_SLOT);
+        let value = UnsafeGetReservedSlot(promise, PROMISE_REACTIONS_OR_RESULT_SLOT);
 
         // Step 8.b.
         EnqueuePromiseReactionJob(reaction, PROMISE_JOB_TYPE_FULFILL, value);
     }
 
     // Step 9.
     else {
         // Step 9.a.
         assert(state === PROMISE_STATE_REJECTED, "Invalid Promise state " + state);
 
         // Step 9.b.
-        let reason = UnsafeGetReservedSlot(promise, PROMISE_RESULT_SLOT);
+        let reason = UnsafeGetReservedSlot(promise, PROMISE_REACTIONS_OR_RESULT_SLOT);
 
         // Step 9.c.
         if (UnsafeGetInt32FromReservedSlot(promise, PROMISE_IS_HANDLED_SLOT) !==
             PROMISE_IS_HANDLED_STATE_HANDLED)
         {
             HostPromiseRejectionTracker(promise, PROMISE_REJECTION_TRACKER_OPERATION_HANDLE);
         }
 
--- a/js/src/builtin/SelfHostingDefines.h
+++ b/js/src/builtin/SelfHostingDefines.h
@@ -52,26 +52,25 @@
 #define ITERATOR_SLOT_ITEM_KIND 2
 // Used for ListIterator.
 #define ITERATOR_SLOT_NEXT_METHOD 2
 
 #define ITEM_KIND_KEY 0
 #define ITEM_KIND_VALUE 1
 #define ITEM_KIND_KEY_AND_VALUE 2
 
-#define PROMISE_STATE_SLOT             0
-#define PROMISE_RESULT_SLOT            1
-#define PROMISE_REACTIONS_SLOT         2
-#define PROMISE_RESOLVE_FUNCTION_SLOT  3
-#define PROMISE_ALLOCATION_SITE_SLOT   4
-#define PROMISE_RESOLUTION_SITE_SLOT   5
-#define PROMISE_ALLOCATION_TIME_SLOT   6
-#define PROMISE_RESOLUTION_TIME_SLOT   7
-#define PROMISE_ID_SLOT                8
-#define PROMISE_IS_HANDLED_SLOT        9
+#define PROMISE_STATE_SLOT               0
+#define PROMISE_REACTIONS_OR_RESULT_SLOT 1
+#define PROMISE_RESOLVE_FUNCTION_SLOT    2
+#define PROMISE_ALLOCATION_SITE_SLOT     3
+#define PROMISE_RESOLUTION_SITE_SLOT     4
+#define PROMISE_ALLOCATION_TIME_SLOT     5
+#define PROMISE_RESOLUTION_TIME_SLOT     6
+#define PROMISE_ID_SLOT                  7
+#define PROMISE_IS_HANDLED_SLOT          8
 
 #define PROMISE_STATE_PENDING   0
 #define PROMISE_STATE_FULFILLED 1
 #define PROMISE_STATE_REJECTED  2
 
 #define PROMISE_IS_HANDLED_STATE_HANDLED   0
 #define PROMISE_IS_HANDLED_STATE_UNHANDLED 1
 #define PROMISE_IS_HANDLED_STATE_REPORTED  2
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -1449,17 +1449,17 @@ SettlePromiseNow(JSContext* cx, unsigned
         return false;
     if (!args[0].isObject() || !args[0].toObject().is<PromiseObject>()) {
         JS_ReportError(cx, "first argument must be a Promise object");
         return false;
     }
 
     RootedNativeObject promise(cx, &args[0].toObject().as<NativeObject>());
     promise->setReservedSlot(PROMISE_STATE_SLOT, Int32Value(PROMISE_STATE_FULFILLED));
-    promise->setReservedSlot(PROMISE_RESULT_SLOT, UndefinedValue());
+    promise->setReservedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT, UndefinedValue());
 
     JS::dbg::onPromiseSettled(cx, promise);
     return true;
 }
 
 #else
 
 static const js::Class FakePromiseClass = {