napi: improve runtime performance of every napi fun call.

Added a new struct CallbackBundle to reduce the number of
GetInternalField() calls -- was: 3; now: 1.

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 is 3 GetInternalField() calls;
after this change, there will be only 1 GetInternalField() call.

Profiling data show that GetInternalField() is slow.
On i7-4770K (3.50GHz), 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 22 ns.

The above data are measured using a new benchmark in
'benchmark/misc/napi_function_call'. This new benchmark measures
the average delay of a 'chatty' napi fun call (max 50M runs).
A simple C++ binding function (e.g. returning a number) is called
'chatty' case for JS<-->napi fun call. This change will speed up
chatty case 1.6x (overall), and will cut down the delay of napi
mechanism to approx. 0.5x

This improvement also applies to getter/setter fun calls.

Makefile

@@ -283,6 +283,14 @@ benchmark/misc/function_call/build/Release/binding.node: all \
 		--directory="$(shell pwd)/benchmark/misc/function_call" \
 		--nodedir="$(shell pwd)"
 
+benchmark/misc/napi_function_call/build/Release/binding.node: all \
+		benchmark/misc/napi_function_call/binding.cc \
+		benchmark/misc/napi_function_call/binding.gyp
+	$(NODE) deps/npm/node_modules/node-gyp/bin/node-gyp rebuild \
+		--python="$(PYTHON)" \
+		--directory="$(shell pwd)/benchmark/misc/napi_function_call" \
+		--nodedir="$(shell pwd)"
+
 # Implicitly depends on $(NODE_EXE).  We don't depend on it explicitly because
 # it always triggers a rebuild due to it being a .PHONY rule.  See the comment
 # near the build-addons rule for more background.

benchmark/misc/napi_function_call/.gitignore

@@ -0,0 +1 @@
+build/

benchmark/misc/napi_function_call/binding.cc

@@ -0,0 +1,25 @@
+#include <node_api.h>
+#include <assert.h>
+
+static int32_t counter = 0;
+
+napi_value Hello(napi_env env, napi_callback_info info) {
+  napi_status status;
+  napi_value value;
+  napi_create_int32(env, counter++, &value);
+  assert(status == napi_ok);
+  return value;
+}
+
+#define DECLARE_NAPI_METHOD(name, func)                          \
+  { name, 0, func, 0, 0, 0, napi_default, 0 }
+
+napi_value Init(napi_env env, napi_value exports) {
+  napi_status status;
+  napi_property_descriptor desc = DECLARE_NAPI_METHOD("hello", Hello);
+  status = napi_define_properties(env, exports, 1, &desc);
+  assert(status == napi_ok);
+  return exports;
+}
+
+NAPI_MODULE(NODE_GYP_MODULE_NAME, Init)

benchmark/misc/napi_function_call/binding.gyp

@@ -0,0 +1,8 @@
+{
+  "targets": [
+    {
+      "target_name": "binding",
+      "sources": [ "binding.cc" ]
+    }
+  ]
+}

benchmark/misc/napi_function_call/index.js

@@ -0,0 +1,41 @@
+// show the difference between calling a short js function
+// relative to a comparable C++ function.
+// Reports n of calls per second.
+// Note that JS speed goes up, while cxx speed stays about the same.
+'use strict';
+
+const assert = require('assert');
+const common = require('../../common.js');
+
+// this fails when we try to open with a different version of node,
+// which is quite common for benchmarks.  so in that case, just
+// abort quietly.
+
+try {
+  var binding = require('./build/Release/binding');
+} catch (er) {
+  console.error('misc/napi_function_call/index.js Binding failed to load');
+  process.exit(0);
+}
+const cxx = binding.hello;
+
+var c = 0;
+function js() {
+  return c++;
+}
+
+assert(js() === cxx());
+
+const bench = common.createBenchmark(main, {
+  type: ['js', 'cxx(napi)'],
+  n: [1e6, 1e7, 5e7]
+});
+
+function main({ n, type }) {
+  const fn = type === 'cxx(napi)' ? cxx : js;
+  bench.start();
+  for (var i = 0; i < n; i++) {
+    fn();
+  }
+  bench.end(n);
+}

src/node_api.cc

