node-api: find the Environment once

Without an async context, napi_make_callback went through the public
node::MakeCallback(), which looks up the Environment from the creation
context of the callback on every call. The napi_env already knows it,
so call into it directly, as the path with an async context does.
node_napi_env__::node_env() also looked it up from the context on every
call: find it once, a napi_env lives as long as its Environment.

Add a benchmark, napi/make_callback_napi.

Refs: nodejs/performance#24
Signed-off-by: Nigro Simone <nigro.simone@gmail.com>

Author: nigrosimone

benchmark/napi/make_callback_napi/binding.c

@@ -0,0 +1,65 @@
+#include <node_api.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <uv.h>
+
+typedef struct {
+  uv_timer_t timer;
+  napi_env env;
+  int64_t n;
+  napi_ref fn;
+  napi_ref done;
+} State;
+
+static void OnClose(uv_handle_t* handle) {
+  free(handle->data);
+}
+
+// napi_make_callback without async context, n times from a libuv timer, then
+// done: every call opens a top-level callback scope, like an I/O callback.
+static void OnTimer(uv_timer_t* handle) {
+  State* state = (State*) handle->data;
+  napi_env env = state->env;
+  napi_handle_scope scope;
+  napi_value fn, done, recv;
+  napi_open_handle_scope(env, &scope);
+  napi_get_reference_value(env, state->fn, &fn);
+  napi_get_reference_value(env, state->done, &done);
+  napi_get_global(env, &recv);
+  for (int64_t i = 0; i < state->n; i++) {
+    napi_handle_scope inner;
+    napi_open_handle_scope(env, &inner);
+    napi_make_callback(env, NULL, recv, fn, 0, NULL, NULL);
+    napi_close_handle_scope(env, inner);
+  }
+  napi_make_callback(env, NULL, recv, done, 0, NULL, NULL);
+  napi_delete_reference(env, state->fn);
+  napi_delete_reference(env, state->done);
+  napi_close_handle_scope(env, scope);
+  uv_close((uv_handle_t*) &state->timer, OnClose);
+}
+
+// run(n, fn, done)
+static napi_value Run(napi_env env, napi_callback_info info) {
+  size_t argc = 3;
+  napi_value argv[3];
+  napi_get_cb_info(env, info, &argc, argv, NULL, NULL);
+  State* state = (State*) calloc(1, sizeof(State));
+  state->env = env;
+  napi_get_value_int64(env, argv[0], &state->n);
+  napi_create_reference(env, argv[1], 1, &state->fn);
+  napi_create_reference(env, argv[2], 1, &state->done);
+  uv_loop_t* loop;
+  napi_get_uv_event_loop(env, &loop);
+  state->timer.data = state;
+  uv_timer_init(loop, &state->timer);
+  uv_timer_start(&state->timer, OnTimer, 0, 0);
+  return NULL;
+}
+
+NAPI_MODULE_INIT() {
+  napi_value run;
+  napi_create_function(env, "run", NAPI_AUTO_LENGTH, Run, NULL, &run);
+  napi_set_named_property(env, exports, "run", run);
+  return exports;
+}

benchmark/napi/make_callback_napi/binding.gyp

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

benchmark/napi/make_callback_napi/index.js

@@ -0,0 +1,23 @@
+'use strict';
+
+const common = require('../../common.js');
+
+// napi_make_callback without async context from a libuv timer, so every call
+// opens a top-level callback scope, like an I/O callback does.
+
+let binding;
+try {
+  binding = require(`./build/${common.buildType}/binding`);
+} catch {
+  console.error('napi/make_callback_napi/index.js Binding failed to load');
+  process.exit(0);
+}
+
+const bench = common.createBenchmark(main, {
+  n: [1e6, 1e7],
+});
+
+function main({ n }) {
+  bench.start();
+  binding.run(n, () => {}, () => bench.end(n));
+}

src/api/callback.cc

