diff --git a/Makefile b/Makefile index 424818eac49eed..f9b35e714bb641 100644 --- a/Makefile +++ b/Makefile @@ -276,6 +276,7 @@ test-check-deopts: all $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) --check-deopts parallel sequential benchmark/misc/function_call/build/Release/binding.node: all \ + benchmark/misc/function_call/napi_binding.c \ benchmark/misc/function_call/binding.cc \ benchmark/misc/function_call/binding.gyp $(NODE) deps/npm/node_modules/node-gyp/bin/node-gyp rebuild \ diff --git a/benchmark/misc/function_call/binding.gyp b/benchmark/misc/function_call/binding.gyp index 3bfb84493f3e87..ac122ed1a07962 100644 --- a/benchmark/misc/function_call/binding.gyp +++ b/benchmark/misc/function_call/binding.gyp @@ -1,5 +1,9 @@ { 'targets': [ + { + 'target_name': 'napi_binding', + 'sources': [ 'napi_binding.c' ] + }, { 'target_name': 'binding', 'sources': [ 'binding.cc' ] diff --git a/benchmark/misc/function_call/index.js b/benchmark/misc/function_call/index.js index cafff3edc122c6..cbc512c9729137 100644 --- a/benchmark/misc/function_call/index.js +++ b/benchmark/misc/function_call/index.js @@ -19,6 +19,15 @@ try { } const cxx = binding.hello; +let napi_binding; +try { + napi_binding = require('./build/Release/napi_binding'); +} catch (er) { + console.error('misc/function_call/index.js NAPI-Binding failed to load'); + process.exit(0); +} +const napi = napi_binding.hello; + var c = 0; function js() { return c++; @@ -27,12 +36,12 @@ function js() { assert(js() === cxx()); const bench = common.createBenchmark(main, { - type: ['js', 'cxx'], + type: ['js', 'cxx', 'napi'], n: [1e6, 1e7, 5e7] }); function main({ n, type }) { - const fn = type === 'cxx' ? cxx : js; + const fn = type === 'cxx' ? cxx : type === 'napi' ? napi : js; bench.start(); for (var i = 0; i < n; i++) { fn(); diff --git a/benchmark/misc/function_call/napi_binding.c b/benchmark/misc/function_call/napi_binding.c new file mode 100644 index 00000000000000..d97170e0fcd9c5 --- /dev/null +++ b/benchmark/misc/function_call/napi_binding.c @@ -0,0 +1,26 @@ +#include +#include + +static int32_t increment = 0; + +static napi_value Hello(napi_env env, napi_callback_info info) { + napi_value result; + napi_status status = napi_create_int32(env, increment++, &result); + assert(status == napi_ok); + return result; +} + +NAPI_MODULE_INIT() { + napi_value hello; + napi_status status = + napi_create_function(env, + "hello", + NAPI_AUTO_LENGTH, + Hello, + NULL, + &hello); + assert(status == napi_ok); + status = napi_set_named_property(env, exports, "hello", hello); + assert(status == napi_ok); + return exports; +} diff --git a/src/node_api.cc b/src/node_api.cc index 4c1f1a2b9a3e27..149cb8731d44f7 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -23,8 +23,6 @@ struct napi_env__ { loop(_loop) {} v8::Isolate* isolate; node::Persistent last_exception; - node::Persistent function_data_template; - node::Persistent accessor_data_template; napi_extended_error_info last_error; int open_handle_scopes = 0; int open_callback_scopes = 0; @@ -34,19 +32,6 @@ struct napi_env__ { #define NAPI_PRIVATE_KEY(context, suffix) \ (node::Environment::GetCurrent((context))->napi_ ## suffix()) -#define ENV_OBJECT_TEMPLATE(env, prefix, destination, field_count) \ - do { \ - if ((env)->prefix ## _template.IsEmpty()) { \ - (destination) = v8::ObjectTemplate::New(isolate); \ - (destination)->SetInternalFieldCount((field_count)); \ - (env)->prefix ## _template.Reset(isolate, (destination)); \ - } else { \ - (destination) = v8::Local::New( \ - isolate, env->prefix ## _template); \ - } \ - } while (0) - - #define RETURN_STATUS_IF_FALSE(env, condition, status) \ do { \ if (!(condition)) { \ @@ -491,15 +476,45 @@ class TryCatch : public v8::TryCatch { //=== Function napi_callback wrapper ================================= -static const int kDataIndex = 0; -static const int kEnvIndex = 1; +// TODO(somebody): these constants can be removed with relevant changes +// in CallbackWrapperBase<> and CallbackBundle. +// Leave them for now just to keep the change set and cognitive load minimal. +static const int kFunctionIndex = 0; // Used in CallbackBundle::cb[] +static const int kGetterIndex = 0; // Used in CallbackBundle::cb[] +static const int kSetterIndex = 1; // Used in CallbackBundle::cb[] +static const int kCallbackCount = 2; // Used in CallbackBundle::cb[] + // Max is "getter + setter" case + +// Use this data structure to associate callback data with each N-API function +// exposed to JavaScript. The structure is stored in a v8::External which gets +// passed into our callback wrapper. This reduces the performance impact of +// calling through N-API. +// Ref: benchmark/misc/function_call +// Discussion (incl. perf. data): https://github.com/nodejs/node/pull/21072 +struct CallbackBundle { + // Bind the lifecycle of `this` C++ object to a JavaScript object. + // We never delete a CallbackBundle C++ object directly. + void BindLifecycleTo(v8::Isolate* isolate, v8::Local target) { + handle.Reset(isolate, target); + handle.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter); + } + + napi_env env; // Necessary to invoke C++ NAPI callback + void* cb_data; // The user provided callback data + napi_callback cb[kCallbackCount]; // Max capacity is 2 (getter + setter) + node::Persistent handle; // Die with this JavaScript object -static const int kFunctionIndex = 2; -static const int kFunctionFieldCount = 3; - -static const int kGetterIndex = 2; -static const int kSetterIndex = 3; -static const int kAccessorFieldCount = 4; + private: + static void WeakCallback(v8::WeakCallbackInfo const& info) { + // Use the "WeakCallback mechanism" to delete the C++ `bundle` object. + // This will be called when the v8::External containing `this` pointer + // is being GC-ed. + CallbackBundle* bundle = info.GetParameter(); + if (bundle != nullptr) { + delete bundle; + } + } +}; // Base class extended by classes that wrap V8 function and property callback // info. @@ -531,10 +546,10 @@ class CallbackWrapperBase : public CallbackWrapper { : CallbackWrapper(JsValueFromV8LocalValue(cbinfo.This()), args_length, nullptr), - _cbinfo(cbinfo), - _cbdata(v8::Local::Cast(cbinfo.Data())) { - _data = v8::Local::Cast(_cbdata->GetInternalField(kDataIndex)) - ->Value(); + _cbinfo(cbinfo) { + _bundle = reinterpret_cast( + v8::Local::Cast(cbinfo.Data())->Value()); + _data = _bundle->cb_data; } napi_value GetNewTarget() override { return nullptr; } @@ -543,13 +558,10 @@ class CallbackWrapperBase : public CallbackWrapper { void InvokeCallback() { napi_callback_info cbinfo_wrapper = reinterpret_cast( static_cast(this)); - napi_callback cb = reinterpret_cast( - v8::Local::Cast( - _cbdata->GetInternalField(kInternalFieldIndex))->Value()); - napi_env env = static_cast( - v8::Local::Cast( - _cbdata->GetInternalField(kEnvIndex))->Value()); + // All other pointers we need are stored in `_bundle` + napi_env env = _bundle->env; + napi_callback cb = _bundle->cb[kInternalFieldIndex]; napi_value result; NAPI_CALL_INTO_MODULE_THROW(env, result = cb(env, cbinfo_wrapper)); @@ -560,7 +572,7 @@ class CallbackWrapperBase : public CallbackWrapper { } const Info& _cbinfo; - const v8::Local _cbdata; + CallbackBundle* _bundle; }; class FunctionCallbackWrapper @@ -682,25 +694,16 @@ class SetterCallbackWrapper // Creates an object to be made available to the static function callback // wrapper, used to retrieve the native callback function and data pointer. static -v8::Local CreateFunctionCallbackData(napi_env env, - napi_callback cb, - void* data) { - v8::Isolate* isolate = env->isolate; - v8::Local context = isolate->GetCurrentContext(); +v8::Local CreateFunctionCallbackData(napi_env env, + napi_callback cb, + void* data) { + CallbackBundle* bundle = new CallbackBundle(); + bundle->cb[kFunctionIndex] = cb; + bundle->cb_data = data; + bundle->env = env; + v8::Local cbdata = v8::External::New(env->isolate, bundle); + bundle->BindLifecycleTo(env->isolate, cbdata); - v8::Local otpl; - ENV_OBJECT_TEMPLATE(env, function_data, otpl, v8impl::kFunctionFieldCount); - v8::Local cbdata = otpl->NewInstance(context).ToLocalChecked(); - - cbdata->SetInternalField( - v8impl::kEnvIndex, - v8::External::New(isolate, static_cast(env))); - cbdata->SetInternalField( - v8impl::kFunctionIndex, - v8::External::New(isolate, reinterpret_cast(cb))); - cbdata->SetInternalField( - v8impl::kDataIndex, - v8::External::New(isolate, data)); return cbdata; } @@ -708,36 +711,18 @@ v8::Local CreateFunctionCallbackData(napi_env env, // callback wrapper, used to retrieve the native getter/setter callback // function and data pointer. static -v8::Local CreateAccessorCallbackData(napi_env env, - napi_callback getter, - napi_callback setter, - void* data) { - v8::Isolate* isolate = env->isolate; - v8::Local context = isolate->GetCurrentContext(); - - v8::Local otpl; - ENV_OBJECT_TEMPLATE(env, accessor_data, otpl, v8impl::kAccessorFieldCount); - v8::Local cbdata = otpl->NewInstance(context).ToLocalChecked(); - - cbdata->SetInternalField( - v8impl::kEnvIndex, - v8::External::New(isolate, static_cast(env))); - - if (getter != nullptr) { - cbdata->SetInternalField( - v8impl::kGetterIndex, - v8::External::New(isolate, reinterpret_cast(getter))); - } - - if (setter != nullptr) { - cbdata->SetInternalField( - v8impl::kSetterIndex, - v8::External::New(isolate, reinterpret_cast(setter))); - } +v8::Local CreateAccessorCallbackData(napi_env env, + napi_callback getter, + napi_callback setter, + void* data) { + CallbackBundle* bundle = new CallbackBundle(); + bundle->cb[kGetterIndex] = getter; + bundle->cb[kSetterIndex] = setter; + bundle->cb_data = data; + bundle->env = env; + v8::Local cbdata = v8::External::New(env->isolate, bundle); + bundle->BindLifecycleTo(env->isolate, cbdata); - cbdata->SetInternalField( - v8impl::kDataIndex, - v8::External::New(isolate, data)); return cbdata; } @@ -1038,7 +1023,7 @@ napi_status napi_create_function(napi_env env, v8::Isolate* isolate = env->isolate; v8::Local return_value; v8::EscapableHandleScope scope(isolate); - v8::Local cbdata = + v8::Local cbdata = v8impl::CreateFunctionCallbackData(env, cb, callback_data); RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure); @@ -1078,7 +1063,7 @@ napi_status napi_define_class(napi_env env, v8::Isolate* isolate = env->isolate; v8::EscapableHandleScope scope(isolate); - v8::Local cbdata = + v8::Local cbdata = v8impl::CreateFunctionCallbackData(env, constructor, callback_data); RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure); @@ -1114,7 +1099,7 @@ napi_status napi_define_class(napi_env env, // This code is similar to that in napi_define_properties(); the // difference is it applies to a template instead of an object. if (p->getter != nullptr || p->setter != nullptr) { - v8::Local cbdata = v8impl::CreateAccessorCallbackData( + v8::Local cbdata = v8impl::CreateAccessorCallbackData( env, p->getter, p->setter, p->data); tpl->PrototypeTemplate()->SetAccessor( @@ -1125,7 +1110,7 @@ napi_status napi_define_class(napi_env env, v8::AccessControl::DEFAULT, attributes); } else if (p->method != nullptr) { - v8::Local cbdata = + v8::Local cbdata = v8impl::CreateFunctionCallbackData(env, p->method, p->data); RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure); @@ -1487,7 +1472,7 @@ napi_status napi_define_properties(napi_env env, v8impl::V8PropertyAttributesFromDescriptor(p); if (p->getter != nullptr || p->setter != nullptr) { - v8::Local cbdata = v8impl::CreateAccessorCallbackData( + v8::Local cbdata = v8impl::CreateAccessorCallbackData( env, p->getter, p->setter, @@ -1506,7 +1491,7 @@ napi_status napi_define_properties(napi_env env, return napi_set_last_error(env, napi_invalid_arg); } } else if (p->method != nullptr) { - v8::Local cbdata = + v8::Local cbdata = v8impl::CreateFunctionCallbackData(env, p->method, p->data); RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);