Skip to content

Commit 827ae05

Browse files
Stephen Belangertargos
Stephen Belanger
authored and
targos
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 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 95db7d5 commit 827ae05

31 files changed

+781
-130
lines changed

‎deps/v8/AUTHORS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ Seo Sanghyeon <[email protected]>
209209
Shawn Anastasio <[email protected]>
210210
Shawn Presser <[email protected]>
211211
Stefan Penner <[email protected]>
212+
Stephen Belanger <[email protected]>
212213
Sylvestre Ledru <[email protected]>
213214
Taketoshi Aono <[email protected]>
214215
Tao Liqiang <[email protected]>

‎deps/v8/include/v8.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10794,6 +10794,18 @@ class V8_EXPORT Context : public Data {
1079410794
*/
1079510795
void SetContinuationPreservedEmbedderData(Local<Value> context);
1079610796

10797+
/**
10798+
* Set or clear hooks to be invoked for promise lifecycle operations.
10799+
* To clear a hook, set it to an empty v8::Function. Each function will
10800+
* receive the observed promise as the first argument. If a chaining
10801+
* operation is used on a promise, the init will additionally receive
10802+
* the parent promise as the second argument.
10803+
*/
10804+
void SetPromiseHooks(Local<Function> init_hook,
10805+
Local<Function> before_hook,
10806+
Local<Function> after_hook,
10807+
Local<Function> resolve_hook);
10808+
1079710809
/**
1079810810
* Stack-allocated class which sets the execution context for all
1079910811
* 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
@@ -6147,6 +6147,45 @@ void Context::SetContinuationPreservedEmbedderData(Local<Value> data) {
61476147
*i::Handle<i::HeapObject>::cast(Utils::OpenHandle(*data)));
61486148
}
61496149

6150+
void v8::Context::SetPromiseHooks(Local<Function> init_hook,
6151+
Local<Function> before_hook,
6152+
Local<Function> after_hook,
6153+
Local<Function> resolve_hook) {
6154+
i::Handle<i::Context> context = Utils::OpenHandle(this);
6155+
i::Isolate* isolate = context->GetIsolate();
6156+
6157+
i::Handle<i::Object> init = isolate->factory()->undefined_value();
6158+
i::Handle<i::Object> before = isolate->factory()->undefined_value();
6159+
i::Handle<i::Object> after = isolate->factory()->undefined_value();
6160+
i::Handle<i::Object> resolve = isolate->factory()->undefined_value();
6161+
6162+
bool has_hook = false;
6163+
6164+
if (!init_hook.IsEmpty()) {
6165+
init = Utils::OpenHandle(*init_hook);
6166+
has_hook = true;
6167+
}
6168+
if (!before_hook.IsEmpty()) {
6169+
before = Utils::OpenHandle(*before_hook);
6170+
has_hook = true;
6171+
}
6172+
if (!after_hook.IsEmpty()) {
6173+
after = Utils::OpenHandle(*after_hook);
6174+
has_hook = true;
6175+
}
6176+
if (!resolve_hook.IsEmpty()) {
6177+
resolve = Utils::OpenHandle(*resolve_hook);
6178+
has_hook = true;
6179+
}
6180+
6181+
isolate->SetHasContextPromiseHooks(has_hook);
6182+
6183+
context->native_context().set_promise_hook_init_function(*init);
6184+
context->native_context().set_promise_hook_before_function(*before);
6185+
context->native_context().set_promise_hook_after_function(*after);
6186+
context->native_context().set_promise_hook_resolve_function(*resolve);
6187+
}
6188+
61506189
MaybeLocal<Context> metrics::Recorder::GetContext(
61516190
Isolate* isolate, metrics::Recorder::ContextId id) {
61526191
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);

‎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(
@@ -199,7 +202,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
199202
const TNode<Object> thenable = LoadObjectField(
200203
microtask, PromiseResolveThenableJobTask::kThenableOffset);
201204

202-
RunPromiseHook(Runtime::kPromiseHookBefore, microtask_context,
205+
RunAllPromiseHooks(PromiseHookType::kBefore, microtask_context,
203206
CAST(promise_to_resolve));
204207

205208
{
@@ -208,7 +211,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
208211
promise_to_resolve, thenable, then);
209212
}
210213

211-
RunPromiseHook(Runtime::kPromiseHookAfter, microtask_context,
214+
RunAllPromiseHooks(PromiseHookType::kAfter, microtask_context,
212215
CAST(promise_to_resolve));
213216

214217
RewindEnteredContext(saved_entered_context_count);
@@ -243,8 +246,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
243246
BIND(&preserved_data_done);
244247

245248
// Run the promise before/debug hook if enabled.
246-
RunPromiseHook(Runtime::kPromiseHookBefore, microtask_context,
247-
promise_or_capability);
249+
RunAllPromiseHooks(PromiseHookType::kBefore, microtask_context,
250+
promise_or_capability);
248251

249252
{
250253
ScopedExceptionHandler handler(this, &if_exception, &var_exception);
@@ -253,8 +256,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
253256
}
254257

255258
// Run the promise after/debug hook if enabled.
256-
RunPromiseHook(Runtime::kPromiseHookAfter, microtask_context,
257-
promise_or_capability);
259+
RunAllPromiseHooks(PromiseHookType::kAfter, microtask_context,
260+
promise_or_capability);
258261

259262
Label preserved_data_reset_done(this);
260263
GotoIf(IsUndefined(preserved_embedder_data), &preserved_data_reset_done);
@@ -296,8 +299,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
296299
BIND(&preserved_data_done);
297300

298301
// Run the promise before/debug hook if enabled.
299-
RunPromiseHook(Runtime::kPromiseHookBefore, microtask_context,
300-
promise_or_capability);
302+
RunAllPromiseHooks(PromiseHookType::kBefore, microtask_context,
303+
promise_or_capability);
301304

302305
{
303306
ScopedExceptionHandler handler(this, &if_exception, &var_exception);
@@ -306,8 +309,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
306309
}
307310

308311
// Run the promise after/debug hook if enabled.
309-
RunPromiseHook(Runtime::kPromiseHookAfter, microtask_context,
310-
promise_or_capability);
312+
RunAllPromiseHooks(PromiseHookType::kAfter, microtask_context,
313+
promise_or_capability);
311314

312315
Label preserved_data_reset_done(this);
313316
GotoIf(IsUndefined(preserved_embedder_data), &preserved_data_reset_done);
@@ -465,12 +468,43 @@ void MicrotaskQueueBuiltinsAssembler::RewindEnteredContext(
465468
saved_entered_context_count);
466469
}
467470

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

‎deps/v8/src/builtins/cast.tq‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,12 @@ Cast<Undefined|Callable>(o: HeapObject): Undefined|Callable
386386
return HeapObjectToCallable(o) otherwise CastError;
387387
}
388388

389+
Cast<Undefined|JSFunction>(o: HeapObject): Undefined|JSFunction
390+
labels CastError {
391+
if (o == Undefined) return Undefined;
392+
return Cast<JSFunction>(o) otherwise CastError;
393+
}
394+
389395
macro Cast<T : type extends Symbol>(o: Symbol): T labels CastError;
390396
Cast<PublicSymbol>(s: Symbol): PublicSymbol labels CastError {
391397
if (s.flags.is_private) goto CastError;

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

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

199+
RunContextPromiseHookResolve(promise);
200+
199201
// 2. Let reactions be promise.[[PromiseFulfillReactions]].
200202
const reactions =
201203
UnsafeCast<(Zero | PromiseReaction)>(promise.reactions_or_result);
@@ -214,17 +216,24 @@ FulfillPromise(implicit context: Context)(
214216
}
215217

216218
extern macro PromiseBuiltinsAssembler::
217-
IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(): bool;
219+
IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(): bool;
220+
221+
extern macro PromiseBuiltinsAssembler::
222+
IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(uint32):
223+
bool;
218224

219225
// https://tc39.es/ecma262/#sec-rejectpromise
220226
transitioning builtin
221227
RejectPromise(implicit context: Context)(
222228
promise: JSPromise, reason: JSAny, debugEvent: Boolean): JSAny {
229+
const promiseHookFlags = PromiseHookFlags();
230+
223231
// If promise hook is enabled or the debugger is active, let
224232
// the runtime handle this operation, which greatly reduces
225233
// the complexity here and also avoids a couple of back and
226234
// forth between JavaScript and C++ land.
227-
if (IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate() ||
235+
if (IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(
236+
promiseHookFlags) ||
228237
!promise.HasHandler()) {
229238
// 7. If promise.[[PromiseIsHandled]] is false, perform
230239
// HostPromiseRejectionTracker(promise, "reject").
@@ -233,6 +242,8 @@ RejectPromise(implicit context: Context)(
233242
return runtime::RejectPromise(promise, reason, debugEvent);
234243
}
235244

245+
RunContextPromiseHookResolve(promise, promiseHookFlags);
246+
236247
// 2. Let reactions be promise.[[PromiseRejectReactions]].
237248
const reactions =
238249
UnsafeCast<(Zero | PromiseReaction)>(promise.reactions_or_result);

0 commit comments

Comments
 (0)