Skip to content

Commit

Permalink
n-api: improve runtime perf of n-api func call
Browse files Browse the repository at this point in the history 
Added a new struct CallbackBundle to eliminate all
GetInternalField() calls.

The principle is to store all required data inside a C++ struct,
and then store the pointer in the JavaScript object. Before this
change, the required data are stored in the JavaScript object in
3 or 4 seperate pointers. For every napi fun call, 3 of them
have to be fetched out, which are 3 GetInternalField() calls;
after this change, the C++ struct will be directly fetched out
by using v8::External::Value(), which is faster.

Profiling data show that GetInternalField() is slow.
On an i7-4770K (3.50GHz) box, a C++ V8-binding fun call is 8 ns,
before this change, napi fun call is 36 ns; after this change,
napi fun call is 20 ns.

The above data are measured using a modified benchmark in
'benchmark/misc/function_call'. The modification adds an indicator
of the average time of a "chatty" napi fun call (max 50M runs).
This change will speed up chatty case 1.8x (overall), and will cut
down the delay of napi mechanism to approx. 0.5x.

Background: a simple C++ binding function (e.g. receiving little
from JS, doing little and returning little to JS) is called
'chatty' case for JS<-->C++ fun call routine.

This improvement also applies to getter/setter fun calls.

Backport-PR-URL: #21733
PR-URL: #21072
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
  • Loading branch information
