Skip to content

Commit 91ddb65

Browse files
MayaLekovaCommit Bot
MayaLekova
authored and
Commit Bot
committed
Revert promises optimizations due to regressions in async hooks
Revert "[async-await] Eliminate throwaway promise in async functions." This reverts commit a840f1f. Revert "[async-generators] Also avoid throwaway promise here." This reverts commit feb545c. Revert "[async-await] Turn await closures into intrinsics." This reverts commit d97bb31. Revert "[async-generators] Add fast-path for primitives in AsyncGeneratorYield." This reverts commit e57b500. Revert "[async-generators] Add fast-path to skip "then" lookup in AsyncGeneratorResolve." This reverts commit c15802e. Revert "[promises] Correctly run before/after hooks for await." This reverts commit ca76392. Bug: v8:7253, v8:7745 Change-Id: I25ad0d2df3cfbc84dbb431aa25b268bce8a39e89 Reviewed-on: https://chromium-review.googlesource.com/1049975 Commit-Queue: Maya Lekova <mslekova@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#53139}
1 parent 989285b commit 91ddb65

36 files changed

Lines changed: 703 additions & 621 deletions

‎src/bootstrapper.cc‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,6 +1587,50 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
15871587
native_context()->set_async_iterator_value_unwrap_shared_fun(*info);
15881588
}
15891589

