[async-generators] Add fast-path to skip "then" lookup in AsyncGeneratorResolve.

bmeurer · Commit Bot · commit c15802e11e30 · 2018-03-19T12:21:13.000Z
This extends the Promise#then protector to also guard the intrinsic %ObjectPrototype%, making it usable for fast-path logic in the AsyncGeneratorResolve operation, where we can skip the "then" lookup on the iteratorResult in that case. We also add a corresponding fast- path to the ResolvePromise builtin itself, which avoids the second "then" lookup on the async iterator side. This reduces execution time of the fibonacci-async-es2017-native test from 186.9ms to 171.2ms, which corresponds to an improvement of 8.4% in this case. Bug: v8:7253 Change-Id: Ia09effc81a016703780d5c308f541e3d797ccabe Reviewed-on: https://chromium-review.googlesource.com/967203 Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#52024}
diff --git a/src/builtins/builtins-async-generator-gen.cc b/src/builtins/builtins-async-generator-gen.cc
@@ -518,11 +518,34 @@ TF_BUILTIN(AsyncGeneratorResolve, AsyncGeneratorBuiltinsAssembler) {
                                    done);
   }
 
-  // Perform Call(promiseCapability.[[Resolve]], undefined, «iteratorResult»).
-  CallBuiltin(Builtins::kResolvePromise, context, promise, iter_result);
+  // We know that {iter_result} itself doesn't have any "then" property and
+  // we also know that the [[Prototype]] of {iter_result} is the intrinsic
+  // %ObjectPrototype%. So we can skip the [[Resolve]] logic here completely
+  // and directly call into the FulfillPromise operation if we can prove
+  // that the %ObjectPrototype% also doesn't have any "then" property. This
+  // is guarded by the Promise#then protector.
+  Label if_fast(this), if_slow(this, Label::kDeferred), return_promise(this);
+  GotoIfForceSlowPath(&if_slow);
+  Branch(IsPromiseThenProtectorCellInvalid(), &if_slow, &if_fast);
+
+  BIND(&if_fast);
+  {
+    // Skip the "then" on {iter_result} and directly fulfill the {promise}
+    // with the {iter_result}.
+    CallBuiltin(Builtins::kFulfillPromise, context, promise, iter_result);
+    Goto(&return_promise);
+  }
+
+  BIND(&if_slow);
+  {
+    // Perform Call(promiseCapability.[[Resolve]], undefined, «iteratorResult»).
+    CallBuiltin(Builtins::kResolvePromise, context, promise, iter_result);
+    Goto(&return_promise);
+  }
 
   // Per spec, AsyncGeneratorResolve() returns undefined. However, for the
   // benefit of %TraceExit(), return the Promise.
+  BIND(&return_promise);
   Return(promise);
 }
 
diff --git a/src/builtins/builtins-promise-gen.cc b/src/builtins/builtins-promise-gen.cc
@@ -1661,21 +1661,23 @@ TF_BUILTIN(ResolvePromise, PromiseBuiltinsAssembler) {
 
   // 7. If Type(resolution) is not Object, then
   GotoIf(TaggedIsSmi(resolution), &if_fulfill);
-  Node* const result_map = LoadMap(resolution);
-  GotoIfNot(IsJSReceiverMap(result_map), &if_fulfill);
+  Node* const resolution_map = LoadMap(resolution);
+  GotoIfNot(IsJSReceiverMap(resolution_map), &if_fulfill);
 
   // We can skip the "then" lookup on {resolution} if its [[Prototype]]
   // is the (initial) Promise.prototype and the Promise#then protector
   // is intact, as that guards the lookup path for the "then" property
   // on JSPromise instances which have the (initial) %PromisePrototype%.
-  Label if_fast(this), if_slow(this, Label::kDeferred);
+  Label if_fast(this), if_generic(this), if_slow(this, Label::kDeferred);
   Node* const native_context = LoadNativeContext(context);
-  BranchIfPromiseThenLookupChainIntact(native_context, result_map, &if_fast,
-                                       &if_slow);
+  GotoIfForceSlowPath(&if_slow);
+  GotoIf(IsPromiseThenProtectorCellInvalid(), &if_slow);
+  GotoIfNot(IsJSPromiseMap(resolution_map), &if_generic);
+  Node* const promise_prototype =
+      LoadContextElement(native_context, Context::PROMISE_PROTOTYPE_INDEX);
+  Branch(WordEqual(LoadMapPrototype(resolution_map), promise_prototype),
+         &if_fast, &if_slow);
 
-  // Resolution is a native promise and if it's already resolved or
-  // rejected, shortcircuit the resolution procedure by directly
-  // reusing the value from the promise.
   BIND(&if_fast);
   {
     Node* const then =
@@ -1684,6 +1686,21 @@ TF_BUILTIN(ResolvePromise, PromiseBuiltinsAssembler) {
     Goto(&do_enqueue);
   }
 
+  BIND(&if_generic);
+  {
+    // We can skip the lookup of "then" if the {resolution} is a (newly
+    // created) IterResultObject, as the Promise#then protector also
+    // ensures that the intrinsic %ObjectPrototype% doesn't contain any
+    // "then" property. This helps to avoid negative lookups on iterator
+    // results from async generators.
+    CSA_ASSERT(this, IsJSReceiverMap(resolution_map));
+    CSA_ASSERT(this, Word32BinaryNot(IsPromiseThenProtectorCellInvalid()));
+    Node* const iterator_result_map =
+        LoadContextElement(native_context, Context::ITERATOR_RESULT_MAP_INDEX);
+    Branch(WordEqual(resolution_map, iterator_result_map), &if_fulfill,
+           &if_slow);
+  }
+
   BIND(&if_slow);
   {
     // 8. Let then be Get(resolution, "then").
diff --git a/src/isolate.h b/src/isolate.h
@@ -1095,7 +1095,10 @@ class Isolate {
   bool IsPromiseResolveLookupChainIntact();
 
   // Make sure a lookup of "then" on any JSPromise whose [[Prototype]] is the
-  // initial %PromisePrototype% yields the initial method.
+  // initial %PromisePrototype% yields the initial method. In addition this
+  // protector also guards the negative lookup of "then" on the intrinsic
+  // %ObjectPrototype%, meaning that such lookups are guaranteed to yield
+  // undefined without triggering any side-effects.
   bool IsPromiseThenLookupChainIntact();
   bool IsPromiseThenLookupChainIntact(Handle<JSReceiver> receiver);
 
diff --git a/src/lookup.cc b/src/lookup.cc
@@ -324,7 +324,14 @@ void LookupIterator::InternalUpdateProtector() {
     if (!isolate_->IsPromiseThenLookupChainIntact()) return;
     // Setting the "then" property on any JSPromise instance or on the
     // initial %PromisePrototype% invalidates the Promise#then protector.
+    // Also setting the "then" property on the initial %ObjectPrototype%
+    // invalidates the Promise#then protector, since we use this protector
+    // to guard the fast-path in AsyncGeneratorResolve, where we can skip
+    // the ResolvePromise step and go directly to FulfillPromise if we
+    // know that the Object.prototype doesn't contain a "then" method.
     if (holder_->IsJSPromise() ||
+        isolate_->IsInAnyContext(*holder_,
+                                 Context::INITIAL_OBJECT_PROTOTYPE_INDEX) ||
         isolate_->IsInAnyContext(*holder_, Context::PROMISE_PROTOTYPE_INDEX)) {
       isolate_->InvalidatePromiseThenProtector();
     }
