diff --git a/benchmark/napi/make_callback/binding.cc b/benchmark/napi/make_callback/binding.cc index 85d335170fb0..3a793b6c1902 100644 --- a/benchmark/napi/make_callback/binding.cc +++ b/benchmark/napi/make_callback/binding.cc @@ -12,12 +12,19 @@ using v8::Local; using v8::Object; using v8::Value; +// Same order as the types in index.js. +enum Type { kMakeCallback, kAsyncResource, kCall }; + struct State { uv_timer_t timer; Isolate* isolate; int64_t n; + Type type; Global fn; Global done; + node::AsyncResource* resource = nullptr; + + ~State() { delete resource; } }; static void OnTimer(uv_timer_t* handle) { @@ -30,7 +37,17 @@ static void OnTimer(uv_timer_t* handle) { Local recv = context->Global(); for (int64_t i = 0; i < state->n; i++) { HandleScope inner_scope(isolate); - (void)node::MakeCallback(isolate, recv, fn, 0, nullptr, {0, 0}); + switch (state->type) { + case kMakeCallback: + (void)node::MakeCallback(isolate, recv, fn, 0, nullptr, {0, 0}); + break; + case kAsyncResource: + (void)state->resource->MakeCallback(fn, 0, nullptr); + break; + case kCall: + (void)fn->Call(context, recv, 0, nullptr); + break; + } } Local done = state->done.Get(isolate); (void)node::MakeCallback(isolate, recv, done, 0, nullptr, {0, 0}); @@ -38,7 +55,7 @@ static void OnTimer(uv_timer_t* handle) { [](uv_handle_t* h) { delete static_cast(h->data); }); } -// run(n, fn, done): calls fn n times from a timer, then calls done. +// run(n, fn, done, type): calls fn n times from a timer, then calls done. static void Run(const FunctionCallbackInfo& args) { Isolate* isolate = args.GetIsolate(); State* state = new State; @@ -46,6 +63,12 @@ static void Run(const FunctionCallbackInfo& args) { state->n = args[0]->IntegerValue(isolate->GetCurrentContext()).FromJust(); state->fn.Reset(isolate, args[1].As()); state->done.Reset(isolate, args[2].As()); + state->type = static_cast( + args[3]->Int32Value(isolate->GetCurrentContext()).FromJust()); + if (state->type == kAsyncResource) { + state->resource = + new node::AsyncResource(isolate, Object::New(isolate), "Benchmark"); + } state->timer.data = state; uv_timer_init(node::GetCurrentEventLoop(isolate), &state->timer); uv_timer_start(&state->timer, OnTimer, 0, 0); diff --git a/benchmark/napi/make_callback/index.js b/benchmark/napi/make_callback/index.js index 8c3dba383437..37e76788be70 100644 --- a/benchmark/napi/make_callback/index.js +++ b/benchmark/napi/make_callback/index.js @@ -2,8 +2,9 @@ const common = require('../../common.js'); -// The addon calls into JS with node::MakeCallback from a libuv timer, so -// every call opens a top-level callback scope, like an I/O callback does. +// The addon calls into JS from a libuv timer, so every call opens a top-level +// callback scope, like an I/O callback does. type=Call is a plain +// v8::Function::Call, what the call costs without Node. let binding; try { @@ -13,11 +14,14 @@ try { process.exit(0); } +const types = ['MakeCallback', 'AsyncResource', 'Call']; + const bench = common.createBenchmark(main, { + type: types, n: [1e6, 1e7], }); -function main({ n }) { +function main({ type, n }) { bench.start(); - binding.run(n, () => {}, () => bench.end(n)); + binding.run(n, () => {}, () => bench.end(n), types.indexOf(type)); } diff --git a/src/api/async_resource.cc b/src/api/async_resource.cc index 621fca4ba4d1..54ed0382f896 100644 --- a/src/api/async_resource.cc +++ b/src/api/async_resource.cc @@ -1,6 +1,7 @@ #include "async_context_frame.h" #include "env-inl.h" #include "node.h" +#include "node_internals.h" namespace node { @@ -10,6 +11,7 @@ using v8::Local; using v8::MaybeLocal; using v8::Object; using v8::String; +using v8::Undefined; using v8::Value; AsyncResource::AsyncResource(Isolate* isolate, @@ -39,33 +41,40 @@ MaybeLocal AsyncResource::MakeCallback(Local callback, int argc, Local* argv) { auto isolate = env_->isolate(); - async_context_frame::Scope async_context_frame_scope( - isolate, context_frame_.Get(isolate)); - - return node::MakeCallback( - isolate, get_resource(), callback, argc, argv, async_context_); + // As in Node-API: node::MakeCallback() would run it with no frame. + return InternalMakeCallback(isolate, + get_resource(), + callback, + argc, + argv, + async_context_, + context_frame_.Get(isolate)); } MaybeLocal AsyncResource::MakeCallback(const char* method, int argc, Local* argv) { - auto isolate = env_->isolate(); - async_context_frame::Scope async_context_frame_scope( - isolate, context_frame_.Get(isolate)); - - return node::MakeCallback( - isolate, get_resource(), method, argc, argv, async_context_); + Local method_string; + if (!String::NewFromUtf8(env_->isolate(), method).ToLocal(&method_string)) { + return {}; + } + return MakeCallback(method_string, argc, argv); } MaybeLocal AsyncResource::MakeCallback(Local symbol, int argc, Local* argv) { auto isolate = env_->isolate(); - async_context_frame::Scope async_context_frame_scope( - isolate, context_frame_.Get(isolate)); - - return node::MakeCallback( - isolate, get_resource(), symbol, argc, argv, async_context_); + // Check can_call_into_js() first because calling Get() might do so. + if (!env_->can_call_into_js()) return {}; + Local callback; + if (!get_resource() + ->Get(isolate->GetCurrentContext(), symbol) + .ToLocal(&callback)) { + return {}; + } + if (!callback->IsFunction()) return Undefined(isolate); + return MakeCallback(callback.As(), argc, argv); } Local AsyncResource::get_resource() { diff --git a/test/addons/async-resource/test-async-local-storage.js b/test/addons/async-resource/test-async-local-storage.js new file mode 100644 index 000000000000..f475de221710 --- /dev/null +++ b/test/addons/async-resource/test-async-local-storage.js @@ -0,0 +1,22 @@ +'use strict'; + +const common = require('../../common'); +const assert = require('assert'); +const { AsyncLocalStorage } = require('async_hooks'); +const binding = require(`./build/${common.buildType}/binding`); + +// AsyncResource::MakeCallback() must run the callback in the async context +// frame that was active when the resource was created. + +const als = new AsyncLocalStorage(); +const object = { + 'methöd': common.mustCall(() => { + assert.strictEqual(als.getStore(), 'store'); + }, 3), +}; +const resource = als.run('store', () => binding.createAsyncResource(object)); + +binding.callViaFunction(resource); +binding.callViaString(resource); +binding.callViaUtf8Name(resource); +binding.destroyAsyncResource(resource);