Skip to content

Commit f054729

Browse files
rmahdavV8 LUCI CQ
rmahdav
authored and
V8 LUCI CQ
committed
[explicit-resource-management] Avoid unnecessary awaits
This CL reduces unnecessary Awaits for nullish values in blocks containing `await using` and in `AsyncDisposableStack` tc39/proposal-explicit-resource-management#219 Bug: 42203814 Change-Id: Ia94755bd19a217512d308b5b4f524471b8321d63 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5819755 Reviewed-by: Shu-yu Guo <[email protected]> Commit-Queue: Rezvan Mahdavi Hezaveh <[email protected]> Cr-Commit-Position: refs/heads/main@{#95982}
1 parent f432c72 commit f054729

10 files changed

Lines changed: 221 additions & 51 deletions

‎src/builtins/promise-abstract-operations.tq‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,18 @@ transitioning macro PerformPromiseThenImpl(
488488
promise.SetHasHandler();
489489
}
490490

491+
transitioning javascript builtin PerformPromiseThenFunction(
492+
js-implicit context: NativeContext, receiver: JSAny)(onFulfilled: JSAny,
493+
onRejected: JSAny): JSAny {
494+
const jsPromise = Cast<JSPromise>(receiver) otherwise unreachable;
495+
const callableOnFulfilled = Cast<Callable>(onFulfilled) otherwise unreachable;
496+
const callableOnRejected = Cast<Callable>(onRejected) otherwise unreachable;
497+
498+
PerformPromiseThenImpl(
499+
jsPromise, callableOnFulfilled, callableOnRejected, Undefined);
500+
return Undefined;
501+
}
502+
491503
// https://tc39.es/ecma262/#sec-performpromisethen
492504
transitioning builtin PerformPromiseThen(
493505
implicit context: Context)(promise: JSPromise,

‎src/diagnostics/objects-printer.cc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1977,6 +1977,8 @@ void JSDisposableStackBase::JSDisposableStackBasePrint(std::ostream& os) {
19771977
os << "\n - stack: " << Brief(stack());
19781978
os << "\n - length: " << length();
19791979
os << "\n - state: " << state();
1980+
os << "\n - needsAwait: " << needsAwait();
1981+
os << "\n - hasAwaited: " << hasAwaited();
19801982
os << "\n - error: " << error();
19811983
JSObjectPrintBody(os, *this);
19821984
}
@@ -1986,6 +1988,8 @@ void JSAsyncDisposableStack::JSAsyncDisposableStackPrint(std::ostream& os) {
19861988
os << "\n - stack: " << Brief(stack());
19871989
os << "\n - length: " << length();
19881990
os << "\n - state: " << state();
1991+
os << "\n - needsAwait: " << needsAwait();
1992+
os << "\n - hasAwaited: " << hasAwaited();
19891993
os << "\n - error: " << error();
19901994
JSObjectPrintBody(os, *this);
19911995
}

‎src/init/bootstrapper.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3272,6 +3272,12 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
32723272
isolate_, prototype, "then", Builtin::kPromisePrototypeThen, 2, kAdapt);
32733273
native_context()->set_promise_then(*promise_then);
32743274

3275+
DirectHandle<JSFunction> perform_promise_then =
3276+
InstallFunctionWithBuiltinId(isolate_, prototype, "performPromiseThen",
3277+
Builtin::kPerformPromiseThenFunction, 2,
3278+
kAdapt);
3279+
native_context()->set_perform_promise_then(*perform_promise_then);
3280+
32753281
InstallFunctionWithBuiltinId(isolate_, prototype, "catch",
32763282
Builtin::kPromisePrototypeCatch, 1, kAdapt);
32773283