@@ -348,13 +348,24 @@ MaybeLocal<Value> InternalMakeCallback(Isolate* isolate,
   }
   Environment* env = Environment::GetCurrent(context);
   CHECK_NOT_NULL(env);
+  return MakeCallbackInEnvironment(
+      env, recv, callback, argc, argv, asyncContext, context_frame);
+}
+
+MaybeLocal<Value> MakeCallbackInEnvironment(Environment* env,
+                                            Local<Object> recv,
+                                            const Local<Function> callback,
+                                            int argc,
+                                            Local<Value> argv[],
+                                            async_context asyncContext,
+                                            Local<Value> context_frame) {
   Context::Scope context_scope(env->context());
   MaybeLocal<Value> ret = InternalMakeCallback(
       env, recv, recv, callback, argc, argv, asyncContext, context_frame);
   if (ret.IsEmpty() && env->async_callback_scope_depth() == 0) {
     // This is only for legacy compatibility and we may want to look into
     // removing/adjusting it.
-    return Undefined(isolate);
+    return Undefined(env->isolate());
   }
   return ret;
 }

src/node_api.cc

@@ -71,7 +71,9 @@ static void ThrowNodeApiVersionError(node::Environment* node_env,
 node_napi_env__::node_napi_env__(v8::Local<v8::Context> context,
                                  const std::string& module_filename,
                                  int32_t module_api_version)
-    : napi_env__(context, module_api_version), filename(module_filename) {
+    : napi_env__(context, module_api_version),
+      filename(module_filename),
+      node_env_(node::Environment::GetCurrent(context)) {
   CHECK_NOT_NULL(node_env());
 }
 
@@ -1026,13 +1028,15 @@ napi_status NAPI_CDECL napi_make_callback(napi_env env,
   v8::MaybeLocal<v8::Value> callback_result;
 
   if (async_context == nullptr) {
-    callback_result = node::MakeCallback(
-        env->isolate,
+    // The Environment of the napi_env, not looked up from the callback again.
+    callback_result = node::MakeCallbackInEnvironment(
+        reinterpret_cast<node_napi_env>(env)->node_env(),
         v8recv,
         v8func,
         argc,
         reinterpret_cast<v8::Local<v8::Value>*>(const_cast<napi_value*>(argv)),
-        {0, 0});
+        {0, 0},
+        v8::Undefined(env->isolate));
   } else {
     v8impl::AsyncContext* node_async_context =
         reinterpret_cast<v8impl::AsyncContext*>(async_context);

src/node_api_internals.h

@@ -33,12 +33,12 @@ struct node_napi_env__ : public napi_env__ {
 
   void DeleteMe() override;
 
-  inline node::Environment* node_env() const {
-    return node::Environment::GetCurrent(context());
-  }
+  // Found once: a napi_env lives as long as its Environment.
+  inline node::Environment* node_env() const { return node_env_; }
   inline const char* GetFilename() const { return filename.c_str(); }
 
   std::string filename;
+  node::Environment* node_env_;
   bool destructing = false;
   bool finalization_scheduled = false;
 };

src/node_internals.h

@@ -230,6 +230,16 @@ v8::MaybeLocal<v8::Value> InternalMakeCallback(
     async_context asyncContext,
     v8::Local<v8::Value> context_frame);
 
+// The same, for a caller that already knows the Environment of `callback`.
+v8::MaybeLocal<v8::Value> MakeCallbackInEnvironment(
+    Environment* env,
+    v8::Local<v8::Object> recv,
+    const v8::Local<v8::Function> callback,
+    int argc,
+    v8::Local<v8::Value> argv[],
+    async_context asyncContext,
+    v8::Local<v8::Value> context_frame);
+
 v8::MaybeLocal<v8::Value> MakeSyncCallback(v8::Isolate* isolate,
                                            v8::Local<v8::Object> recv,
                                            v8::Local<v8::Function> callback,