1590+
{ // --- A s y n c G e n e r a t o r ---
1591+
Handle<JSFunction> await_caught =
1592+
SimpleCreateFunction(isolate, factory->empty_string(),
1593+
Builtins::kAsyncGeneratorAwaitCaught, 1, false);
1594+
native_context()->set_async_generator_await_caught(*await_caught);
1595+
1596+
Handle<JSFunction> await_uncaught =
1597+
SimpleCreateFunction(isolate, factory->empty_string(),
1598+
Builtins::kAsyncGeneratorAwaitUncaught, 1, false);
1599+
native_context()->set_async_generator_await_uncaught(*await_uncaught);
1600+
1601+
Handle<SharedFunctionInfo> info = SimpleCreateSharedFunctionInfo(
1602+
isolate, Builtins::kAsyncGeneratorAwaitResolveClosure,
1603+
factory->empty_string(), 1);
1604+
native_context()->set_async_generator_await_resolve_shared_fun(*info);
1605+
1606+
info = SimpleCreateSharedFunctionInfo(
1607+
isolate, Builtins::kAsyncGeneratorAwaitRejectClosure,
1608+
factory->empty_string(), 1);
1609+
native_context()->set_async_generator_await_reject_shared_fun(*info);
1610+
1611+
info = SimpleCreateSharedFunctionInfo(
1612+
isolate, Builtins::kAsyncGeneratorYieldResolveClosure,
1613+
factory->empty_string(), 1);
1614+
native_context()->set_async_generator_yield_resolve_shared_fun(*info);
1615+
1616+
info = SimpleCreateSharedFunctionInfo(
1617+
isolate, Builtins::kAsyncGeneratorReturnResolveClosure,
1618+
factory->empty_string(), 1);
1619+
native_context()->set_async_generator_return_resolve_shared_fun(*info);
1620+
1621+
info = SimpleCreateSharedFunctionInfo(
1622+
isolate, Builtins::kAsyncGeneratorReturnClosedResolveClosure,
1623+
factory->empty_string(), 1);
1624+
native_context()->set_async_generator_return_closed_resolve_shared_fun(
1625+
*info);
1626+
1627+
info = SimpleCreateSharedFunctionInfo(
1628+
isolate, Builtins::kAsyncGeneratorReturnClosedRejectClosure,
1629+
factory->empty_string(), 1);
1630+
native_context()->set_async_generator_return_closed_reject_shared_fun(
1631+
*info);
1632+
}
1633+
15901634
{ // --- A r r a y ---
15911635
Handle<JSFunction> array_function = InstallFunction(
15921636
global, "Array", JS_ARRAY_TYPE, JSArray::kSize, 0,
@@ -4025,6 +4069,34 @@ void Bootstrapper::ExportFromRuntime(Isolate* isolate,
40254069
JSFunction::SetPrototype(async_function_constructor,
40264070
async_function_prototype);
40274071

4072+
{
4073+
Handle<JSFunction> function =
4074+
SimpleCreateFunction(isolate, factory->empty_string(),
4075+
Builtins::kAsyncFunctionAwaitCaught, 2, false);
4076+
native_context->set_async_function_await_caught(*function);
4077+
}
4078+
4079+
{
4080+
Handle<JSFunction> function =
4081+
SimpleCreateFunction(isolate, factory->empty_string(),
4082+
Builtins::kAsyncFunctionAwaitUncaught, 2, false);
4083+
native_context->set_async_function_await_uncaught(*function);
4084+
}
4085+
4086+
{
4087+
Handle<SharedFunctionInfo> info = SimpleCreateSharedFunctionInfo(
4088+
isolate, Builtins::kAsyncFunctionAwaitRejectClosure,
4089+
factory->empty_string(), 1);
4090+
native_context->set_async_function_await_reject_shared_fun(*info);
4091+
}
4092+
4093+
{
4094+
Handle<SharedFunctionInfo> info = SimpleCreateSharedFunctionInfo(
4095+
isolate, Builtins::kAsyncFunctionAwaitResolveClosure,
4096+
factory->empty_string(), 1);
4097+
native_context->set_async_function_await_resolve_shared_fun(*info);
4098+
}
4099+
40284100
{
40294101
Handle<JSFunction> function =
40304102
SimpleCreateFunction(isolate, factory->empty_string(),

‎src/builtins/builtins-async-function-gen.cc‎

Lines changed: 69 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,37 @@ class AsyncFunctionBuiltinsAssembler : public AsyncBuiltinsAssembler {
2121
Node* const awaited, Node* const outer_promise,
2222
const bool is_predicted_as_caught);
2323

24-
void AsyncFunctionAwaitResume(Node* const context, Node* const argument,
25-
Node* const generator,
26-
JSGeneratorObject::ResumeMode resume_mode);
24+
void AsyncFunctionAwaitResumeClosure(
25+
Node* const context, Node* const sent_value,
26+
JSGeneratorObject::ResumeMode resume_mode);
2727
};
2828

29-
void AsyncFunctionBuiltinsAssembler::AsyncFunctionAwaitResume(
30-
Node* const context, Node* const argument, Node* const generator,
29+
namespace {
30+
31+
// Describe fields of Context associated with AsyncFunctionAwait resume
32+
// closures.
33+
// TODO(jgruber): Refactor to reuse code for upcoming async-generators.
34+
class AwaitContext {
35+
public:
36+
enum Fields { kGeneratorSlot = Context::MIN_CONTEXT_SLOTS, kLength };
37+
};
38+
39+
} // anonymous namespace
40+
41+
void AsyncFunctionBuiltinsAssembler::AsyncFunctionAwaitResumeClosure(
42+
Node* context, Node* sent_value,
3143
JSGeneratorObject::ResumeMode resume_mode) {
32-
CSA_ASSERT(this, IsJSGeneratorObject(generator));
3344
DCHECK(resume_mode == JSGeneratorObject::kNext ||
3445
resume_mode == JSGeneratorObject::kThrow);
3546

47+
Node* const generator =
48+
LoadContextElement(context, AwaitContext::kGeneratorSlot);
49+
CSA_SLOW_ASSERT(this, HasInstanceType(generator, JS_GENERATOR_OBJECT_TYPE));
50+
51+
// Inline version of GeneratorPrototypeNext / GeneratorPrototypeReturn with
52+
// unnecessary runtime checks removed.
53+
// TODO(jgruber): Refactor to reuse code from builtins-generator.cc.
54+
3655
// Ensure that the generator is neither closed nor running.
3756
CSA_SLOW_ASSERT(
3857
this,
@@ -47,23 +66,31 @@ void AsyncFunctionBuiltinsAssembler::AsyncFunctionAwaitResume(
4766

4867
// Resume the {receiver} using our trampoline.
4968
Callable callable = CodeFactory::ResumeGenerator(isolate());
50-
TailCallStub(callable, context, argument, generator);
69+
CallStub(callable, context, sent_value, generator);
70+
71+
// The resulting Promise is a throwaway, so it doesn't matter what it
72+
// resolves to. What is important is that we don't end up keeping the
73+
// whole chain of intermediate Promises alive by returning the return value
74+
// of ResumeGenerator, as that would create a memory leak.
5175
}
5276

53-
TF_BUILTIN(AsyncFunctionAwaitFulfill, AsyncFunctionBuiltinsAssembler) {
54-
Node* const argument = Parameter(Descriptor::kArgument);
55-
Node* const generator = Parameter(Descriptor::kGenerator);
77+
TF_BUILTIN(AsyncFunctionAwaitRejectClosure, AsyncFunctionBuiltinsAssembler) {
78+
CSA_ASSERT_JS_ARGC_EQ(this, 1);
79+
Node* const sentError = Parameter(Descriptor::kSentError);
5680
Node* const context = Parameter(Descriptor::kContext);
57-
AsyncFunctionAwaitResume(context, argument, generator,
58-
JSGeneratorObject::kNext);
81+
82+
AsyncFunctionAwaitResumeClosure(context, sentError,
83+
JSGeneratorObject::kThrow);
84+
Return(UndefinedConstant());
5985
}
6086

61-
TF_BUILTIN(AsyncFunctionAwaitReject, AsyncFunctionBuiltinsAssembler) {
62-
Node* const argument = Parameter(Descriptor::kArgument);
63-
Node* const generator = Parameter(Descriptor::kGenerator);
87+
TF_BUILTIN(AsyncFunctionAwaitResolveClosure, AsyncFunctionBuiltinsAssembler) {
88+
CSA_ASSERT_JS_ARGC_EQ(this, 1);
89+
Node* const sentValue = Parameter(Descriptor::kSentValue);
6490
Node* const context = Parameter(Descriptor::kContext);
65-
AsyncFunctionAwaitResume(context, argument, generator,
66-
JSGeneratorObject::kThrow);
91+
92+
AsyncFunctionAwaitResumeClosure(context, sentValue, JSGeneratorObject::kNext);
93+
Return(UndefinedConstant());
6794
}
6895

6996
// ES#abstract-ops-async-function-await
@@ -78,12 +105,25 @@ TF_BUILTIN(AsyncFunctionAwaitReject, AsyncFunctionBuiltinsAssembler) {
78105
void AsyncFunctionBuiltinsAssembler::AsyncFunctionAwait(
79106
Node* const context, Node* const generator, Node* const awaited,
80107
Node* const outer_promise, const bool is_predicted_as_caught) {
81-
CSA_SLOW_ASSERT(this, IsJSGeneratorObject(generator));
82-
CSA_SLOW_ASSERT(this, IsJSPromise(outer_promise));
83-
84-
Await(context, generator, awaited, outer_promise,
85-
Builtins::kAsyncFunctionAwaitFulfill,
86-
Builtins::kAsyncFunctionAwaitReject, is_predicted_as_caught);
108+
CSA_SLOW_ASSERT(this, HasInstanceType(generator, JS_GENERATOR_OBJECT_TYPE));
109+
CSA_SLOW_ASSERT(this, HasInstanceType(outer_promise, JS_PROMISE_TYPE));
110+
111+
ContextInitializer init_closure_context = [&](Node* context) {
112+
StoreContextElementNoWriteBarrier(context, AwaitContext::kGeneratorSlot,
113+
generator);
114+
};
115+
116+
// TODO(jgruber): AsyncBuiltinsAssembler::Await currently does not reuse
117+
// the awaited promise if it is already a promise. Reuse is non-spec compliant
118+
// but part of our old behavior gives us a couple of percent
119+
// performance boost.
120+
// TODO(jgruber): Use a faster specialized version of
121+
// InternalPerformPromiseThen.
122+
123+
Await(context, generator, awaited, outer_promise, AwaitContext::kLength,
124+
init_closure_context, Context::ASYNC_FUNCTION_AWAIT_RESOLVE_SHARED_FUN,
125+
Context::ASYNC_FUNCTION_AWAIT_REJECT_SHARED_FUN,
126+
is_predicted_as_caught);
87127

88128
// Return outer promise to avoid adding an load of the outer promise before
89129
// suspending in BytecodeGenerator.
@@ -93,28 +133,30 @@ void AsyncFunctionBuiltinsAssembler::AsyncFunctionAwait(
93133
// Called by the parser from the desugaring of 'await' when catch
94134
// prediction indicates that there is a locally surrounding catch block.
95135
TF_BUILTIN(AsyncFunctionAwaitCaught, AsyncFunctionBuiltinsAssembler) {
136+
CSA_ASSERT_JS_ARGC_EQ(this, 3);
96137
Node* const generator = Parameter(Descriptor::kGenerator);
97-
Node* const value = Parameter(Descriptor::kValue);
138+
Node* const awaited = Parameter(Descriptor::kAwaited);
98139
Node* const outer_promise = Parameter(Descriptor::kOuterPromise);
99140
Node* const context = Parameter(Descriptor::kContext);
100141

101142
static const bool kIsPredictedAsCaught = true;
102143

103-
AsyncFunctionAwait(context, generator, value, outer_promise,
144+
AsyncFunctionAwait(context, generator, awaited, outer_promise,
104145
kIsPredictedAsCaught);
105146
}
106147

107148
// Called by the parser from the desugaring of 'await' when catch
108149
// prediction indicates no locally surrounding catch block.
109150
TF_BUILTIN(AsyncFunctionAwaitUncaught, AsyncFunctionBuiltinsAssembler) {
151+
CSA_ASSERT_JS_ARGC_EQ(this, 3);
110152
Node* const generator = Parameter(Descriptor::kGenerator);
111-
Node* const value = Parameter(Descriptor::kValue);
153+
Node* const awaited = Parameter(Descriptor::kAwaited);
112154
Node* const outer_promise = Parameter(Descriptor::kOuterPromise);
113155
Node* const context = Parameter(Descriptor::kContext);
114156

115157
static const bool kIsPredictedAsCaught = false;
116158

117-
AsyncFunctionAwait(context, generator, value, outer_promise,
159+
AsyncFunctionAwait(context, generator, awaited, outer_promise,
118160
kIsPredictedAsCaught);
119161
}
120162

0 commit comments

Comments
 (0)