[async-await] Fix INIT hook with --harmony-await-optimization

Split the runtime function for initializing a promise into AwaitPromisesInit
and AwaitPromisesInitOld, the former not firing the INIT hook and being used
by the AwaitOptimized builtin. In addition to this the AsyncHooks now caches
all the previously inited promises and checks that the init hook is not fired
twice for the same promise.

Modified test expectations for the new async ids in the async hooks tests.

Bug: v8:8300
Change-Id: If4a17e501b2a233578fa70b6442f219473f001d9
Reviewed-on: https://chromium-review.googlesource.com/c/1280442
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56642}

src/async-hooks-wrapper.cc

@@ -142,8 +142,13 @@ void AsyncHooks::ShellPromiseHook(PromiseHookType type, Local<Promise> promise,
     Local<Integer> async_id =
         Integer::New(hooks->isolate_, hooks->current_async_id);
 
+    CHECK(!promise
+               ->HasPrivate(currentContext,
+                            hooks->async_id_smb.Get(hooks->isolate_))
+               .ToChecked());
     promise->SetPrivate(currentContext,
                         hooks->async_id_smb.Get(hooks->isolate_), async_id);
+
     if (parent->IsPromise()) {
       Local<Promise> parent_promise = parent.As<Promise>();
       Local<Value> parent_async_id =

src/builtins/builtins-async-gen.cc

@@ -100,7 +100,7 @@ Node* AsyncBuiltinsAssembler::AwaitOld(Node* context, Node* generator,
   Branch(IsPromiseHookEnabledOrHasAsyncEventDelegate(), &if_debugging,
          &do_resolve_promise);
   BIND(&if_debugging);
-  var_throwaway.Bind(CallRuntime(Runtime::kAwaitPromisesInit, context, value,
+  var_throwaway.Bind(CallRuntime(Runtime::kAwaitPromisesInitOld, context, value,
                                  wrapped_value, outer_promise, on_reject,
                                  is_predicted_as_caught));
   Goto(&do_resolve_promise);
@@ -113,15 +113,13 @@ Node* AsyncBuiltinsAssembler::AwaitOld(Node* context, Node* generator,
                      on_resolve, on_reject, var_throwaway.value());
 }
 
-Node* AsyncBuiltinsAssembler::AwaitOptimized(
-    Node* context, Node* generator, Node* value, Node* outer_promise,
-    Node* on_resolve_context_index, Node* on_reject_context_index,
-    Node* is_predicted_as_caught) {
+Node* AsyncBuiltinsAssembler::AwaitOptimized(Node* context, Node* generator,
+                                             Node* promise, Node* outer_promise,
+                                             Node* on_resolve_context_index,
+                                             Node* on_reject_context_index,
+                                             Node* is_predicted_as_caught) {
   Node* const native_context = LoadNativeContext(context);
-  Node* const promise_fun =
-      LoadContextElement(native_context, Context::PROMISE_FUNCTION_INDEX);
-  CSA_ASSERT(this, IsFunctionWithPrototypeSlotMap(LoadMap(promise_fun)));
-  CSA_ASSERT(this, IsConstructor(promise_fun));
+  CSA_ASSERT(this, IsJSPromise(promise));
 
   static const int kResolveClosureOffset =
       FixedArray::SizeFor(Context::MIN_CONTEXT_SLOTS);
@@ -131,8 +129,8 @@ Node* AsyncBuiltinsAssembler::AwaitOptimized(
       kRejectClosureOffset + JSFunction::kSizeWithoutPrototype;
 
   // 2. Let promise be ? PromiseResolve(« promise »).
-  Node* const promise =
-      CallBuiltin(Builtins::kPromiseResolve, context, promise_fun, value);
+  // Node* const promise =
+  // CallBuiltin(Builtins::kPromiseResolve, context, promise_fun, value);
 
   Node* const base = AllocateInNewSpace(kTotalSize);
   Node* const closure_context = base;
@@ -174,7 +172,7 @@ Node* AsyncBuiltinsAssembler::AwaitOptimized(
   Branch(IsPromiseHookEnabledOrHasAsyncEventDelegate(), &if_debugging,
          &do_perform_promise_then);
   BIND(&if_debugging);
-  var_throwaway.Bind(CallRuntime(Runtime::kAwaitPromisesInit, context, value,
+  var_throwaway.Bind(CallRuntime(Runtime::kAwaitPromisesInit, context, promise,
                                  promise, outer_promise, on_reject,
                                  is_predicted_as_caught));
   Goto(&do_perform_promise_then);
@@ -190,16 +188,47 @@ Node* AsyncBuiltinsAssembler::Await(Node* context, Node* generator, Node* value,
                                     Node* on_reject_context_index,
                                     Node* is_predicted_as_caught) {
   VARIABLE(result, MachineRepresentation::kTagged);
-  Label if_old(this), if_new(this), done(this);
+  Label if_old(this), if_new(this), done(this),
+      if_slow_constructor(this, Label::kDeferred);
 
   STATIC_ASSERT(sizeof(FLAG_harmony_await_optimization) == 1);
-
   TNode<Word32T> flag_value = UncheckedCast<Word32T>(Load(
       MachineType::Uint8(),
       ExternalConstant(
           ExternalReference::address_of_harmony_await_optimization_flag())));
-
-  Branch(Word32Equal(flag_value, Int32Constant(0)), &if_old, &if_new);
+  GotoIf(Word32Equal(flag_value, Int32Constant(0)), &if_old);
+
+  // We're running with --harmony-await-optimization enabled, which means
+  // we do the `PromiseResolve(%Promise%,value)` avoiding to unnecessarily
+  // create wrapper promises. Now if {value} is already a promise with the
+  // intrinsics %Promise% constructor as its "constructor", we don't need
+  // to allocate the wrapper promise and can just use the `AwaitOptimized`
+  // logic.
+  GotoIf(TaggedIsSmi(value), &if_old);
+  Node* const value_map = LoadMap(value);
+  GotoIfNot(IsJSPromiseMap(value_map), &if_old);
+  // We can skip the "constructor" lookup on {value} if it's [[Prototype]]
+  // is the (initial) Promise.prototype and the @@species protector is
+  // intact, as that guards the lookup path for "constructor" on
+  // JSPromise instances which have the (initial) Promise.prototype.
+  Node* const native_context = LoadNativeContext(context);
+  Node* const promise_prototype =
+      LoadContextElement(native_context, Context::PROMISE_PROTOTYPE_INDEX);
+  GotoIfNot(WordEqual(LoadMapPrototype(value_map), promise_prototype),
+            &if_slow_constructor);
+  Branch(IsPromiseSpeciesProtectorCellInvalid(), &if_slow_constructor, &if_new);
+
+  // At this point, {value} doesn't have the initial promise prototype or
+  // the promise @@species protector was invalidated, but {value} could still
+  // have the %Promise% as its "constructor", so we need to check that as well.
+  BIND(&if_slow_constructor);
+  {
+    Node* const value_constructor =
+        GetProperty(context, value, isolate()->factory()->constructor_string());
+    Node* const promise_function =
+        LoadContextElement(native_context, Context::PROMISE_FUNCTION_INDEX);
+    Branch(WordEqual(value_constructor, promise_function), &if_new, &if_old);
+  }
 
   BIND(&if_old);
   result.Bind(AwaitOld(context, generator, value, outer_promise,

src/runtime/runtime-promise.cc

@@ -131,18 +131,18 @@ RUNTIME_FUNCTION(Runtime_PromiseHookInit) {
   return ReadOnlyRoots(isolate).undefined_value();
 }
 
-RUNTIME_FUNCTION(Runtime_AwaitPromisesInit) {
-  DCHECK_EQ(5, args.length());
-  HandleScope scope(isolate);
-  CONVERT_ARG_HANDLE_CHECKED(Object, value, 0);
-  CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 1);
-  CONVERT_ARG_HANDLE_CHECKED(JSPromise, outer_promise, 2);
-  CONVERT_ARG_HANDLE_CHECKED(JSFunction, reject_handler, 3);
-  CONVERT_BOOLEAN_ARG_CHECKED(is_predicted_as_caught, 4);
+namespace {
 
-  // Allocate the throwaway promise and fire the appropriate PromiseHooks.
+Handle<JSPromise> AwaitPromisesInitCommon(Isolate* isolate,
+                                          Handle<Object> value,
+                                          Handle<JSPromise> promise,
+                                          Handle<JSPromise> outer_promise,
+                                          Handle<JSFunction> reject_handler,
+                                          bool is_predicted_as_caught) {
+  // Allocate the throwaway promise and fire the appropriate init
+  // hook for the throwaway promise (passing the {promise} as its
+  // parent).
   Handle<JSPromise> throwaway = isolate->factory()->NewJSPromiseWithoutHook();
-  isolate->RunPromiseHook(PromiseHookType::kInit, promise, outer_promise);
   isolate->RunPromiseHook(PromiseHookType::kInit, throwaway, promise);
 
   // On inspector side we capture async stack trace and store it by
@@ -175,7 +175,37 @@ RUNTIME_FUNCTION(Runtime_AwaitPromisesInit) {
         .Check();
   }
 
-  return *throwaway;
+  return throwaway;
+}
+
+}  // namespace
+
+RUNTIME_FUNCTION(Runtime_AwaitPromisesInit) {
+  DCHECK_EQ(5, args.length());
+  HandleScope scope(isolate);
+  CONVERT_ARG_HANDLE_CHECKED(Object, value, 0);
+  CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 1);
+  CONVERT_ARG_HANDLE_CHECKED(JSPromise, outer_promise, 2);
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, reject_handler, 3);
+  CONVERT_BOOLEAN_ARG_CHECKED(is_predicted_as_caught, 4);
+  return *AwaitPromisesInitCommon(isolate, value, promise, outer_promise,
+                                  reject_handler, is_predicted_as_caught);
+}
+
+RUNTIME_FUNCTION(Runtime_AwaitPromisesInitOld) {
+  DCHECK_EQ(5, args.length());
+  HandleScope scope(isolate);
+  CONVERT_ARG_HANDLE_CHECKED(Object, value, 0);
+  CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 1);
+  CONVERT_ARG_HANDLE_CHECKED(JSPromise, outer_promise, 2);
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, reject_handler, 3);
+  CONVERT_BOOLEAN_ARG_CHECKED(is_predicted_as_caught, 4);
+
+  // Fire the init hook for the wrapper promise (that we created for the
+  // {value} previously).
+  isolate->RunPromiseHook(PromiseHookType::kInit, promise, outer_promise);
+  return *AwaitPromisesInitCommon(isolate, value, promise, outer_promise,
+                                  reject_handler, is_predicted_as_caught);
 }
 
 RUNTIME_FUNCTION(Runtime_PromiseHookBefore) {

src/runtime/runtime.h

@@ -353,6 +353,7 @@ namespace internal {
   F(PromiseHookBefore, 1, 1)             \
   F(PromiseHookInit, 2, 1)               \
   F(AwaitPromisesInit, 5, 1)             \
+  F(AwaitPromisesInitOld, 5, 1)          \
   F(PromiseMarkAsHandled, 1, 1)          \
   F(PromiseRejectEventFromStack, 2, 1)   \
   F(PromiseResult, 1, 1)                 \

test/mjsunit/async-hooks/async-await-tree.js

@@ -25,7 +25,7 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-// Flags: --expose-async-hooks
+// Flags: --expose-async-hooks --harmony-await-optimization
 
 // Check for async/await asyncIds relation
 (function() {
@@ -66,10 +66,9 @@
 
   assertPromiseResult(
     foo().then(function() {
-      assertEquals(triggerIds[2], asyncIds[0]);
-      assertEquals(triggerIds[3], asyncIds[2]);
-      assertEquals(triggerIds[4], asyncIds[0]);
-      assertEquals(triggerIds[5], asyncIds[4]);
-      assertEquals(triggerIds[7], asyncIds[6]);
+      assertEquals(triggerIds[2], asyncIds[1]);
+      assertEquals(triggerIds[3], asyncIds[0]);
+      assertEquals(triggerIds[4], asyncIds[3]);
+      assertEquals(triggerIds[6], asyncIds[5]);
     }));
 })();