Skip to content

Commit

Permalink
async_hooks: optimize fast-path promise hook for ALS (iter 2)
Browse files Browse the repository at this point in the history 
Refs: nodejs#34512
  • Loading branch information
@puzpuzpuz
puzpuzpuz committed Jul 28, 2020
1 parent 53870dd commit e937178
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 113 deletions.
2 changes: 1 addition & 1 deletion configure.py
View file
Expand Up @@ -1322,7 +1322,7 @@ def configure_v8(o):
o['variables']['v8_optimized_debug'] = 0 if options.v8_non_optimized_debug else 1
o['variables']['dcheck_always_on'] = 1 if options.v8_with_dchecks else 0
o['variables']['v8_random_seed'] = 0 # Use a random seed for hash tables.
o['variables']['v8_promise_internal_field_count'] = 1 # Add internal field to promises for async hooks.
o['variables']['v8_promise_internal_field_count'] = 2 # Add internal fields to promises for async hooks.
o['variables']['v8_use_siphash'] = 0 if options.without_siphash else 1
o['variables']['v8_enable_pointer_compression'] = 1 if options.enable_pointer_compression else 0
o['variables']['v8_enable_31bit_smis_on_64bit_arch'] = 1 if options.enable_pointer_compression else 0
Expand Down
27 changes: 3 additions & 24 deletions lib/internal/async_hooks.js
View file
Expand Up @@ -6,7 +6,6 @@ const {
FunctionPrototypeBind,
ObjectPrototypeHasOwnProperty,
ObjectDefineProperty,
Promise,
ReflectApply,
Symbol,
} = primordials;
Expand Down Expand Up @@ -297,34 +296,14 @@ function restoreActiveHooks() {
active_hooks.tmp_fields = null;
}

