[promises] Remove deferred object

src/builtins/builtins-promise.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/builtins/builtins-promise.h"
 #include "src/builtins/builtins-utils.h"
 #include "src/builtins/builtins.h"
 #include "src/code-factory.h"
 #include "src/code-stub-assembler.h"
 #include "src/promise-utils.h"
 
 namespace v8 {
 namespace internal {
 
 typedef compiler::Node Node;
 typedef CodeStubAssembler::ParameterMode ParameterMode;
 typedef compiler::CodeAssemblerState CodeAssemblerState;
 
+Node* PromiseBuiltinsAssembler::AllocateAndInitPromise(Node* context,
+                                                       Node* parent) {
+  Node* const instance = AllocateJSPromise(context);
+  PromiseInit(instance);
+
+  Label out(this);
+  GotoUnless(IsPromiseHookEnabled(), &out);
+  CallRuntime(Runtime::kPromiseHookInit, context, instance, parent);
+  Goto(&out);
+
+  Bind(&out);
+  return instance;
+}
+
 Node* PromiseBuiltinsAssembler::CreatePromiseResolvingFunctionsContext(
     Node* promise, Node* debug_event, Node* native_context) {
   Node* const context =
       Allocate(FixedArray::SizeFor(PromiseUtils::kPromiseContextLength));
   StoreMapNoWriteBarrier(context, Heap::kFunctionContextMapRootIndex);
   StoreObjectFieldNoWriteBarrier(
       context, FixedArray::kLengthOffset,
       SmiConstant(PromiseUtils::kPromiseContextLength));
 
   Node* const empty_fn =
@@ skipping 145 matching lines @@
   Node* new_elements = AllocateFixedArray(kind, new_capacity, mode, flags);
 
   CopyFixedArrayElements(kind, elements, new_elements, length, barrier_mode,
                          mode);
   StoreFixedArrayElement(new_elements, length, value, barrier_mode,
                          additional_offset, mode);
 
   StoreObjectField(promise, offset, new_elements);
 }
 
-Node* PromiseBuiltinsAssembler::InternalPerformPromiseThen(Node* context,
-                                                           Node* promise,
-                                                           Node* on_resolve,
-                                                           Node* on_reject,
-                                                           Node* deferred) {
+Node* PromiseBuiltinsAssembler::InternalPerformPromiseThen(
+    Node* context, Node* promise, Node* on_resolve, Node* on_reject,
+    Node* deferred_promise, Node* deferred_on_resolve,
+    Node* deferred_on_reject) {
   Node* const native_context = LoadNativeContext(context);
 
   Variable var_on_resolve(this, MachineRepresentation::kTagged),
       var_on_reject(this, MachineRepresentation::kTagged);
 
   var_on_resolve.Bind(on_resolve);
   var_on_reject.Bind(on_reject);
 
   Label out(this), if_onresolvenotcallable(this), onrejectcheck(this),
       append_callbacks(this);
@@ skipping 27 matching lines @@
     }
   }
 
   Bind(&append_callbacks);
   {
     Label fulfilled_check(this);
     Node* const status = LoadObjectField(promise, JSPromise::kStatusOffset);
     GotoUnless(SmiEqual(status, SmiConstant(v8::Promise::kPending)),
                &fulfilled_check);
 
-    Node* const existing_deferred =
-        LoadObjectField(promise, JSPromise::kDeferredOffset);
+    Node* const existing_deferred_promise =
+        LoadObjectField(promise, JSPromise::kDeferredPromiseOffset);
 
     Label if_noexistingcallbacks(this), if_existingcallbacks(this);
-    Branch(IsUndefined(existing_deferred), &if_noexistingcallbacks,
+    Branch(IsUndefined(existing_deferred_promise), &if_noexistingcallbacks,
            &if_existingcallbacks);
 
     Bind(&if_noexistingcallbacks);
     {
       // Store callbacks directly in the slots.
-      StoreObjectField(promise, JSPromise::kDeferredOffset, deferred);
+      StoreObjectField(promise, JSPromise::kDeferredPromiseOffset,
+                       deferred_promise);
+      StoreObjectField(promise, JSPromise::kDeferredOnResolveOffset,
+                       deferred_on_resolve);
+      StoreObjectField(promise, JSPromise::kDeferredOnRejectOffset,
+                       deferred_on_reject);
       StoreObjectField(promise, JSPromise::kFulfillReactionsOffset,
                        var_on_resolve.value());
       StoreObjectField(promise, JSPromise::kRejectReactionsOffset,
                        var_on_reject.value());
       Goto(&out);
     }
 
     Bind(&if_existingcallbacks);
     {
       Label if_singlecallback(this), if_multiplecallbacks(this);
-      BranchIfJSObject(existing_deferred, &if_singlecallback,
+      BranchIfJSObject(existing_deferred_promise, &if_singlecallback,
                        &if_multiplecallbacks);
 
       Bind(&if_singlecallback);
       {
         // Create new FixedArrays to store callbacks, and migrate
         // existing callbacks.
-        Node* const deferreds =
+        Node* const deferred_promise_arr =
             AllocateFixedArray(FAST_ELEMENTS, IntPtrConstant(2));
-        StoreFixedArrayElement(deferreds, 0, existing_deferred);
-        StoreFixedArrayElement(deferreds, 1, deferred);
+        StoreFixedArrayElement(deferred_promise_arr, 0,
+                               existing_deferred_promise);
+        StoreFixedArrayElement(deferred_promise_arr, 1, deferred_promise);

[jgruber, 2016/12/21 08:08:28]

Maybe refactor this into a NewFixedArrayWithElements helper on
PromiseBuiltinsAssembler?

[gsathya, 2016/12/21 19:26:28]

This is slightly tricky and doesn't conform together well enough to be
refactored out because I have the existing_deferred_promise already in the first
case but not for the rest.

Leaving this as such for now. 

+
+        Node* const deferred_on_resolve_arr =
+            AllocateFixedArray(FAST_ELEMENTS, IntPtrConstant(2));
+        StoreFixedArrayElement(
+            deferred_on_resolve_arr, 0,
+            LoadObjectField(promise, JSPromise::kDeferredOnResolveOffset));
+        StoreFixedArrayElement(deferred_on_resolve_arr, 1, deferred_on_resolve);
+
+        Node* const deferred_on_reject_arr =
+            AllocateFixedArray(FAST_ELEMENTS, IntPtrConstant(2));
+        StoreFixedArrayElement(
+            deferred_on_reject_arr, 0,
+            LoadObjectField(promise, JSPromise::kDeferredOnRejectOffset));
+        StoreFixedArrayElement(deferred_on_reject_arr, 1, deferred_on_reject);
 
         Node* const fulfill_reactions =
             AllocateFixedArray(FAST_ELEMENTS, IntPtrConstant(2));
         StoreFixedArrayElement(
             fulfill_reactions, 0,
             LoadObjectField(promise, JSPromise::kFulfillReactionsOffset));
         StoreFixedArrayElement(fulfill_reactions, 1, var_on_resolve.value());
 
         Node* const reject_reactions =
             AllocateFixedArray(FAST_ELEMENTS, IntPtrConstant(2));
         StoreFixedArrayElement(
             reject_reactions, 0,
             LoadObjectField(promise, JSPromise::kRejectReactionsOffset));
         StoreFixedArrayElement(reject_reactions, 1, var_on_reject.value());
 
         // Store new FixedArrays in promise.
-        StoreObjectField(promise, JSPromise::kDeferredOffset, deferreds);
+        StoreObjectField(promise, JSPromise::kDeferredPromiseOffset,
+                         deferred_promise_arr);
+        StoreObjectField(promise, JSPromise::kDeferredOnResolveOffset,
+                         deferred_on_resolve_arr);
+        StoreObjectField(promise, JSPromise::kDeferredOnRejectOffset,
+                         deferred_on_reject_arr);
         StoreObjectField(promise, JSPromise::kFulfillReactionsOffset,
                          fulfill_reactions);
         StoreObjectField(promise, JSPromise::kRejectReactionsOffset,
                          reject_reactions);
         Goto(&out);
       }
 
       Bind(&if_multiplecallbacks);
       {
-        AppendPromiseCallback(JSPromise::kDeferredOffset, promise, deferred);
+        AppendPromiseCallback(JSPromise::kDeferredPromiseOffset, promise,
+                              deferred_promise);
+        AppendPromiseCallback(JSPromise::kDeferredOnResolveOffset, promise,
+                              deferred_on_resolve);
+        AppendPromiseCallback(JSPromise::kDeferredOnRejectOffset, promise,
+                              deferred_on_reject);
         AppendPromiseCallback(JSPromise::kFulfillReactionsOffset, promise,
                               var_on_resolve.value());
         AppendPromiseCallback(JSPromise::kRejectReactionsOffset, promise,
                               var_on_reject.value());
         Goto(&out);
       }
     }
 
     Bind(&fulfilled_check);
     {
       Label reject(this);
       Node* const result = LoadObjectField(promise, JSPromise::kResultOffset);
       GotoUnless(WordEqual(status, SmiConstant(v8::Promise::kFulfilled)),
                  &reject);
 
-      // TODO(gsathya): Move this to TF.
-      CallRuntime(Runtime::kEnqueuePromiseReactionJob, context, promise, result,
-                  var_on_resolve.value(), deferred,
+      Node* info = AllocatePromiseReactionJobInfo(
+          promise, result, var_on_resolve.value(), deferred_promise,
+          deferred_on_resolve, deferred_on_reject, context);
+      CallRuntime(Runtime::kEnqueuePromiseReactionJob, context, info,

[jgruber, 2016/12/21 08:08:28]

It'd be awesome to move this to CSA as well at some point. Could you add a TODO?

[gsathya, 2016/12/21 19:26:28]

Done.

                   SmiConstant(v8::Promise::kFulfilled));
       Goto(&out);
 
       Bind(&reject);
       {
         Node* const has_handler = PromiseHasHandler(promise);
         Label enqueue(this);
 
         // TODO(gsathya): Fold these runtime calls and move to TF.
         GotoIf(has_handler, &enqueue);
         CallRuntime(Runtime::kPromiseRevokeReject, context, promise);
         Goto(&enqueue);
 
         Bind(&enqueue);
         {
-          CallRuntime(Runtime::kEnqueuePromiseReactionJob, context, promise,
-                      result, var_on_reject.value(), deferred,
+          Node* info = AllocatePromiseReactionJobInfo(
+              promise, result, var_on_reject.value(), deferred_promise,
+              deferred_on_resolve, deferred_on_reject, context);
+          CallRuntime(Runtime::kEnqueuePromiseReactionJob, context, info,
                       SmiConstant(v8::Promise::kRejected));
-
           Goto(&out);
         }
       }
     }
   }
 
   Bind(&out);
   PromiseSetHasHandler(promise);

[jgruber, 2016/12/21 08:08:28]

By the way, we could make PromiseSetHasHandler cheaper by not smi-untagging &
tagging, and storing the smi without a write barrier.

[jgruber, 2016/12/21 08:45:07]

CL for that here: https://codereview.chromium.org/2591193002/

[gsathya, 2016/12/21 19:26:28]

Thanks!

-
-  // TODO(gsathya): This call will be removed once we don't have to
-  // deal with deferred objects.
-  Isolate* isolate = this->isolate();
-  Callable getproperty_callable = CodeFactory::GetProperty(isolate);
-  Node* const key =
-      HeapConstant(isolate->factory()->NewStringFromAsciiChecked("promise"));
-  Node* const result = CallStub(getproperty_callable, context, deferred, key);
-
-  return result;
+  return deferred_promise;
 }
 
 // Promise fast path implementations rely on unmodified JSPromise instances.
 // We use a fairly coarse granularity for this and simply check whether both
 // the promise itself is unmodified (i.e. its map has not changed) and its
 // prototype is unmodified.
 // TODO(gsathya): Refactor this out to prevent code dupe with builtins-regexp
 void PromiseBuiltinsAssembler::BranchIfFastPath(Node* context, Node* promise,
                                                 Label* if_isunmodified,
                                                 Label* if_ismodified) {
@@ skipping 339 matching lines @@
     Node* const message_id =
         SmiConstant(MessageTemplate::kResolverNotAFunction);
     CallRuntime(Runtime::kThrowTypeError, context, message_id, executor);
     Return(UndefinedConstant());  // Never reached.
   }
 }
 
 TF_BUILTIN(PromiseInternalConstructor, PromiseBuiltinsAssembler) {
   Node* const parent = Parameter(1);
   Node* const context = Parameter(4);
-  Node* const instance = AllocateJSPromise(context);
-  PromiseInit(instance);
-
-  Label out(this);
-  GotoUnless(IsPromiseHookEnabled(), &out);
-  CallRuntime(Runtime::kPromiseHookInit, context, instance, parent);
-  Goto(&out);
-  Bind(&out);
-
-  Return(instance);
+  Return(AllocateAndInitPromise(context, parent));
 }
 
 TF_BUILTIN(PromiseCreateAndSet, PromiseBuiltinsAssembler) {
   Node* const status = Parameter(1);
   Node* const result = Parameter(2);
   Node* const context = Parameter(5);
 
   Node* const instance = AllocateJSPromise(context);
   PromiseSet(instance, status, result);
 
@@ skipping 17 matching lines @@
   Return(result);
 
   Bind(&if_notpromise);
   Return(FalseConstant());
 }
 
 TF_BUILTIN(PerformPromiseThen, PromiseBuiltinsAssembler) {
   Node* const promise = Parameter(1);
   Node* const on_resolve = Parameter(2);
   Node* const on_reject = Parameter(3);
-  Node* const deferred = Parameter(4);
+  Node* const deferred_promise = Parameter(4);
   Node* const context = Parameter(7);
 
-  Node* const result = InternalPerformPromiseThen(context, promise, on_resolve,
-                                                  on_reject, deferred);
+  // No deferred_on_resolve/deferred_on_reject because this is just an
+  // internal promise created by async-await.
+  Node* const result = InternalPerformPromiseThen(
+      context, promise, on_resolve, on_reject, deferred_promise,
+      UndefinedConstant(), UndefinedConstant());
 
   // TODO(gsathya): This is unused, but value is returned according to spec.
   Return(result);
 }
 
 TF_BUILTIN(PromiseThen, PromiseBuiltinsAssembler) {
   // 1. Let promise be the this value.
   Node* const promise = Parameter(0);
   Node* const on_resolve = Parameter(1);
   Node* const on_reject = Parameter(2);
   Node* const context = Parameter(5);
   Isolate* isolate = this->isolate();
 
   // 2. If IsPromise(promise) is false, throw a TypeError exception.
   ThrowIfNotInstanceType(context, promise, JS_PROMISE_TYPE,
                          "Promise.prototype.then");
 
   Node* const native_context = LoadNativeContext(context);
   Node* const promise_fun =
       LoadContextElement(native_context, Context::PROMISE_FUNCTION_INDEX);
 
   // 3. Let C be ? SpeciesConstructor(promise, %Promise%).
   Node* constructor = SpeciesConstructor(context, promise, promise_fun);
 
   // 4. Let resultCapability be ? NewPromiseCapability(C).
   Callable call_callable = CodeFactory::Call(isolate);
   Label fast_promise_capability(this), promise_capability(this),
       perform_promise_then(this);
-  Variable var_deferred(this, MachineRepresentation::kTagged);
+  Variable var_deferred_promise(this, MachineRepresentation::kTagged),
+      var_deferred_on_resolve(this, MachineRepresentation::kTagged),
+      var_deferred_on_reject(this, MachineRepresentation::kTagged);
 
   Branch(WordEqual(promise_fun, constructor), &fast_promise_capability,
          &promise_capability);
 
-  // TODO(gsathya): Remove deferred object and move
-  // NewPromiseCapabability functions to TF.
   Bind(&fast_promise_capability);
   {
-    // TODO(gsathya): Move this to TF.
-    Node* const promise_internal_capability = LoadContextElement(
-        native_context, Context::INTERNAL_PROMISE_CAPABILITY_INDEX);
-    Node* const capability =
-        CallJS(call_callable, context, promise_internal_capability,
-               UndefinedConstant(), promise);
-    var_deferred.Bind(capability);
+    Node* const deferred_promise = AllocateAndInitPromise(context, promise);
+    PromiseInit(deferred_promise);
+    var_deferred_promise.Bind(deferred_promise);
+    var_deferred_on_resolve.Bind(UndefinedConstant());
+    var_deferred_on_reject.Bind(UndefinedConstant());
     Goto(&perform_promise_then);
   }
 
   Bind(&promise_capability);
   {
     // TODO(gsathya): Move this to TF.
     Node* const new_promise_capability = LoadContextElement(
         native_context, Context::NEW_PROMISE_CAPABILITY_INDEX);
-    Node* const capability =
+    Node* const deferred =
         CallJS(call_callable, context, new_promise_capability,
                UndefinedConstant(), constructor);
-    var_deferred.Bind(capability);
+    Callable getproperty_callable = CodeFactory::GetProperty(isolate);
+    Node* key = HeapConstant(isolate->factory()->promise_string());
+    Node* const deferred_promise =
+        CallStub(getproperty_callable, context, deferred, key);
+    var_deferred_promise.Bind(deferred_promise);
+
+    key = HeapConstant(isolate->factory()->resolve_string());
+    Node* const deferred_on_resolve =
+        CallStub(getproperty_callable, context, deferred, key);
+    var_deferred_on_resolve.Bind(deferred_on_resolve);
+
+    key = HeapConstant(isolate->factory()->reject_string());
+    Node* const deferred_on_reject =
+        CallStub(getproperty_callable, context, deferred, key);
+    var_deferred_on_reject.Bind(deferred_on_reject);
+
     Goto(&perform_promise_then);
   }
 
   // 5. Return PerformPromiseThen(promise, onFulfilled, onRejected,
   //    resultCapability).
   Bind(&perform_promise_then);
   Node* const result = InternalPerformPromiseThen(
-      context, promise, on_resolve, on_reject, var_deferred.value());
+      context, promise, on_resolve, on_reject, var_deferred_promise.value(),
+      var_deferred_on_resolve.value(), var_deferred_on_reject.value());
   Return(result);
 }
 
 // ES#sec-promise-resolve-functions
 // Promise Resolve Functions
 TF_BUILTIN(PromiseResolveClosure, PromiseBuiltinsAssembler) {
   Node* const value = Parameter(1);
   Node* const context = Parameter(4);
 
   Label out(this);
@@ skipping 58 matching lines @@
   {
     CallJS(call_callable, context, on_reject, UndefinedConstant(), exception);
     Return(UndefinedConstant());
   }
 }
 
 TF_BUILTIN(PromiseHandle, PromiseBuiltinsAssembler) {
   Node* const promise = Parameter(1);
   Node* const value = Parameter(2);
   Node* const handler = Parameter(3);
-  Node* const deferred = Parameter(4);
-  Node* const context = Parameter(7);
+  Node* const deferred_promise = Parameter(4);
+  Node* const deferred_on_resolve = Parameter(5);
+  Node* const deferred_on_reject = Parameter(6);
+  Node* const context = Parameter(9);
   Isolate* isolate = this->isolate();
 
   // Get promise from deferred

[jgruber, 2016/12/21 08:08:28]

Leftover comment?

[gsathya, 2016/12/21 19:26:28]

Done.

-  // TODO(gsathya): Remove this lookup by getting rid of the deferred object.
-  Callable getproperty_callable = CodeFactory::GetProperty(isolate);
-  Node* const key = HeapConstant(isolate->factory()->promise_string());
-  Node* const deferred_promise =
-      CallStub(getproperty_callable, context, deferred, key);
-
   Variable var_reason(this, MachineRepresentation::kTagged);
 
   Node* const is_debug_active = IsDebugActive();
   Label run_handler(this), if_rejectpromise(this), promisehook_before(this),
       promisehook_after(this), debug_pop(this);
 
   GotoUnless(is_debug_active, &promisehook_before);
   CallRuntime(Runtime::kDebugPushPromise, context, deferred_promise);
   Goto(&promisehook_before);
 
   Bind(&promisehook_before);
   {
     GotoUnless(IsPromiseHookEnabled(), &run_handler);
     CallRuntime(Runtime::kPromiseHookBefore, context, promise);
     Goto(&run_handler);
   }
 
   Bind(&run_handler);
   {
     Callable call_callable = CodeFactory::Call(isolate);
-
     Node* const result =
         CallJS(call_callable, context, handler, UndefinedConstant(), value);
 
     GotoIfException(result, &if_rejectpromise, &var_reason);
 
-    // TODO(gsathya): Remove this lookup by getting rid of the deferred object.
-    Node* const key = HeapConstant(isolate->factory()->resolve_string());
-    Node* const on_resolve =
-        CallStub(getproperty_callable, context, deferred, key);
-
     Label if_internalhandler(this), if_customhandler(this, Label::kDeferred);
-    Branch(IsUndefined(on_resolve), &if_internalhandler, &if_customhandler);
+    Branch(IsUndefined(deferred_on_resolve), &if_internalhandler,
+           &if_customhandler);
 
     Bind(&if_internalhandler);
     InternalResolvePromise(context, deferred_promise, result,
                            &promisehook_after);
 
     Bind(&if_customhandler);
     {
-      Node* const maybe_exception = CallJS(call_callable, context, on_resolve,
-                                           UndefinedConstant(), result);
+      Node* const maybe_exception =
+          CallJS(call_callable, context, deferred_on_resolve,
+                 UndefinedConstant(), result);
       GotoIfException(maybe_exception, &if_rejectpromise, &var_reason);
       Goto(&promisehook_after);
     }
   }
 
   Bind(&if_rejectpromise);
   {
-    // TODO(gsathya): Remove this lookup by getting rid of the deferred object.
-    Node* const key = HeapConstant(isolate->factory()->reject_string());
-    Node* const on_reject =
-        CallStub(getproperty_callable, context, deferred, key);
-
     Callable promise_handle_reject = CodeFactory::PromiseHandleReject(isolate);
-    CallStub(promise_handle_reject, context, deferred_promise, on_reject,
-             var_reason.value());
+    CallStub(promise_handle_reject, context, deferred_promise,
+             deferred_on_reject, var_reason.value());
     Goto(&promisehook_after);
   }
 
   Bind(&promisehook_after);
   {
     GotoUnless(IsPromiseHookEnabled(), &debug_pop);
     CallRuntime(Runtime::kPromiseHookAfter, context, promise);
     Goto(&debug_pop);
   }
 
   Bind(&debug_pop);
   {
     Label out(this);
 
     GotoUnless(is_debug_active, &out);
     CallRuntime(Runtime::kDebugPopPromise, context);
     Goto(&out);
 
     Bind(&out);
     Return(UndefinedConstant());
   }
 }
 
 }  // namespace internal
 }  // namespace v8