[promise] Reset promise context slot to undefined after closure is run

We no longer need the kAlreadyVisitedSlot because we can just check
for undefined in the kPromiseSlot to know if the clsoure was already
fulfilled.

This means we save one word per context per promise resolving closure.

Bug: v8:7037
Change-Id: Ib8f0fb445d2e143714d57fe644ba6d7a3f04c1f7
Reviewed-on: https://chromium-review.googlesource.com/756176Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49200}

src/builtins/builtins-promise-gen.cc

@@ -221,8 +221,6 @@ Node* PromiseBuiltinsAssembler::CreatePromiseResolvingFunctionsContext(
     Node* promise, Node* debug_event, Node* native_context) {
   Node* const context =
       CreatePromiseContext(native_context, kPromiseContextLength);
-  StoreContextElementNoWriteBarrier(context, kAlreadyVisitedSlot,
-                                    SmiConstant(0));
   StoreContextElementNoWriteBarrier(context, kPromiseSlot, promise);
   StoreContextElementNoWriteBarrier(context, kDebugEventSlot, debug_event);
   return context;
@@ -979,37 +977,39 @@ void PromiseBuiltinsAssembler::SetPromiseHandledByIfTrue(
   BIND(&done);
 }
 
-// ES#sec-promise-reject-functions
-// Promise Reject Functions
-TF_BUILTIN(PromiseRejectClosure, PromiseBuiltinsAssembler) {
-  Node* const value = Parameter(Descriptor::kValue);
-  Node* const context = Parameter(Descriptor::kContext);
-
+void PromiseBuiltinsAssembler::PerformFulfillClosure(Node* context, Node* value,
+                                                     bool should_resolve) {
   Label out(this);
 
-  // 3. Let alreadyResolved be F.[[AlreadyResolved]].
-  int has_already_visited_slot = kAlreadyVisitedSlot;
-
-  Node* const has_already_visited =
-      LoadContextElement(context, has_already_visited_slot);
-
-  // 4. If alreadyResolved.[[Value]] is true, return undefined.
-  GotoIf(SmiEqual(has_already_visited, SmiConstant(1)), &out);
-
-  // 5.Set alreadyResolved.[[Value]] to true.
-  StoreContextElementNoWriteBarrier(context, has_already_visited_slot,
-                                    SmiConstant(1));
-
   // 2. Let promise be F.[[Promise]].
-  Node* const promise =
-      LoadContextElement(context, IntPtrConstant(kPromiseSlot));
-  Node* const debug_event =
-      LoadContextElement(context, IntPtrConstant(kDebugEventSlot));
+  Node* const promise_slot = IntPtrConstant(kPromiseSlot);
+  Node* const promise = LoadContextElement(context, promise_slot);
+
+  // We use `undefined` as a marker to know that this callback was
+  // already called.
+  GotoIf(IsUndefined(promise), &out);
+
+  if (should_resolve) {
+    InternalResolvePromise(context, promise, value);
+  } else {
+    Node* const debug_event =
+        LoadContextElement(context, IntPtrConstant(kDebugEventSlot));
+    InternalPromiseReject(context, promise, value, debug_event);
+  }
 
-  InternalPromiseReject(context, promise, value, debug_event);
-  Return(UndefinedConstant());
+  StoreContextElement(context, promise_slot, UndefinedConstant());
+  Goto(&out);
 
   BIND(&out);
+}
+
+// ES#sec-promise-reject-functions
+// Promise Reject Functions
+TF_BUILTIN(PromiseRejectClosure, PromiseBuiltinsAssembler) {
+  Node* const value = Parameter(Descriptor::kValue);
+  Node* const context = Parameter(Descriptor::kContext);
+
+  PerformFulfillClosure(context, value, false);
   Return(UndefinedConstant());
 }
 
@@ -1153,29 +1153,7 @@ TF_BUILTIN(PromiseResolveClosure, PromiseBuiltinsAssembler) {
   Node* const value = Parameter(Descriptor::kValue);
   Node* const context = Parameter(Descriptor::kContext);
 
-  Label out(this);
-
-  // 3. Let alreadyResolved be F.[[AlreadyResolved]].
-  int has_already_visited_slot = kAlreadyVisitedSlot;
-
-  Node* const has_already_visited =
-      LoadContextElement(context, has_already_visited_slot);
-
-  // 4. If alreadyResolved.[[Value]] is true, return undefined.
-  GotoIf(SmiEqual(has_already_visited, SmiConstant(1)), &out);
-
-  // 5.Set alreadyResolved.[[Value]] to true.
-  StoreContextElementNoWriteBarrier(context, has_already_visited_slot,
-                                    SmiConstant(1));
-
-  // 2. Let promise be F.[[Promise]].
-  Node* const promise =
-      LoadContextElement(context, IntPtrConstant(kPromiseSlot));
-
-  InternalResolvePromise(context, promise, value);
-  Return(UndefinedConstant());
-
-  BIND(&out);
+  PerformFulfillClosure(context, value, true);
   Return(UndefinedConstant());
 }
 

src/builtins/builtins-promise-gen.h

@@ -16,11 +16,10 @@ typedef compiler::CodeAssemblerState CodeAssemblerState;
 class PromiseBuiltinsAssembler : public CodeStubAssembler {
  public:
   enum PromiseResolvingFunctionContextSlot {
-    // Whether the resolve/reject callback was already called.
-    kAlreadyVisitedSlot = Context::MIN_CONTEXT_SLOTS,
-
-    // The promise which resolve/reject callbacks fulfill.
-    kPromiseSlot,
+    // The promise which resolve/reject callbacks fulfill. If this is
+    // undefined, then we've already visited this callback and it
+    // should be a no-op.
+    kPromiseSlot = Context::MIN_CONTEXT_SLOTS,
 
     // Whether to trigger a debug event or not. Used in catch
     // prediction.
@@ -176,6 +175,7 @@ class PromiseBuiltinsAssembler : public CodeStubAssembler {
                                  const NodeGenerator& handled_by);
 
   Node* PromiseStatus(Node* promise);
+  void PerformFulfillClosure(Node* context, Node* value, bool should_resolve);
 
  private:
   Node* IsPromiseStatus(Node* actual, v8::Promise::PromiseState expected);

test/cctest/test-code-stub-assembler.cc

@@ -2133,8 +2133,6 @@ TEST(CreatePromiseResolvingFunctionsContext) {
   CHECK_EQ(isolate->native_context()->closure(), context_js->closure());
   CHECK_EQ(isolate->heap()->the_hole_value(), context_js->extension());
   CHECK_EQ(*isolate->native_context(), context_js->native_context());
-  CHECK_EQ(Smi::FromInt(0),
-           context_js->get(PromiseBuiltinsAssembler::kAlreadyVisitedSlot));
   CHECK(context_js->get(PromiseBuiltinsAssembler::kPromiseSlot)->IsJSPromise());
   CHECK_EQ(isolate->heap()->false_value(),
            context_js->get(PromiseBuiltinsAssembler::kDebugEventSlot));