‎src/interpreter/bytecode-generator.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2732,7 +2732,7 @@ void BytecodeGenerator::BuildDisposeScope(WrappedFunc wrapped_func,
27322732

27332733
builder()
27342734
->StoreAccumulatorInRegister(result_register)
2735-
.LoadUndefined()
2735+
.LoadTrue()
27362736
.CompareReference(result_register);
27372737

27382738
loop_builder.BreakIfTrue(ToBooleanMode::kConvertToBoolean);

‎src/objects/contexts.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ enum ContextLookupFlags {
5454
async_module_evaluate_internal) \
5555
V(REFLECT_APPLY_INDEX, JSFunction, reflect_apply) \
5656
V(REFLECT_CONSTRUCT_INDEX, JSFunction, reflect_construct) \
57+
V(PERFORM_PROMISE_THEN_INDEX, JSFunction, perform_promise_then) \
5758
V(PROMISE_THEN_INDEX, JSFunction, promise_then) \
5859
V(PROMISE_RESOLVE_INDEX, JSFunction, promise_resolve) \
5960
V(FUNCTION_PROTOTYPE_APPLY_INDEX, JSFunction, function_prototype_apply) \

‎src/objects/js-disposable-stack.cc‎

Lines changed: 89 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -70,65 +70,93 @@ MaybeHandle<Object> JSDisposableStackBase::DisposeResources(
7070
DisposeCallTypeBit::decode(stack_type_case);
7171
DisposeMethodHint hint = DisposeHintBit::decode(stack_type_case);
7272

73-
// (TODO:rezvan):
74-
// https://github.com/tc39/proposal-explicit-resource-management/pull/219
7573
// d. If hint is sync-dispose and needsAwait is true and hasAwaited is
7674
// false, then
7775
// i. Perform ! Await(undefined).
7876
// ii. Set needsAwait to false.
7977

80-
// e. If method is not undefined, then
81-
// i. Let result be Completion(Call(method, value)).
82-
// ii. If result is a normal completion and hint is async-dispose, then
83-
// 1. Set result to Completion(Await(result.[[Value]])).
84-
// 2. Set hasAwaited to true.
85-
// iii. If result is a throw completion, then
86-
// Else,
87-
// i. Assert: hint is async-dispose.
88-
// ii. Set needsAwait to true.
89-
// iii. NOTE: This can only indicate a case where either null or
90-
// undefined was the initialized value of an await using declaration.
91-
// 4. If needsAwait is true and hasAwaited is false, then
92-
// a. Perform ! Await(undefined).
93-
v8::TryCatch try_catch(reinterpret_cast<v8::Isolate*>(isolate));
94-
try_catch.SetVerbose(false);
95-
try_catch.SetCaptureMessage(false);
96-
97-
if (call_type == DisposeMethodCallType::kValueIsReceiver) {
98-
result = Execution::Call(isolate, method, value, 0, nullptr);
99-
} else if (call_type == DisposeMethodCallType::kValueIsArgument) {
100-
result = Execution::Call(isolate, method,
101-
ReadOnlyRoots(isolate).undefined_value_handle(),
102-
1, argv);
103-
}
78+
if (hint == DisposeMethodHint::kSyncDispose &&
79+
disposable_stack->needsAwait() == true &&
80+
disposable_stack->hasAwaited() == false) {
81+
// i. Perform ! Await(undefined).
82+
// ii. Set needsAwait to false.
83+
disposable_stack->set_needsAwait(false);
10484

105-
Handle<Object> result_handle;
85+
return ResolveAPromiseWithValueAndReturnIt(
86+
isolate, ReadOnlyRoots(isolate).undefined_value_handle());
87+
}
10688

107-
if (result.ToHandle(&result_handle)) {
108-
if (hint == DisposeMethodHint::kAsyncDispose) {
109-
DCHECK_NE(resources_type, DisposableStackResourcesType::kAllSync);
110-
disposable_stack->set_length(length);
89+
// e. If method is not undefined, then
90+
if (!IsUndefined(*method)) {
91+
// i. Let result be Completion(Call(method, value)).
92+
v8::TryCatch try_catch(reinterpret_cast<v8::Isolate*>(isolate));
93+
try_catch.SetVerbose(false);
94+
try_catch.SetCaptureMessage(false);
95+
96+
if (call_type == DisposeMethodCallType::kValueIsReceiver) {
97+
result = Execution::Call(isolate, method, value, 0, nullptr);
98+
} else if (call_type == DisposeMethodCallType::kValueIsArgument) {
99+
result = Execution::Call(
100+
isolate, method, ReadOnlyRoots(isolate).undefined_value_handle(), 1,
101+
argv);
102+
}
111103

112-
Handle<JSFunction> promise_function = isolate->promise_function();
113-
Handle<Object> argv[] = {result_handle};
114-
Handle<Object> resolve_result =
115-
Execution::CallBuiltin(isolate, isolate->promise_resolve(),
116-
promise_function, arraysize(argv), argv)
117-
.ToHandleChecked();
118-
return Cast<JSReceiver>(resolve_result);
104+
Handle<Object> result_handle;
105+
// ii. If result is a normal completion and hint is async-dispose, then
106+
// 1. Set result to Completion(Await(result.[[Value]])).
107+
// 2. Set hasAwaited to true.
108+
if (result.ToHandle(&result_handle)) {
109+
if (hint == DisposeMethodHint::kAsyncDispose) {
110+
DCHECK_NE(resources_type, DisposableStackResourcesType::kAllSync);
111+
disposable_stack->set_length(length);
112+
113+
disposable_stack->set_hasAwaited(true);
114+
115+
return ResolveAPromiseWithValueAndReturnIt(isolate, result_handle);
116+
}
117+
} else {
118+
// iii. If result is a throw completion, then
119+
// 1. If completion is a throw completion, then
120+
// a. Set result to result.[[Value]].
121+
// b. Let suppressed be completion.[[Value]].
122+
// c. Let error be a newly created SuppressedError object.
123+
// d. Perform CreateNonEnumerableDataPropertyOrThrow(error,
124+
// "error", result). e. Perform
125+
// CreateNonEnumerableDataPropertyOrThrow(error, "suppressed",
126+
// suppressed). f. Set completion to ThrowCompletion(error).
127+
// 2. Else,
128+
// a. Set completion to result.
129+
DCHECK(isolate->has_exception());
130+
DCHECK(try_catch.HasCaught());
131+
Handle<Object> current_error(isolate->exception(), isolate);
132+
if (!isolate->is_catchable_by_javascript(*current_error)) {
133+
return {};
134+
}
135+
HandleErrorInDisposal(isolate, disposable_stack, current_error);
119136
}
120137
} else {
121-
// b. If result is a throw completion, then
122-
DCHECK(isolate->has_exception());
123-
DCHECK(try_catch.HasCaught());
124-
Handle<Object> current_error(isolate->exception(), isolate);
125-
if (!isolate->is_catchable_by_javascript(*current_error)) {
126-
return {};
127-
}
128-
HandleErrorInDisposal(isolate, disposable_stack, current_error);
138+
// Else,
139+
// i. Assert: hint is async-dispose.
140+
DCHECK_EQ(hint, DisposeMethodHint::kAsyncDispose);
141+
// ii. Set needsAwait to true.
142+
// iii. NOTE: This can only indicate a case where either null or
143+
// undefined was the initialized value of an await using declaration.
144+
disposable_stack->set_length(length);
145+
disposable_stack->set_needsAwait(true);
129146
}
130147
}
131148

149+
// 4. If needsAwait is true and hasAwaited is false, then
150+
// a. Perform ! Await(undefined).
151+
if (disposable_stack->needsAwait() == true &&
152+
disposable_stack->hasAwaited() == false) {
153+
disposable_stack->set_length(length);
154+
disposable_stack->set_hasAwaited(true);
155+
156+
return ResolveAPromiseWithValueAndReturnIt(
157+
isolate, ReadOnlyRoots(isolate).undefined_value_handle());
158+
}
159+
132160
// 5. NOTE: After disposeCapability has been disposed, it will never be used
133161
// again. The contents of disposeCapability.[[DisposableResourceStack]] can be
134162
// discarded in implementations, such as by garbage collection, at this point.
@@ -146,7 +174,18 @@ MaybeHandle<Object> JSDisposableStackBase::DisposeResources(
146174
isolate->Throw(*existing_error_handle);
147175
return MaybeHandle<Object>();
148176
}
149-
return isolate->factory()->undefined_value();
177+
return isolate->factory()->true_value();
178+
}
179+
180+
Handle<JSReceiver> JSDisposableStackBase::ResolveAPromiseWithValueAndReturnIt(
181+
Isolate* isolate, Handle<Object> value) {
182+
Handle<JSFunction> promise_function = isolate->promise_function();
183+
Handle<Object> argv[] = {value};
184+
Handle<Object> resolve_result =
185+
Execution::CallBuiltin(isolate, isolate->promise_resolve(),
186+
promise_function, arraysize(argv), argv)
187+
.ToHandleChecked();
188+
return Cast<JSReceiver>(resolve_result);
150189
}
151190

152191
Maybe<bool> JSAsyncDisposableStack::NextDisposeAsyncIteration(
@@ -169,7 +208,7 @@ Maybe<bool> JSAsyncDisposableStack::NextDisposeAsyncIteration(
169208
Handle<Object> result_handle;
170209

171210
if (result.ToHandle(&result_handle)) {
172-
if (!IsUndefined(*result_handle)) {
211+
if (!IsTrue(*result_handle)) {
173212
Handle<Context> async_disposable_stack_context =
174213
isolate->factory()->NewBuiltinContext(
175214
isolate->native_context(),
@@ -201,8 +240,8 @@ Maybe<bool> JSAsyncDisposableStack::NextDisposeAsyncIteration(
201240
.Build();
202241

203242
Handle<Object> argv[] = {on_fulfilled, on_rejected};
204-
// (TODO:rezvan): Add a wrapper function for PerformPromiseThen.
205-
Execution::CallBuiltin(isolate, isolate->promise_then(),
243+
244+
Execution::CallBuiltin(isolate, isolate->perform_promise_then(),
206245
Cast<JSPromise>(result_handle), arraysize(argv),
207246
argv)
208247
.ToHandleChecked();

‎src/objects/js-disposable-stack.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ class JSDisposableStackBase
8080
Isolate* isolate, DirectHandle<JSDisposableStackBase> disposable_stack,
8181
MaybeHandle<Object> maybe_continuation_error,
8282
DisposableStackResourcesType resources_type);
83+
static Handle<JSReceiver> ResolveAPromiseWithValueAndReturnIt(
84+
Isolate* isolate, Handle<Object> value);
8385
static void HandleErrorInDisposal(
8486
Isolate* isolate, DirectHandle<JSDisposableStackBase> disposable_stack,
8587
Handle<Object> current_error);

‎test/mjsunit/harmony/async-disposable-stack-with-null-undefined-tick-count.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright 2024 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
//
5+
// Flags: --js-explicit-resource-management
6+
7+
let values = [];
8+
9+
(async () => {
10+
for (let i = 0; i < 10; ++i) {
11+
values.push(i);
12+
await 0;
13+
}
14+
})();
15+
16+
async function TestCountTicks() {
17+
values.push(42);
18+
let stack = new AsyncDisposableStack();
19+
stack.use(null);
20+
stack.use(undefined);
21+
stack.use(null);
22+
stack.use(undefined);
23+
stack.use(null);
24+
stack.use(undefined);
25+
await stack.disposeAsync();
26+
values.push(43);
27+
}
28+
29+
async function RunTest() {
30+
await TestCountTicks();
31+
assertSame('0,42,1,2,43,3', values.join(','));
32+
}
33+
34+
RunTest();

‎test/mjsunit/harmony/await-using-with-sync-async-null-undefined-tick-count.js‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright 2024 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
//
5+
// Flags: --js-explicit-resource-management
6+
7+
let values = [];
8+
9+
(async () => {
10+
for (let i = 0; i < 10; ++i) {
11+
values.push(i);
12+
await 0;
13+
}
14+
})();
15+
16+
async function TestCountTicks() {
17+
using x = {
18+
value: 1,
19+
[Symbol.dispose]() {
20+
values.push(42);
21+
}
22+
};
23+
await using y = null;
24+
await using z = {
25+
value: 1,
26+
[Symbol.asyncDispose]() {
27+
values.push(43);
28+
}
29+
};
30+
await using w = undefined;
31+
values.push(44);
32+
}
33+
34+
async function RunTest() {
35+
await TestCountTicks();
36+
assertSame('0,44,43,1,42,2', values.join(','));
37+
}
38+
39+
RunTest();

‎test/mjsunit/harmony/await-using-with-sync-null-undefined-tick-count.js‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright 2024 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
//
5+
// Flags: --js-explicit-resource-management
6+
7+
let values = [];
8+
9+
(async () => {
10+
for (let i = 0; i < 10; ++i) {
11+
values.push(i);
12+
await 0;
13+
}
14+
})();
15+
16+
async function TestCountTicks() {
17+
using x = {
18+
value: 1,
19+
[Symbol.dispose]() {
20+
values.push(42);
21+
}
22+
};
23+
await using y = null;
24+
await using z = undefined;
25+
values.push(44);
26+
}
27+
28+
async function RunTest() {
29+
await TestCountTicks();
30+
assertSame('0,44,1,42,2', values.join(','));
31+
}
32+
33+
RunTest();

0 commit comments

Comments
 (0)