@@ -492,15 +492,51 @@ class TryCatch : public v8::TryCatch {
 
 //=== Function napi_callback wrapper =================================
 
-static const int kDataIndex = 0;
-static const int kEnvIndex = 1;
+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
+
+static const int kCallbackBundleIndex = 0;  // The first and the only one
+static const int kInternalFieldCount = 1;
+
+// Use this data structure to reduce the number
+// of GetInternalField() calls to only 1 (was: 3).
+// This leads to better performance in runtime.
+// Ref: benchmark/misc/napi_function_call
+struct CallbackBundle {
+  ~CallbackBundle() {
+    if (handle.IsEmpty()) {
+      return;
+    }
+
+    handle.ClearWeak();
+    handle.Reset();
+  }
+
+  // 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::Object> obj) {
+    handle.Reset(isolate, obj);
+    handle.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
+  }
 
-static const int kFunctionIndex = 2;
-static const int kFunctionFieldCount = 3;
+  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::Object> handle;  // Die with this JavaScript object
 
-static const int kGetterIndex = 2;
-static const int kSetterIndex = 3;
-static const int kAccessorFieldCount = 4;
+ private:
+  static void WeakCallback(v8::WeakCallbackInfo<CallbackBundle> const& info) {
+    // Use WeakCallback mechanism to delete the C++ `bundle` object.
+    // This will be called when object in `handle` is being GC-ed.
+    if (CallbackBundle* bundle = info.GetParameter()) {
+      bundle->handle.Reset();
+      delete bundle;
+    }
+  }
+};
 
 // Base class extended by classes that wrap V8 function and property callback
 // info.
@@ -534,8 +570,11 @@ class CallbackWrapperBase : public CallbackWrapper {
                         nullptr),
         _cbinfo(cbinfo),
         _cbdata(v8::Local<v8::Object>::Cast(cbinfo.Data())) {
-    _data = v8::Local<v8::External>::Cast(_cbdata->GetInternalField(kDataIndex))
-                ->Value();
+    // Note that there is no way we can tell whether `_bundle` is legit
+    _bundle = reinterpret_cast<CallbackBundle*>(
+        v8::Local<v8::External>::Cast(
+            _cbdata->GetInternalField(kCallbackBundleIndex))->Value());
+    _data = _bundle->cb_data;
   }
 
   napi_value GetNewTarget() override { return nullptr; }
@@ -544,14 +583,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());
 
+    // Now we just use the pointers 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));
 
@@ -562,6 +597,8 @@ class CallbackWrapperBase : public CallbackWrapper {
 
   const Info& _cbinfo;
   const v8::Local<v8::Object> _cbdata;
+  // Note: the deletion of `_bundle` is with the the GC of _cbdata object
+  CallbackBundle* _bundle;
 };
 
 class FunctionCallbackWrapper
@@ -690,18 +727,19 @@ v8::Local<v8::Object> CreateFunctionCallbackData(napi_env env,
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
 
   v8::Local<v8::ObjectTemplate> otpl;
-  ENV_OBJECT_TEMPLATE(env, function_data, otpl, v8impl::kFunctionFieldCount);
+  ENV_OBJECT_TEMPLATE(env, function_data, otpl, v8impl::kInternalFieldCount);
   v8::Local<v8::Object> cbdata = otpl->NewInstance(context).ToLocalChecked();
 
+  CallbackBundle* bundle = new CallbackBundle();
+  bundle->cb[kFunctionIndex] = cb;
+  bundle->cb_data = data;
+  bundle->env = env;
+  bundle->BindLifecycleTo(env->isolate, cbdata);
+
   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));
+      v8impl::kCallbackBundleIndex,
+      v8::External::New(isolate, reinterpret_cast<void*>(bundle)));
+
   return cbdata;
 }
 
@@ -717,28 +755,20 @@ v8::Local<v8::Object> CreateAccessorCallbackData(napi_env env,
   v8::Local<v8::Context> context = isolate->GetCurrentContext();
 
   v8::Local<v8::ObjectTemplate> otpl;
-  ENV_OBJECT_TEMPLATE(env, accessor_data, otpl, v8impl::kAccessorFieldCount);
+  ENV_OBJECT_TEMPLATE(env, accessor_data, otpl, v8impl::kInternalFieldCount);
   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)));
-  }
+  CallbackBundle* bundle = new CallbackBundle();
+  bundle->cb[kGetterIndex] = getter;
+  bundle->cb[kSetterIndex] = setter;
+  bundle->cb_data = data;
+  bundle->env = env;
+  bundle->BindLifecycleTo(env->isolate, cbdata);
 
   cbdata->SetInternalField(
-      v8impl::kDataIndex,
-      v8::External::New(isolate, data));
+      v8impl::kCallbackBundleIndex,
+      v8::External::New(isolate, reinterpret_cast<void*>(bundle)));
+
   return cbdata;
 }