Commit 0f6eafe8 authored by Sathya Gunasekaran's avatar Sathya Gunasekaran Committed by Commit Bot

[promise] Remove incorrect fast path

Previously we would directly take the result from a fulfilled native
promise bypassing the microtask queue. This is observably different
from the spec.

Note: Our variant of the bluebird benchmark is heavily favored towards
fulfilled native promises because we don't use setTimeout (unlike the
original benchmark). I suspect this pattern doesn't appear often in
the wild so it's fine to take this hit for now.

PSA for Perf sheriffs: this is going to tank some benchmarks.

Bug: chromium:800651, v8:5691, v8:6007
Change-Id: Ic273bf2195529424b0d87359d28d5267060d5252
Reviewed-on: https://chromium-review.googlesource.com/895416
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51010}
parent b596ca4b
......@@ -587,14 +587,6 @@ void PromiseBuiltinsAssembler::InternalResolvePromise(Node* context,
// reusing the value from the promise.
BIND(&if_nativepromise);
{
Node* const thenable_status = PromiseStatus(result);
Node* const thenable_value =
LoadObjectField(result, JSPromise::kReactionsOrResultOffset);
Label if_isnotpending(this);
GotoIfNot(IsPromiseStatus(thenable_status, v8::Promise::kPending),
&if_isnotpending);
// TODO(gsathya): Use a marker here instead of the actual then
// callback, and check for the marker in PromiseResolveThenableJob
// and perform PromiseThen.
......@@ -602,39 +594,6 @@ void PromiseBuiltinsAssembler::InternalResolvePromise(Node* context,
LoadContextElement(native_context, Context::PROMISE_THEN_INDEX);
var_then.Bind(then);
Goto(&do_enqueue);
BIND(&if_isnotpending);
{
Label if_fulfilled(this), if_rejected(this);
Branch(IsPromiseStatus(thenable_status, v8::Promise::kFulfilled),
&if_fulfilled, &if_rejected);
BIND(&if_fulfilled);
{
PromiseFulfill(context, promise, thenable_value,
v8::Promise::kFulfilled);
PromiseSetHasHandler(promise);
Goto(&out);
}
BIND(&if_rejected);
{
Label reject(this);
Node* const has_handler = PromiseHasHandler(result);
// Promise has already been rejected, but had no handler.
// Revoke previously triggered reject event.
GotoIf(has_handler, &reject);
CallRuntime(Runtime::kPromiseRevokeReject, context, result);
Goto(&reject);
BIND(&reject);
// Don't cause a debug event as this case is forwarding a rejection.
InternalPromiseReject(context, promise, thenable_value, false);
PromiseSetHasHandler(result);
Goto(&out);
}
}
}
BIND(&if_notnativepromise);
......
// Copyright 2018 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.
//
// Flags: --allow-natives-syntax
var log = "";
var result;
Promise.resolve()
.then(() => log += "|turn1")
.then(() => log += "|turn2")
.then(() => log += "|turn3")
.then(() => log += "|turn4")
.then(() => result = "|start|turn1|fast-resolve|turn2|turn3|slow-resolve|turn4\n"+log)
.catch(e => print("ERROR", e));
Promise.resolve(Promise.resolve()).then(() => log += "|fast-resolve");
(class extends Promise {}).resolve(Promise.resolve()).then(() => log += "|slow-resolve");
log += "|start";
%RunMicrotasks();
assertEquals("|start|turn1|fast-resolve|turn2|turn3|slow-resolve|turn4\n\
|start|turn1|fast-resolve|turn2|turn3|slow-resolve|turn4", result);
// Copyright 2018 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.
//
// Flags: --allow-natives-syntax
var list = [];
function log(item) { list.push(item); }
async function f() {
try {
let namespace = await import(/a/);
} catch(e) {
log(1);
}
}
f();
async function g() {
try {
let namespace = await import({ get toString() { return undefined; }});
} catch(e) {
log(2);
}
}
g();
%RunMicrotasks();
assertEquals(list, [1,2]);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment