Skip to content

Commit 214e568

Browse files
Stephen BelangerMylesBorins
Stephen Belanger
authored and
MylesBorins
committed
deps: V8: backport c0fceaa0669b
Original commit message: Reland "[api] JSFunction PromiseHook for v8::Context" This is a reland of d5457f5fb7ea05ca05a697599ffa50d35c1ae3c7 after a speculative revert. Additionally it fixes an issue with throwing promise hooks. Original change's description: > [api] JSFunction PromiseHook for v8::Context > > This will enable Node.js to get much better performance from async_hooks > as currently PromiseHook delegates to C++ for the hook function and then > Node.js delegates it right back to JavaScript, introducing several > unnecessary barrier hops in code that gets called very, very frequently > in modern, promise-heavy applications. > > This API mirrors the form of the original C++ function based PromiseHook > API, however it is intentionally separate to allow it to use JSFunctions > triggered within generated code to, as much as possible, avoid entering > runtime functions entirely. > > Because PromiseHook has internal use also, beyond just the Node.js use, > I have opted to leave the existing API intact and keep this separate to > avoid conflicting with any possible behaviour expectations of other API > users. > > The design ideas for this new API stemmed from discussion with some V8 > team members at a previous Node.js Diagnostics Summit hosted by Google > in Munich, and the relevant documentation of the discussion can be found > here: https://docs.google.com/document/d/1g8OrG5lMIUhRn1zbkutgY83MiTSMx-0NHDs8Bf-nXxM/edit#heading=h.w1bavzz80l1e > > A summary of the reasons for why this new design is important can be > found here: https://docs.google.com/document/d/1vtgoT4_kjgOr-Bl605HR2T6_SC-C8uWzYaOPDK5pmRo/edit?usp=sharing > > Bug: v8:11025 > Change-Id: I0b403b00c37d3020b5af07b654b860659d3a7697 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2759188 > Reviewed-by: Marja Hölttä <[email protected]> > Reviewed-by: Camillo Bruni <[email protected]> > Reviewed-by: Anton Bikineev <[email protected]> > Reviewed-by: Igor Sheludko <[email protected]> > Commit-Queue: Camillo Bruni <[email protected]> > Cr-Commit-Position: refs/heads/master@{#73858} Bug: v8:11025 Bug: chromium:1197475 Change-Id: I73a71e97d9c3dff89a2b092c3fe4adff81ede8ef Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2823917 Reviewed-by: Marja Hölttä <[email protected]> Reviewed-by: Igor Sheludko <[email protected]> Reviewed-by: Anton Bikineev <[email protected]> Reviewed-by: Camillo Bruni <[email protected]> Commit-Queue: Camillo Bruni <[email protected]> Cr-Commit-Position: refs/heads/master@{#74071} Refs: v8/v8@c0fceaa PR-URL: #36394 Backport-PR-URL: #38577 Reviewed-By: Bryan English <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent f421422 commit 214e568

31 files changed

Lines changed: 772 additions & 132 deletions

‎common.gypi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
# Reset this number to 0 on major V8 upgrades.
3838
# Increment by one for each non-official patch applied to deps/v8.
39-
'v8_embedder_string': '-node.76',
39+
'v8_embedder_string': '-node.77',
4040

4141
##### V8 defaults for Node.js #####
4242

‎deps/v8/AUTHORS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ Seo Sanghyeon <[email protected]>
190190
Shawn Anastasio <[email protected]>
191191
Shawn Presser <[email protected]>
192192
Stefan Penner <[email protected]>
193+
Stephen Belanger <[email protected]>
193194
Sylvestre Ledru <[email protected]>
194195
Taketoshi Aono <[email protected]>
195196
Teddy Katz <[email protected]>

‎deps/v8/include/v8.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10488,6 +10488,18 @@ class V8_EXPORT Context {
1048810488
*/
1048910489
void SetContinuationPreservedEmbedderData(Local<Value> context);
1049010490

10491+
/**
10492+
* Set or clear hooks to be invoked for promise lifecycle operations.
10493+
* To clear a hook, set it to an empty v8::Function. Each function will
10494+
* receive the observed promise as the first argument. If a chaining
10495+
* operation is used on a promise, the init will additionally receive
10496+
* the parent promise as the second argument.
10497+
*/
10498+
void SetPromiseHooks(Local<Function> init_hook,
10499+
Local<Function> before_hook,
10500+
Local<Function> after_hook,
10501+
Local<Function> resolve_hook);
10502+
1049110503
/**
1049210504
* Stack-allocated class which sets the execution context for all
1049310505
* operations executed within a local scope.

‎deps/v8/src/api/api.cc‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6078,6 +6078,45 @@ void Context::SetContinuationPreservedEmbedderData(Local<Value> data) {
60786078
*i::Handle<i::HeapObject>::cast(Utils::OpenHandle(*data)));
60796079
}
60806080

6081+
void v8::Context::SetPromiseHooks(Local<Function> init_hook,
6082+
Local<Function> before_hook,
6083+
Local<Function> after_hook,
6084+
Local<Function> resolve_hook) {
6085+
i::Handle<i::Context> context = Utils::OpenHandle(this);
6086+
i::Isolate* isolate = context->GetIsolate();
6087+
6088+
i::Handle<i::Object> init = isolate->factory()->undefined_value();
6089+
i::Handle<i::Object> before = isolate->factory()->undefined_value();
6090+
i::Handle<i::Object> after = isolate->factory()->undefined_value();
6091+
i::Handle<i::Object> resolve = isolate->factory()->undefined_value();
6092+
6093+
bool has_hook = false;
6094+
6095+
if (!init_hook.IsEmpty()) {
6096+
init = Utils::OpenHandle(*init_hook);
6097+
has_hook = true;
6098+
}
6099+
if (!before_hook.IsEmpty()) {
6100+
before = Utils::OpenHandle(*before_hook);
6101+
has_hook = true;
6102+
}
6103+
if (!after_hook.IsEmpty()) {
6104+
after = Utils::OpenHandle(*after_hook);
6105+
has_hook = true;
6106+
}
6107+
if (!resolve_hook.IsEmpty()) {
6108+
resolve = Utils::OpenHandle(*resolve_hook);
6109+
has_hook = true;
6110+
}
6111+
6112+
isolate->SetHasContextPromiseHooks(has_hook);
6113+
6114+
context->native_context().set_promise_hook_init_function(*init);
6115+
context->native_context().set_promise_hook_before_function(*before);
6116+
context->native_context().set_promise_hook_after_function(*after);
6117+
context->native_context().set_promise_hook_resolve_function(*resolve);
6118+
}
6119+
60816120
namespace {
60826121
i::Address* GetSerializedDataFromFixedArray(i::Isolate* isolate,
60836122
i::FixedArray list, size_t index) {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,14 @@ TF_BUILTIN(AsyncFunctionEnter, AsyncFunctionBuiltinsAssembler) {
157157
StoreObjectFieldNoWriteBarrier(
158158
async_function_object, JSAsyncFunctionObject::kPromiseOffset, promise);
159159

160+
RunContextPromiseHookInit(context, promise, UndefinedConstant());
161+
160162
// Fire promise hooks if enabled and push the Promise under construction
161163
// in an async function on the catch prediction stack to handle exceptions
162164
// thrown before the first await.
163165
Label if_instrumentation(this, Label::kDeferred),
164166
if_instrumentation_done(this);
165-
Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
167+
Branch(IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
166168
&if_instrumentation, &if_instrumentation_done);
167169
BIND(&if_instrumentation);
168170
{

‎deps/v8/src/builtins/builtins-async-gen.cc‎

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -99,18 +99,11 @@ TNode<Object> AsyncBuiltinsAssembler::AwaitOld(
9999

100100
TVARIABLE(HeapObject, var_throwaway, UndefinedConstant());
101101

102-
// Deal with PromiseHooks and debug support in the runtime. This
103-
// also allocates the throwaway promise, which is only needed in
104-
// case of PromiseHooks or debugging.
105-
Label if_debugging(this, Label::kDeferred), do_resolve_promise(this);
106-
Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
107-
&if_debugging, &do_resolve_promise);
108-
BIND(&if_debugging);
109-
var_throwaway =
110-
CAST(CallRuntime(Runtime::kAwaitPromisesInitOld, context, value, promise,
111-
outer_promise, on_reject, is_predicted_as_caught));
112-
Goto(&do_resolve_promise);
113-
BIND(&do_resolve_promise);
102+
RunContextPromiseHookInit(context, promise, outer_promise);
103+
104+
InitAwaitPromise(Runtime::kAwaitPromisesInitOld, context, value, promise,
105+
outer_promise, on_reject, is_predicted_as_caught,
106+
&var_throwaway);
114107

115108
// Perform ! Call(promiseCapability.[[Resolve]], undefined, « promise »).
116109
CallBuiltin(Builtins::kResolvePromise, context, promise, value);
@@ -170,21 +163,46 @@ TNode<Object> AsyncBuiltinsAssembler::AwaitOptimized(
170163

171164
TVARIABLE(HeapObject, var_throwaway, UndefinedConstant());
172165

166+
InitAwaitPromise(Runtime::kAwaitPromisesInit, context, promise, promise,
167+
outer_promise, on_reject, is_predicted_as_caught,
168+
&var_throwaway);
169+
170+
return CallBuiltin(Builtins::kPerformPromiseThen, native_context, promise,
171+
on_resolve, on_reject, var_throwaway.value());
172+
}
173+
174+
void AsyncBuiltinsAssembler::InitAwaitPromise(
175+
Runtime::FunctionId id, TNode<Context> context, TNode<Object> value,
176+
TNode<Object> promise, TNode<Object> outer_promise,
177+
TNode<HeapObject> on_reject, TNode<Oddball> is_predicted_as_caught,
178+
TVariable<HeapObject>* var_throwaway) {
173179
// Deal with PromiseHooks and debug support in the runtime. This
174180
// also allocates the throwaway promise, which is only needed in
175181
// case of PromiseHooks or debugging.
176-
Label if_debugging(this, Label::kDeferred), do_perform_promise_then(this);
177-
Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
178-
&if_debugging, &do_perform_promise_then);
182+
Label if_debugging(this, Label::kDeferred),
183+
if_promise_hook(this, Label::kDeferred),
184+
not_debugging(this),
185+
do_nothing(this);
186+
TNode<Uint32T> promiseHookFlags = PromiseHookFlags();
187+
Branch(IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(
188+
promiseHookFlags), &if_debugging, &not_debugging);
179189
BIND(&if_debugging);
180-
var_throwaway =
181-
CAST(CallRuntime(Runtime::kAwaitPromisesInit, context, promise, promise,
190+
*var_throwaway =
191+
CAST(CallRuntime(id, context, value, promise,
182192
outer_promise, on_reject, is_predicted_as_caught));
183-
Goto(&do_perform_promise_then);
184-
BIND(&do_perform_promise_then);
185-
186-
return CallBuiltin(Builtins::kPerformPromiseThen, native_context, promise,
187-
on_resolve, on_reject, var_throwaway.value());
193+
Goto(&do_nothing);
194+
BIND(&not_debugging);
195+
196+
// This call to NewJSPromise is to keep behaviour parity with what happens
197+
// in Runtime::kAwaitPromisesInit above if native hooks are set. It will
198+
// create a throwaway promise that will trigger an init event and will get
199+
// passed into Builtins::kPerformPromiseThen below.
200+
Branch(IsContextPromiseHookEnabled(promiseHookFlags), &if_promise_hook,
201+
&do_nothing);
202+
BIND(&if_promise_hook);
203+
*var_throwaway = NewJSPromise(context, promise);
204+
Goto(&do_nothing);
205+
BIND(&do_nothing);
188206
}
189207

190208
TNode<Object> AsyncBuiltinsAssembler::Await(

‎deps/v8/src/builtins/builtins-async-gen.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ class AsyncBuiltinsAssembler : public PromiseBuiltinsAssembler {
6262
TNode<SharedFunctionInfo> on_resolve_sfi,
6363
TNode<SharedFunctionInfo> on_reject_sfi,
6464
TNode<Oddball> is_predicted_as_caught);
65+
66+
void InitAwaitPromise(
67+
Runtime::FunctionId id, TNode<Context> context, TNode<Object> value,
68+
TNode<Object> promise, TNode<Object> outer_promise,
69+
TNode<HeapObject> on_reject, TNode<Oddball> is_predicted_as_caught,
70+
TVariable<HeapObject>* var_throwaway);
6571
};
6672

6773
} // namespace internal

‎deps/v8/src/builtins/builtins-async-generator-gen.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ TF_BUILTIN(AsyncGeneratorResolve, AsyncGeneratorBuiltinsAssembler) {
520520
// the "promiseResolve" hook would not be fired otherwise.
521521
Label if_fast(this), if_slow(this, Label::kDeferred), return_promise(this);
522522
GotoIfForceSlowPath(&if_slow);
523-
GotoIf(IsPromiseHookEnabled(), &if_slow);
523+
GotoIf(IsIsolatePromiseHookEnabledOrHasAsyncEventDelegate(), &if_slow);
524524
Branch(IsPromiseThenProtectorCellInvalid(), &if_slow, &if_fast);
525525

526526
BIND(&if_fast);

‎deps/v8/src/builtins/builtins-microtask-queue-gen.cc‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,11 @@ class MicrotaskQueueBuiltinsAssembler : public CodeStubAssembler {
4646
void EnterMicrotaskContext(TNode<Context> native_context);
4747
void RewindEnteredContext(TNode<IntPtrT> saved_entered_context_count);
4848

49+
void RunAllPromiseHooks(PromiseHookType type, TNode<Context> context,
50+
TNode<HeapObject> promise_or_capability);
4951
void RunPromiseHook(Runtime::FunctionId id, TNode<Context> context,
50-
TNode<HeapObject> promise_or_capability);
52+
TNode<HeapObject> promise_or_capability,
53+
TNode<Uint32T> promiseHookFlags);
5154
};
5255

5356
TNode<RawPtrT> MicrotaskQueueBuiltinsAssembler::GetMicrotaskQueue(
@@ -198,7 +201,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
198201
const TNode<Object> thenable = LoadObjectField(
199202
microtask, PromiseResolveThenableJobTask::kThenableOffset);
200203

201-
RunPromiseHook(Runtime::kPromiseHookBefore, microtask_context,
204+
RunAllPromiseHooks(PromiseHookType::kBefore, microtask_context,
202205
CAST(promise_to_resolve));
203206

204207
{
@@ -207,7 +210,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
207210
promise_to_resolve, thenable, then);
208211
}
209212

210-
RunPromiseHook(Runtime::kPromiseHookAfter, microtask_context,
213+
RunAllPromiseHooks(PromiseHookType::kAfter, microtask_context,
211214
CAST(promise_to_resolve));
212215

213216
RewindEnteredContext(saved_entered_context_count);
@@ -242,8 +245,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
242245
BIND(&preserved_data_done);
243246

244247
// Run the promise before/debug hook if enabled.
245-
RunPromiseHook(Runtime::kPromiseHookBefore, microtask_context,
246-
promise_or_capability);
248+
RunAllPromiseHooks(PromiseHookType::kBefore, microtask_context,
249+
promise_or_capability);
247250

248251
{
249252
ScopedExceptionHandler handler(this, &if_exception, &var_exception);
@@ -252,8 +255,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
252255
}
253256

254257
// Run the promise after/debug hook if enabled.
255-
RunPromiseHook(Runtime::kPromiseHookAfter, microtask_context,
256-
promise_or_capability);
258+
RunAllPromiseHooks(PromiseHookType::kAfter, microtask_context,
259+
promise_or_capability);
257260

258261
Label preserved_data_reset_done(this);
259262
GotoIf(IsUndefined(preserved_embedder_data), &preserved_data_reset_done);
@@ -295,8 +298,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
295298
BIND(&preserved_data_done);
296299

297300
// Run the promise before/debug hook if enabled.
298-
RunPromiseHook(Runtime::kPromiseHookBefore, microtask_context,
299-
promise_or_capability);
301+
RunAllPromiseHooks(PromiseHookType::kBefore, microtask_context,
302+
promise_or_capability);
300303

301304
{
302305
ScopedExceptionHandler handler(this, &if_exception, &var_exception);
@@ -305,8 +308,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
305308
}
306309

307310
// Run the promise after/debug hook if enabled.
308-
RunPromiseHook(Runtime::kPromiseHookAfter, microtask_context,
309-
promise_or_capability);
311+
RunAllPromiseHooks(PromiseHookType::kAfter, microtask_context,
312+
promise_or_capability);
310313

311314
Label preserved_data_reset_done(this);
312315
GotoIf(IsUndefined(preserved_embedder_data), &preserved_data_reset_done);
@@ -464,12 +467,43 @@ void MicrotaskQueueBuiltinsAssembler::RewindEnteredContext(
464467
saved_entered_context_count);
465468
}
466469

470+
void MicrotaskQueueBuiltinsAssembler::RunAllPromiseHooks(
471+
PromiseHookType type, TNode<Context> context,
472+
TNode<HeapObject> promise_or_capability) {
473+
Label hook(this, Label::kDeferred), done_hook(this);
474+
TNode<Uint32T> promiseHookFlags = PromiseHookFlags();
475+
Branch(IsAnyPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(
476+
promiseHookFlags), &hook, &done_hook);
477+
BIND(&hook);
478+
{
479+
switch (type) {
480+
case PromiseHookType::kBefore:
481+
RunContextPromiseHookBefore(context, promise_or_capability,
482+
promiseHookFlags);
483+
RunPromiseHook(Runtime::kPromiseHookBefore, context,
484+
promise_or_capability, promiseHookFlags);
485+
break;
486+
case PromiseHookType::kAfter:
487+
RunContextPromiseHookAfter(context, promise_or_capability,
488+
promiseHookFlags);
489+
RunPromiseHook(Runtime::kPromiseHookAfter, context,
490+
promise_or_capability, promiseHookFlags);
491+
break;
492+
default:
493+
UNREACHABLE();
494+
}
495+
Goto(&done_hook);
496+
}
497+
BIND(&done_hook);
498+
}
499+
467500
void MicrotaskQueueBuiltinsAssembler::RunPromiseHook(
468501
Runtime::FunctionId id, TNode<Context> context,
469-
TNode<HeapObject> promise_or_capability) {
502+
TNode<HeapObject> promise_or_capability,
503+
TNode<Uint32T> promiseHookFlags) {
470504
Label hook(this, Label::kDeferred), done_hook(this);
471-
Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(), &hook,
472-
&done_hook);
505+
Branch(IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(
506+
promiseHookFlags), &hook, &done_hook);
473507
BIND(&hook);
474508
{
475509
// Get to the underlying JSPromise instance.

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,8 @@ FulfillPromise(implicit context: Context)(
186186
// Assert: The value of promise.[[PromiseState]] is "pending".
187187
assert(promise.Status() == PromiseState::kPending);
188188

189+
RunContextPromiseHookResolve(promise);
190+
189191
// 2. Let reactions be promise.[[PromiseFulfillReactions]].
190192
const reactions =
191193
UnsafeCast<(Zero | PromiseReaction)>(promise.reactions_or_result);
@@ -204,17 +206,24 @@ FulfillPromise(implicit context: Context)(
204206
}
205207

206208
extern macro PromiseBuiltinsAssembler::
207-
IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(): bool;
209+
IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(): bool;
210+
211+
extern macro PromiseBuiltinsAssembler::
212+
IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(uint32):
213+
bool;
208214

209215
// https://tc39.es/ecma262/#sec-rejectpromise
210216
transitioning builtin
211217
RejectPromise(implicit context: Context)(
212218
promise: JSPromise, reason: JSAny, debugEvent: Boolean): JSAny {
219+
const promiseHookFlags = PromiseHookFlags();
220+
213221
// If promise hook is enabled or the debugger is active, let
214222
// the runtime handle this operation, which greatly reduces
215223
// the complexity here and also avoids a couple of back and
216224
// forth between JavaScript and C++ land.
217-
if (IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate() ||
225+
if (IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(
226+
promiseHookFlags) ||
218227
!promise.HasHandler()) {
219228
// 7. If promise.[[PromiseIsHandled]] is false, perform
220229
// HostPromiseRejectionTracker(promise, "reject").
@@ -223,6 +232,8 @@ RejectPromise(implicit context: Context)(
223232
return runtime::RejectPromise(promise, reason, debugEvent);
224233
}
225234

235+
RunContextPromiseHookResolve(promise, promiseHookFlags);
236+
226237
// 2. Let reactions be promise.[[PromiseRejectReactions]].
227238
const reactions =
228239
UnsafeCast<(Zero | PromiseReaction)>(promise.reactions_or_result);

0 commit comments

Comments
 (0)