Skip to content

Commit

Permalink
deps: V8: backport c0fceaa0669b
Browse files Browse the repository at this point in the history
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ä <marja@chromium.org>
    > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    > Reviewed-by: Anton Bikineev <bikineev@chromium.org>
    > Reviewed-by: Igor Sheludko <ishell@chromium.org>
    > Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    > 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ä <marja@chromium.org>
    Reviewed-by: Igor Sheludko <ishell@chromium.org>
    Reviewed-by: Anton Bikineev <bikineev@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#74071}

Refs: v8/v8@c0fceaa

PR-URL: #36394
Backport-PR-URL: #38577
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
Stephen Belanger authored and MylesBorins committed Aug 31, 2021
1 parent f421422 commit 214e568
Show file tree
Hide file tree
Showing 31 changed files with 772 additions and 132 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

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

##### V8 defaults for Node.js #####

Expand Down
1 change: 1 addition & 0 deletions deps/v8/AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ Seo Sanghyeon <sanxiyn@gmail.com>
Shawn Anastasio <shawnanastasio@gmail.com>
Shawn Presser <shawnpresser@gmail.com>
Stefan Penner <stefan.penner@gmail.com>
Stephen Belanger <stephen.belanger@datadoghq.com>
Sylvestre Ledru <sledru@mozilla.com>
Taketoshi Aono <brn@b6n.ch>
Teddy Katz <teddy.katz@gmail.com>
Expand Down
12 changes: 12 additions & 0 deletions deps/v8/include/v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -10488,6 +10488,18 @@ class V8_EXPORT Context {
*/
void SetContinuationPreservedEmbedderData(Local<Value> context);

/**
* Set or clear hooks to be invoked for promise lifecycle operations.
* To clear a hook, set it to an empty v8::Function. Each function will
* receive the observed promise as the first argument. If a chaining
* operation is used on a promise, the init will additionally receive
* the parent promise as the second argument.
*/
void SetPromiseHooks(Local<Function> init_hook,
Local<Function> before_hook,
Local<Function> after_hook,
Local<Function> resolve_hook);

/**
* Stack-allocated class which sets the execution context for all
* operations executed within a local scope.
Expand Down
39 changes: 39 additions & 0 deletions deps/v8/src/api/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6078,6 +6078,45 @@ void Context::SetContinuationPreservedEmbedderData(Local<Value> data) {
*i::Handle<i::HeapObject>::cast(Utils::OpenHandle(*data)));
}

void v8::Context::SetPromiseHooks(Local<Function> init_hook,
Local<Function> before_hook,
Local<Function> after_hook,
Local<Function> resolve_hook) {
i::Handle<i::Context> context = Utils::OpenHandle(this);
i::Isolate* isolate = context->GetIsolate();

i::Handle<i::Object> init = isolate->factory()->undefined_value();
i::Handle<i::Object> before = isolate->factory()->undefined_value();
i::Handle<i::Object> after = isolate->factory()->undefined_value();
i::Handle<i::Object> resolve = isolate->factory()->undefined_value();

bool has_hook = false;

if (!init_hook.IsEmpty()) {
init = Utils::OpenHandle(*init_hook);
has_hook = true;
}
if (!before_hook.IsEmpty()) {
before = Utils::OpenHandle(*before_hook);
has_hook = true;
}
if (!after_hook.IsEmpty()) {
after = Utils::OpenHandle(*after_hook);
has_hook = true;
}
if (!resolve_hook.IsEmpty()) {
resolve = Utils::OpenHandle(*resolve_hook);
has_hook = true;
}

isolate->SetHasContextPromiseHooks(has_hook);

context->native_context().set_promise_hook_init_function(*init);
context->native_context().set_promise_hook_before_function(*before);
context->native_context().set_promise_hook_after_function(*after);
context->native_context().set_promise_hook_resolve_function(*resolve);
}

namespace {
i::Address* GetSerializedDataFromFixedArray(i::Isolate* isolate,
i::FixedArray list, size_t index) {
Expand Down
4 changes: 3 additions & 1 deletion deps/v8/src/builtins/builtins-async-function-gen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,14 @@ TF_BUILTIN(AsyncFunctionEnter, AsyncFunctionBuiltinsAssembler) {
StoreObjectFieldNoWriteBarrier(
async_function_object, JSAsyncFunctionObject::kPromiseOffset, promise);

RunContextPromiseHookInit(context, promise, UndefinedConstant());

// Fire promise hooks if enabled and push the Promise under construction
// in an async function on the catch prediction stack to handle exceptions
// thrown before the first await.
Label if_instrumentation(this, Label::kDeferred),
if_instrumentation_done(this);
Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
Branch(IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
&if_instrumentation, &if_instrumentation_done);
BIND(&if_instrumentation);
{
Expand Down
62 changes: 40 additions & 22 deletions deps/v8/src/builtins/builtins-async-gen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,11 @@ TNode<Object> AsyncBuiltinsAssembler::AwaitOld(

TVARIABLE(HeapObject, var_throwaway, UndefinedConstant());

// Deal with PromiseHooks and debug support in the runtime. This
// also allocates the throwaway promise, which is only needed in
// case of PromiseHooks or debugging.
Label if_debugging(this, Label::kDeferred), do_resolve_promise(this);
Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
&if_debugging, &do_resolve_promise);
BIND(&if_debugging);
var_throwaway =
CAST(CallRuntime(Runtime::kAwaitPromisesInitOld, context, value, promise,
outer_promise, on_reject, is_predicted_as_caught));
Goto(&do_resolve_promise);
BIND(&do_resolve_promise);
RunContextPromiseHookInit(context, promise, outer_promise);

InitAwaitPromise(Runtime::kAwaitPromisesInitOld, context, value, promise,
outer_promise, on_reject, is_predicted_as_caught,
&var_throwaway);

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

TVARIABLE(HeapObject, var_throwaway, UndefinedConstant());

InitAwaitPromise(Runtime::kAwaitPromisesInit, context, promise, promise,
outer_promise, on_reject, is_predicted_as_caught,
&var_throwaway);

return CallBuiltin(Builtins::kPerformPromiseThen, native_context, promise,
on_resolve, on_reject, var_throwaway.value());
}

void AsyncBuiltinsAssembler::InitAwaitPromise(
Runtime::FunctionId id, TNode<Context> context, TNode<Object> value,
TNode<Object> promise, TNode<Object> outer_promise,
TNode<HeapObject> on_reject, TNode<Oddball> is_predicted_as_caught,
TVariable<HeapObject>* var_throwaway) {
// Deal with PromiseHooks and debug support in the runtime. This
// also allocates the throwaway promise, which is only needed in
// case of PromiseHooks or debugging.
Label if_debugging(this, Label::kDeferred), do_perform_promise_then(this);
Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(),
&if_debugging, &do_perform_promise_then);
Label if_debugging(this, Label::kDeferred),
if_promise_hook(this, Label::kDeferred),
not_debugging(this),
do_nothing(this);
TNode<Uint32T> promiseHookFlags = PromiseHookFlags();
Branch(IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(
promiseHookFlags), &if_debugging, &not_debugging);
BIND(&if_debugging);
var_throwaway =
CAST(CallRuntime(Runtime::kAwaitPromisesInit, context, promise, promise,
*var_throwaway =
CAST(CallRuntime(id, context, value, promise,
outer_promise, on_reject, is_predicted_as_caught));
Goto(&do_perform_promise_then);
BIND(&do_perform_promise_then);

return CallBuiltin(Builtins::kPerformPromiseThen, native_context, promise,
on_resolve, on_reject, var_throwaway.value());
Goto(&do_nothing);
BIND(&not_debugging);

// This call to NewJSPromise is to keep behaviour parity with what happens
// in Runtime::kAwaitPromisesInit above if native hooks are set. It will
// create a throwaway promise that will trigger an init event and will get
// passed into Builtins::kPerformPromiseThen below.
Branch(IsContextPromiseHookEnabled(promiseHookFlags), &if_promise_hook,
&do_nothing);
BIND(&if_promise_hook);
*var_throwaway = NewJSPromise(context, promise);
Goto(&do_nothing);
BIND(&do_nothing);
}

TNode<Object> AsyncBuiltinsAssembler::Await(
Expand Down
6 changes: 6 additions & 0 deletions deps/v8/src/builtins/builtins-async-gen.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ class AsyncBuiltinsAssembler : public PromiseBuiltinsAssembler {
TNode<SharedFunctionInfo> on_resolve_sfi,
TNode<SharedFunctionInfo> on_reject_sfi,
TNode<Oddball> is_predicted_as_caught);

void InitAwaitPromise(
Runtime::FunctionId id, TNode<Context> context, TNode<Object> value,
TNode<Object> promise, TNode<Object> outer_promise,
TNode<HeapObject> on_reject, TNode<Oddball> is_predicted_as_caught,
TVariable<HeapObject>* var_throwaway);
};

} // namespace internal
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/builtins/builtins-async-generator-gen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ TF_BUILTIN(AsyncGeneratorResolve, AsyncGeneratorBuiltinsAssembler) {
// the "promiseResolve" hook would not be fired otherwise.
Label if_fast(this), if_slow(this, Label::kDeferred), return_promise(this);
GotoIfForceSlowPath(&if_slow);
GotoIf(IsPromiseHookEnabled(), &if_slow);
GotoIf(IsIsolatePromiseHookEnabledOrHasAsyncEventDelegate(), &if_slow);
Branch(IsPromiseThenProtectorCellInvalid(), &if_slow, &if_fast);

BIND(&if_fast);
Expand Down
62 changes: 48 additions & 14 deletions deps/v8/src/builtins/builtins-microtask-queue-gen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,11 @@ class MicrotaskQueueBuiltinsAssembler : public CodeStubAssembler {
void EnterMicrotaskContext(TNode<Context> native_context);
void RewindEnteredContext(TNode<IntPtrT> saved_entered_context_count);

void RunAllPromiseHooks(PromiseHookType type, TNode<Context> context,
TNode<HeapObject> promise_or_capability);
void RunPromiseHook(Runtime::FunctionId id, TNode<Context> context,
TNode<HeapObject> promise_or_capability);
TNode<HeapObject> promise_or_capability,
TNode<Uint32T> promiseHookFlags);
};

TNode<RawPtrT> MicrotaskQueueBuiltinsAssembler::GetMicrotaskQueue(
Expand Down Expand Up @@ -198,7 +201,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
const TNode<Object> thenable = LoadObjectField(
microtask, PromiseResolveThenableJobTask::kThenableOffset);

RunPromiseHook(Runtime::kPromiseHookBefore, microtask_context,
RunAllPromiseHooks(PromiseHookType::kBefore, microtask_context,
CAST(promise_to_resolve));

{
Expand All @@ -207,7 +210,7 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
promise_to_resolve, thenable, then);
}

RunPromiseHook(Runtime::kPromiseHookAfter, microtask_context,
RunAllPromiseHooks(PromiseHookType::kAfter, microtask_context,
CAST(promise_to_resolve));

RewindEnteredContext(saved_entered_context_count);
Expand Down Expand Up @@ -242,8 +245,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
BIND(&preserved_data_done);

// Run the promise before/debug hook if enabled.
RunPromiseHook(Runtime::kPromiseHookBefore, microtask_context,
promise_or_capability);
RunAllPromiseHooks(PromiseHookType::kBefore, microtask_context,
promise_or_capability);

{
ScopedExceptionHandler handler(this, &if_exception, &var_exception);
Expand All @@ -252,8 +255,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
}

// Run the promise after/debug hook if enabled.
RunPromiseHook(Runtime::kPromiseHookAfter, microtask_context,
promise_or_capability);
RunAllPromiseHooks(PromiseHookType::kAfter, microtask_context,
promise_or_capability);

Label preserved_data_reset_done(this);
GotoIf(IsUndefined(preserved_embedder_data), &preserved_data_reset_done);
Expand Down Expand Up @@ -295,8 +298,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
BIND(&preserved_data_done);

// Run the promise before/debug hook if enabled.
RunPromiseHook(Runtime::kPromiseHookBefore, microtask_context,
promise_or_capability);
RunAllPromiseHooks(PromiseHookType::kBefore, microtask_context,
promise_or_capability);

{
ScopedExceptionHandler handler(this, &if_exception, &var_exception);
Expand All @@ -305,8 +308,8 @@ void MicrotaskQueueBuiltinsAssembler::RunSingleMicrotask(
}

// Run the promise after/debug hook if enabled.
RunPromiseHook(Runtime::kPromiseHookAfter, microtask_context,
promise_or_capability);
RunAllPromiseHooks(PromiseHookType::kAfter, microtask_context,
promise_or_capability);

Label preserved_data_reset_done(this);
GotoIf(IsUndefined(preserved_embedder_data), &preserved_data_reset_done);
Expand Down Expand Up @@ -464,12 +467,43 @@ void MicrotaskQueueBuiltinsAssembler::RewindEnteredContext(
saved_entered_context_count);
}

void MicrotaskQueueBuiltinsAssembler::RunAllPromiseHooks(
PromiseHookType type, TNode<Context> context,
TNode<HeapObject> promise_or_capability) {
Label hook(this, Label::kDeferred), done_hook(this);
TNode<Uint32T> promiseHookFlags = PromiseHookFlags();
Branch(IsAnyPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(
promiseHookFlags), &hook, &done_hook);
BIND(&hook);
{
switch (type) {
case PromiseHookType::kBefore:
RunContextPromiseHookBefore(context, promise_or_capability,
promiseHookFlags);
RunPromiseHook(Runtime::kPromiseHookBefore, context,
promise_or_capability, promiseHookFlags);
break;
case PromiseHookType::kAfter:
RunContextPromiseHookAfter(context, promise_or_capability,
promiseHookFlags);
RunPromiseHook(Runtime::kPromiseHookAfter, context,
promise_or_capability, promiseHookFlags);
break;
default:
UNREACHABLE();
}
Goto(&done_hook);
}
BIND(&done_hook);
}

void MicrotaskQueueBuiltinsAssembler::RunPromiseHook(
Runtime::FunctionId id, TNode<Context> context,
TNode<HeapObject> promise_or_capability) {
TNode<HeapObject> promise_or_capability,
TNode<Uint32T> promiseHookFlags) {
Label hook(this, Label::kDeferred), done_hook(this);
Branch(IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(), &hook,
&done_hook);
Branch(IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(
promiseHookFlags), &hook, &done_hook);
BIND(&hook);
{
// Get to the underlying JSPromise instance.
Expand Down
15 changes: 13 additions & 2 deletions deps/v8/src/builtins/promise-abstract-operations.tq
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ FulfillPromise(implicit context: Context)(
// Assert: The value of promise.[[PromiseState]] is "pending".
assert(promise.Status() == PromiseState::kPending);

RunContextPromiseHookResolve(promise);

// 2. Let reactions be promise.[[PromiseFulfillReactions]].
const reactions =
UnsafeCast<(Zero | PromiseReaction)>(promise.reactions_or_result);
Expand All @@ -204,17 +206,24 @@ FulfillPromise(implicit context: Context)(
}

extern macro PromiseBuiltinsAssembler::
IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(): bool;
IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(): bool;

extern macro PromiseBuiltinsAssembler::
IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(uint32):
bool;

// https://tc39.es/ecma262/#sec-rejectpromise
transitioning builtin
RejectPromise(implicit context: Context)(
promise: JSPromise, reason: JSAny, debugEvent: Boolean): JSAny {
const promiseHookFlags = PromiseHookFlags();

// If promise hook is enabled or the debugger is active, let
// the runtime handle this operation, which greatly reduces
// the complexity here and also avoids a couple of back and
// forth between JavaScript and C++ land.
if (IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate() ||
if (IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate(
promiseHookFlags) ||
!promise.HasHandler()) {
// 7. If promise.[[PromiseIsHandled]] is false, perform
// HostPromiseRejectionTracker(promise, "reject").
Expand All @@ -223,6 +232,8 @@ RejectPromise(implicit context: Context)(
return runtime::RejectPromise(promise, reason, debugEvent);
}

RunContextPromiseHookResolve(promise, promiseHookFlags);

// 2. Let reactions be promise.[[PromiseRejectReactions]].
const reactions =
UnsafeCast<(Zero | PromiseReaction)>(promise.reactions_or_result);
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/builtins/promise-all.tq
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ Reject(Object) {
// PerformPromiseThen), since this is only necessary for DevTools and
// PromiseHooks.
if (promiseResolveFunction != Undefined ||
IsPromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate() ||
IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate() ||
IsPromiseSpeciesProtectorCellInvalid() || Is<Smi>(nextValue) ||
!IsPromiseThenLookupChainIntact(
nativeContext, UnsafeCast<HeapObject>(nextValue).map)) {
Expand Down
7 changes: 3 additions & 4 deletions deps/v8/src/builtins/promise-constructor.tq
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ extern macro ConstructorBuiltinsAssembler::EmitFastNewObject(
Context, JSFunction, JSReceiver): JSObject;

extern macro
PromiseBuiltinsAssembler::IsPromiseHookEnabledOrHasAsyncEventDelegate(): bool;
PromiseBuiltinsAssembler::IsIsolatePromiseHookEnabledOrHasAsyncEventDelegate(
uint32): bool;

// https://tc39.es/ecma262/#sec-promise-executor
transitioning javascript builtin
Expand Down Expand Up @@ -74,9 +75,7 @@ PromiseConstructor(
result = UnsafeCast<JSPromise>(EmitFastNewObject(
context, promiseFun, UnsafeCast<JSReceiver>(newTarget)));
PromiseInit(result);
if (IsPromiseHookEnabledOrHasAsyncEventDelegate()) {
runtime::PromiseHookInit(result, Undefined);
}
RunAnyPromiseHookInit(result, Undefined);
}

const isDebugActive = IsDebugActive();
Expand Down
Loading

0 comments on commit 214e568

Please sign in to comment.