@kenny-y @MylesBorins
kenny-y authored and MylesBorins committed Aug 1, 2018
1 parent 093b777 commit 5026ab4
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 92 deletions.
1 change: 1 addition & 0 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ test-check-deopts: all
$(PYTHON) tools/test.py --mode=release --check-deopts parallel sequential -J

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 \
Expand Down
4 changes: 4 additions & 0 deletions benchmark/misc/function_call/binding.gyp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
{
'targets': [
{
'target_name': 'napi_binding',
'sources': [ 'napi_binding.c' ]
},
{
'target_name': 'binding',
'sources': [ 'binding.cc' ]
Expand Down
13 changes: 11 additions & 2 deletions benchmark/misc/function_call/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
Expand All @@ -27,14 +36,14 @@ function js() {
assert(js() === cxx());

const bench = common.createBenchmark(main, {
type: ['js', 'cxx'],
type: ['js', 'cxx', 'napi'],
millions: [1, 10, 50]
});

function main(conf) {
const n = +conf.millions * 1e6;

const fn = conf.type === 'cxx' ? cxx : js;
const fn = conf.type === 'cxx' ? cxx : conf.type === 'napi' ? napi : js;
bench.start();
for (var i = 0; i < n; i++) {
fn();
Expand Down
28 changes: 28 additions & 0 deletions benchmark/misc/function_call/napi_binding.c
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#include <assert.h>
#include <node_api.h>

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;
}

static napi_value Init(napi_env env, napi_value exports) {
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;
}

NAPI_MODULE(NODE_GYP_MODULE_NAME, Init)
167 changes: 77 additions & 90 deletions src/node_api.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,9 @@ struct napi_env__ {
loop(_loop) {}
~napi_env__() {
last_exception.Reset();
wrap_template.Reset();
function_data_template.Reset();
accessor_data_template.Reset();
}
v8::Isolate* isolate;
v8::Persistent<v8::Value> last_exception;
v8::Persistent<v8::ObjectTemplate> wrap_template;
v8::Persistent<v8::ObjectTemplate> function_data_template;
v8::Persistent<v8::ObjectTemplate> accessor_data_template;
napi_extended_error_info last_error;
int open_handle_scopes = 0;
int open_callback_scopes = 0;
Expand All @@ -41,19 +35,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<v8::ObjectTemplate>::New( \
isolate, env->prefix ## _template); \
} \
} while (0)


#define RETURN_STATUS_IF_FALSE(env, condition, status) \
do { \
if (!(condition)) { \
Expand Down Expand Up @@ -510,15 +491,51 @@ 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
class CallbackBundle {
public:
~CallbackBundle() {
handle.ClearWeak();
handle.Reset();
}

static const int kFunctionIndex = 2;
static const int kFunctionFieldCount = 3;
// 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<v8::Value> target) {
handle.Reset(isolate, target);
handle.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
}

static const int kGetterIndex = 2;
static const int kSetterIndex = 3;
static const int kAccessorFieldCount = 4;
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)
v8::Persistent<v8::Value> handle; // Die with this JavaScript object

private:
static void WeakCallback(v8::WeakCallbackInfo<CallbackBundle> 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.
Expand Down Expand Up @@ -550,10 +567,10 @@ class CallbackWrapperBase : public CallbackWrapper {
: CallbackWrapper(JsValueFromV8LocalValue(cbinfo.This()),
args_length,
nullptr),
_cbinfo(cbinfo),
_cbdata(v8::Local<v8::Object>::Cast(cbinfo.Data())) {
_data = v8::Local<v8::External>::Cast(_cbdata->GetInternalField(kDataIndex))
->Value();
_cbinfo(cbinfo) {
_bundle = reinterpret_cast<CallbackBundle*>(
v8::Local<v8::External>::Cast(cbinfo.Data())->Value());
_data = _bundle->cb_data;
}

napi_value GetNewTarget() override { return nullptr; }
Expand All @@ -562,13 +579,10 @@ class CallbackWrapperBase : public CallbackWrapper {
void InvokeCallback() {
napi_callback_info cbinfo_wrapper = reinterpret_cast<napi_callback_info>(
static_cast<CallbackWrapper*>(this));
napi_callback cb = reinterpret_cast<napi_callback>(
v8::Local<v8::External>::Cast(
_cbdata->GetInternalField(kInternalFieldIndex))->Value());

napi_env env = static_cast<napi_env>(
v8::Local<v8::External>::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));
Expand All @@ -579,7 +593,7 @@ class CallbackWrapperBase : public CallbackWrapper {
}

const Info& _cbinfo;
const v8::Local<v8::Object> _cbdata;
CallbackBundle* _bundle;
};

class FunctionCallbackWrapper
Expand Down Expand Up @@ -701,62 +715,35 @@ 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<v8::Object> CreateFunctionCallbackData(napi_env env,
napi_callback cb,
void* data) {
v8::Isolate* isolate = env->isolate;
v8::Local<v8::Context> context = isolate->GetCurrentContext();
v8::Local<v8::Value> 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<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
bundle->BindLifecycleTo(env->isolate, cbdata);

v8::Local<v8::ObjectTemplate> otpl;
ENV_OBJECT_TEMPLATE(env, function_data, otpl, v8impl::kFunctionFieldCount);
v8::Local<v8::Object> cbdata = otpl->NewInstance(context).ToLocalChecked();

cbdata->SetInternalField(
v8impl::kEnvIndex,
v8::External::New(isolate, static_cast<void*>(env)));
cbdata->SetInternalField(
v8impl::kFunctionIndex,
v8::External::New(isolate, reinterpret_cast<void*>(cb)));
cbdata->SetInternalField(
v8impl::kDataIndex,
v8::External::New(isolate, data));
return cbdata;
}

// Creates an object to be made available to the static getter/setter
// callback wrapper, used to retrieve the native getter/setter callback
// function and data pointer.
static
v8::Local<v8::Object> CreateAccessorCallbackData(napi_env env,
napi_callback getter,
napi_callback setter,
void* data) {
v8::Isolate* isolate = env->isolate;
v8::Local<v8::Context> context = isolate->GetCurrentContext();

v8::Local<v8::ObjectTemplate> otpl;
ENV_OBJECT_TEMPLATE(env, accessor_data, otpl, v8impl::kAccessorFieldCount);
v8::Local<v8::Object> cbdata = otpl->NewInstance(context).ToLocalChecked();

cbdata->SetInternalField(
v8impl::kEnvIndex,
v8::External::New(isolate, static_cast<void*>(env)));

if (getter != nullptr) {
cbdata->SetInternalField(
v8impl::kGetterIndex,
v8::External::New(isolate, reinterpret_cast<void*>(getter)));
}

if (setter != nullptr) {
cbdata->SetInternalField(
v8impl::kSetterIndex,
v8::External::New(isolate, reinterpret_cast<void*>(setter)));
}
v8::Local<v8::Value> 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<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
bundle->BindLifecycleTo(env->isolate, cbdata);

cbdata->SetInternalField(
v8impl::kDataIndex,
v8::External::New(isolate, data));
return cbdata;
}

Expand Down Expand Up @@ -1025,7 +1012,7 @@ napi_status napi_create_function(napi_env env,
v8::Isolate* isolate = env->isolate;
v8::Local<v8::Function> return_value;
v8::EscapableHandleScope scope(isolate);
v8::Local<v8::Object> cbdata =
v8::Local<v8::Value> cbdata =
v8impl::CreateFunctionCallbackData(env, cb, callback_data);

RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
Expand Down Expand Up @@ -1065,7 +1052,7 @@ napi_status napi_define_class(napi_env env,
v8::Isolate* isolate = env->isolate;

v8::EscapableHandleScope scope(isolate);
v8::Local<v8::Object> cbdata =
v8::Local<v8::Value> cbdata =
v8impl::CreateFunctionCallbackData(env, constructor, callback_data);

RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
Expand Down Expand Up @@ -1101,7 +1088,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<v8::Object> cbdata = v8impl::CreateAccessorCallbackData(
v8::Local<v8::Value> cbdata = v8impl::CreateAccessorCallbackData(
env, p->getter, p->setter, p->data);

tpl->PrototypeTemplate()->SetAccessor(
Expand All @@ -1112,7 +1099,7 @@ napi_status napi_define_class(napi_env env,
v8::AccessControl::DEFAULT,
attributes);
} else if (p->method != nullptr) {
v8::Local<v8::Object> cbdata =
v8::Local<v8::Value> cbdata =
v8impl::CreateFunctionCallbackData(env, p->method, p->data);

RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
Expand Down Expand Up @@ -1474,7 +1461,7 @@ napi_status napi_define_properties(napi_env env,
v8impl::V8PropertyAttributesFromDescriptor(p);

if (p->getter != nullptr || p->setter != nullptr) {
v8::Local<v8::Object> cbdata = v8impl::CreateAccessorCallbackData(
v8::Local<v8::Value> cbdata = v8impl::CreateAccessorCallbackData(
env,
p->getter,
p->setter,
Expand All @@ -1493,7 +1480,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<v8::Object> cbdata =
v8::Local<v8::Value> cbdata =
v8impl::CreateFunctionCallbackData(env, p->method, p->data);

RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
Expand Down

0 comments on commit 5026ab4

Please sign in to comment.