function trackPromise(promise, parent, silent) {
const asyncId = getOrSetAsyncId(promise);

promise[trigger_async_id_symbol] = parent ? getOrSetAsyncId(parent) :
getDefaultTriggerAsyncId();

if (!silent && initHooksExist()) {
const triggerId = promise[trigger_async_id_symbol];
// TODO(puzpuzpuz) remove
function fastPromiseHook(type, promise, asyncId, triggerId) {
if (type === kInit && initHooksExist()) {
emitInitScript(asyncId, 'PROMISE', triggerId, promise);
}
}

function fastPromiseHook(type, promise, parent) {
if (type === kInit || !promise[async_id_symbol]) {
const silent = type !== kInit;
if (parent instanceof Promise) {
trackPromise(promise, parent, silent);
} else {
trackPromise(promise, null, silent);
}

if (!silent) return;
}

const asyncId = promise[async_id_symbol];
switch (type) {
case kBefore:
const triggerId = promise[trigger_async_id_symbol];
emitBeforeScript(asyncId, triggerId, promise);
break;
case kAfter:
Expand Down
165 changes: 93 additions & 72 deletions src/async_wrap.cc
View file
Expand Up @@ -189,6 +189,43 @@ void AsyncWrap::EmitAfter(Environment* env, double async_id) {
env->async_hooks_after_function());
}

static double GetOrAssignPromiseAsyncId(Environment* env,
Local<Promise> promise,
bool read_only) {
if (!read_only &&
promise->GetAlignedPointerFromInternalField(0) == nullptr) {
double async_id = env->new_async_id();
promise->SetInternalField(0, Number::New(env->isolate(), async_id));
return async_id;
}

Local<Value> obj = promise->GetInternalField(0);
if (obj->IsNumber()) {
return obj.As<Number>()->Value();
}

return AsyncWrap::kInvalidAsyncId;
}

static double GetPromiseTriggerAsyncId(Environment* env,
Local<Promise> promise) {
if (promise->GetAlignedPointerFromInternalField(1) == nullptr) {
return AsyncWrap::kInvalidAsyncId;
}

Local<Value> obj = promise->GetInternalField(1);
return obj->IsNumber() ? obj.As<Number>()->Value()
: AsyncWrap::kInvalidAsyncId;

return AsyncWrap::kInvalidAsyncId;
}

static void AssignPromiseTriggerAsyncId(Environment* env,
Local<Promise> promise,
double trigger_async_id) {
promise->SetInternalField(1, Number::New(env->isolate(), trigger_async_id));
}

class PromiseWrap : public AsyncWrap {
public:
PromiseWrap(Environment* env, Local<Object> object, bool silent)
Expand Down Expand Up @@ -226,24 +263,21 @@ PromiseWrap* PromiseWrap::New(Environment* env,
if (!env->promise_wrap_template()->NewInstance(context).ToLocal(&obj))
return nullptr;

CHECK_NULL(promise->GetAlignedPointerFromInternalField(0));
promise->SetInternalField(0, obj);

double async_id = kInvalidAsyncId;
double trigger_async_id = kInvalidAsyncId;
// Skip for init events
if (silent) {
Local<Value> maybe_async_id = promise
->Get(context, env->async_id_symbol())
.ToLocalChecked();

Local<Value> maybe_trigger_async_id = promise
->Get(context, env->trigger_async_id_symbol())
.ToLocalChecked();

if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) {
double async_id = maybe_async_id.As<Number>()->Value();
double trigger_async_id = maybe_trigger_async_id.As<Number>()->Value();
return new PromiseWrap(env, obj, async_id, trigger_async_id);
}
// Interop with fast hook.
async_id = GetOrAssignPromiseAsyncId(env, promise, true);
trigger_async_id = GetPromiseTriggerAsyncId(env, promise);
}

promise->SetInternalField(0, obj);

if (async_id != kInvalidAsyncId && trigger_async_id != kInvalidAsyncId) {
// Clean up internal fields.
promise->SetInternalField(1, Undefined(env->isolate()));
return new PromiseWrap(env, obj, async_id, trigger_async_id);
}

return new PromiseWrap(env, obj, silent);
Expand Down Expand Up @@ -312,75 +346,62 @@ static uint16_t ToAsyncHooksType(PromiseHookType type) {
UNREACHABLE();
}

// Simplified JavaScript hook fast-path for when there is no destroy hook
// Simplified fast-path hook for when there is no destroy hook
static void FastPromiseHook(PromiseHookType type, Local<Promise> promise,
Local<Value> parent) {
Local<Context> context = promise->CreationContext();
Environment* env = Environment::GetCurrent(context);
if (env == nullptr) return;

if (type == PromiseHookType::kBefore &&
env->async_hooks()->fields()[AsyncHooks::kBefore] == 0) {
Local<Value> maybe_async_id;
if (!promise->Get(context, env->async_id_symbol())
.ToLocal(&maybe_async_id)) {
return;
}

Local<Value> maybe_trigger_async_id;
if (!promise->Get(context, env->trigger_async_id_symbol())
.ToLocal(&maybe_trigger_async_id)) {
return;
}

if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) {
double async_id = maybe_async_id.As<Number>()->Value();
double trigger_async_id = maybe_trigger_async_id.As<Number>()->Value();
env->async_hooks()->push_async_context(
async_id, trigger_async_id, promise);
}

return;
double async_id = GetOrAssignPromiseAsyncId(env, promise, false);
if (async_id == AsyncWrap::kInvalidAsyncId) {
// Interop with slow hook.
PromiseWrap* wrap = extractPromiseWrap(promise);
if (wrap == nullptr) return;
async_id = wrap->get_async_id();
}

if (type == PromiseHookType::kAfter &&
env->async_hooks()->fields()[AsyncHooks::kAfter] == 0) {
Local<Value> maybe_async_id;
if (!promise->Get(context, env->async_id_symbol())
.ToLocal(&maybe_async_id)) {
return;
}

if (maybe_async_id->IsNumber()) {
double async_id = maybe_async_id.As<Number>()->Value();
if (env->execution_async_id() == async_id) {
// This condition might not be true if async_hooks was enabled during
// the promise callback execution.
env->async_hooks()->pop_async_context(async_id);
double trigger_async_id = GetPromiseTriggerAsyncId(env, promise);
if (trigger_async_id == AsyncWrap::kInvalidAsyncId) {
// Interop with slow hook.
PromiseWrap* wrap = extractPromiseWrap(promise);
if (wrap != nullptr) {
trigger_async_id = wrap->get_trigger_async_id();
} else if (parent->IsPromise()) {
Local<Promise> parent_promise = parent.As<Promise>();
trigger_async_id = GetOrAssignPromiseAsyncId(env, parent_promise, false);
if (trigger_async_id == AsyncWrap::kInvalidAsyncId) {
// Interop with slow hook.
PromiseWrap* wrap = extractPromiseWrap(parent_promise);
if (wrap == nullptr) return;
trigger_async_id = wrap->get_trigger_async_id();
} else {
AssignPromiseTriggerAsyncId(env, promise, trigger_async_id);
}
} else {
trigger_async_id = env->get_default_trigger_async_id();
}

return;
}

if (type == PromiseHookType::kResolve &&
env->async_hooks()->fields()[AsyncHooks::kPromiseResolve] == 0) {
return;
if (type == PromiseHookType::kInit) {
AsyncWrap::EmitAsyncInit(env, promise,
env->async_hooks()
->provider_string(AsyncWrap::PROVIDER_PROMISE),
async_id, trigger_async_id);
} else if (type == PromiseHookType::kBefore) {
env->async_hooks()->push_async_context(
async_id, trigger_async_id, promise);
AsyncWrap::EmitBefore(env, async_id);
} else if (type == PromiseHookType::kAfter) {
AsyncWrap::EmitAfter(env, async_id);
if (env->execution_async_id() == async_id) {
// This condition might not be true if async_hooks was enabled during
// the promise callback execution.
env->async_hooks()->pop_async_context(async_id);
}
} else if (type == PromiseHookType::kResolve) {
AsyncWrap::EmitPromiseResolve(env, async_id);
}

// Getting up to this point means either init type or
// that there are active hooks of another type.
// In both cases fast-path JS hook should be called.

Local<Value> argv[] = {
Integer::New(env->isolate(), ToAsyncHooksType(type)),
promise,
parent
};

TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal);
Local<Function> promise_hook = env->promise_hook_handler();
USE(promise_hook->Call(context, Undefined(env->isolate()), 3, argv));
}

static void FullPromiseHook(PromiseHookType type, Local<Promise> promise,
Expand Down
17 changes: 12 additions & 5 deletions test/addons/async-hooks-promise/binding.cc
View file
Expand Up @@ -6,7 +6,7 @@ namespace {
using v8::FunctionCallbackInfo;
using v8::Isolate;
using v8::Local;
using v8::NewStringType;
using v8::Number;
using v8::Object;
using v8::Promise;
using v8::String;
Expand All @@ -23,13 +23,20 @@ static void GetPromiseField(const FunctionCallbackInfo<Value>& args) {
auto isolate = args.GetIsolate();

if (!args[0]->IsPromise())
return ThrowError(isolate, "arg is not an Promise");
return ThrowError(isolate, "arg 0 is not an Promise");

if (!args[1]->IsNumber())
return ThrowError(isolate, "arg 1 is not a number");

auto p = args[0].As<Promise>();
if (p->InternalFieldCount() < 1)
return ThrowError(isolate, "Promise has no internal field");
if (p->InternalFieldCount() < 2)
return ThrowError(isolate, "Promise has no internal fields");

double fieldIdx = args[1].As<Number>()->Value();
if (fieldIdx != 0 && fieldIdx != 1)
return ThrowError(isolate, "Index has to be 0 or 1");

auto l = p->GetInternalField(0);
auto l = p->GetInternalField(fieldIdx);
args.GetReturnValue().Set(l);
}

Expand Down
31 changes: 20 additions & 11 deletions test/addons/async-hooks-promise/test.js
View file
Expand Up @@ -13,16 +13,19 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) {
return;
}

// Baseline to make sure the internal field isn't being set.
// Baseline to make sure both internal fields aren't set.
assert.strictEqual(
binding.getPromiseField(Promise.resolve(1)),
binding.getPromiseField(Promise.resolve(1), 0),
0);
assert.strictEqual(
binding.getPromiseField(Promise.resolve(1), 1),
0);

const emptyHook = async_hooks.createHook({}).enable();

// Check that no PromiseWrap is created when there are no hook callbacks.
assert.strictEqual(
binding.getPromiseField(Promise.resolve(1)),
binding.getPromiseField(Promise.resolve(1), 1),
0);

emptyHook.disable();
Expand All @@ -41,10 +44,11 @@ const initOnlyHook = async_hooks.createHook({
// Check that no PromiseWrap is created when only using an init hook.
{
const promise = Promise.resolve(1);
assert.strictEqual(binding.getPromiseField(promise), 0);
assert.strictEqual(typeof binding.getPromiseField(promise, 0), 'number');
assert.strictEqual(typeof binding.getPromiseField(promise, 1), 'number');
assert.strictEqual(lastResource, promise);
assert.strictEqual(lastAsyncId, promise[async_id_symbol]);
assert.strictEqual(lastTriggerAsyncId, promise[trigger_async_id_symbol]);
assert.strictEqual(lastAsyncId, binding.getPromiseField(promise, 0));
assert.strictEqual(lastTriggerAsyncId, binding.getPromiseField(promise, 1));
}

initOnlyHook.disable();
Expand All @@ -65,7 +69,8 @@ const hookWithDestroy = async_hooks.createHook({
// Check that the internal field returns the same PromiseWrap passed to init().
{
const promise = Promise.resolve(1);
const promiseWrap = binding.getPromiseField(promise);
const promiseWrap = binding.getPromiseField(promise, 0);
assert.strictEqual(binding.getPromiseField(promise, 1), 0);
assert.strictEqual(lastResource, promiseWrap);
assert.strictEqual(lastAsyncId, promiseWrap[async_id_symbol]);
assert.strictEqual(lastTriggerAsyncId, promiseWrap[trigger_async_id_symbol]);
Expand All @@ -78,7 +83,10 @@ hookWithDestroy.disable();
// future microtask.
setImmediate(() => {
assert.strictEqual(
binding.getPromiseField(Promise.resolve(1)),
binding.getPromiseField(Promise.resolve(1), 0),
0);
assert.strictEqual(
binding.getPromiseField(Promise.resolve(1), 1),
0);

const noDestroyHook = async_hooks.createHook({
Expand All @@ -95,10 +103,11 @@ setImmediate(() => {

// Check that no PromiseWrap is created when there is no destroy hook.
const promise = Promise.resolve(1);
assert.strictEqual(binding.getPromiseField(promise), 0);
assert.strictEqual(typeof binding.getPromiseField(promise, 0), 'number');
assert.strictEqual(typeof binding.getPromiseField(promise, 1), 'number');
assert.strictEqual(lastResource, promise);
assert.strictEqual(lastAsyncId, promise[async_id_symbol]);
assert.strictEqual(lastTriggerAsyncId, promise[trigger_async_id_symbol]);
assert.strictEqual(lastAsyncId, binding.getPromiseField(promise, 0));
assert.strictEqual(lastTriggerAsyncId, binding.getPromiseField(promise, 1));

noDestroyHook.disable();
});

0 comments on commit e937178

Please sign in to comment.