timers: cross JS/C++ border less frequently

This removes the `process._needImmediateCallback` property and its semantics of having a 1/0 switch that tells C++ whether immediates are currently scheduled. Instead, a counter keeping track of all immediates is created, that can be increased on `setImmediate()` or decreased when an immediate is run or cleared. This is faster, because rather than reading/writing a C++ getter, this operation can be performed as a direct memory read/write via a typed array. The only C++ call that is left to make is activating the native handles upon creation of the first `Immediate` after the queue is empty. One other (good!) side-effect is that `immediate._destroyed` now reliably tells whether an `immediate` is still scheduled to run or not. Also, as a nice extra, this should make it easier to implement an internal variant of `setImmediate` for C++ that piggybacks off the same mechanism, which should be useful at least for async hooks and HTTP/2. Benchmark results: $ ./node benchmark/compare.js --new ./node --old ./node-master-1b093cb93df0 --runs 10 --filter immediate timers | Rscript benchmark/compare.R [00:08:53|% 100| 4/4 files | 20/20 runs | 1/1 configs]: Done improvement confidence p.value timers/immediate.js type="breadth" thousands=2000 25.61 % ** 1.432301e-03 timers/immediate.js type="breadth1" thousands=2000 7.66 % 1.320233e-01 timers/immediate.js type="breadth4" thousands=2000 4.61 % 5.669053e-01 timers/immediate.js type="clear" thousands=2000 311.40 % *** 3.896291e-07 timers/immediate.js type="depth" thousands=2000 17.54 % ** 9.755389e-03 timers/immediate.js type="depth1" thousands=2000 17.09 % *** 7.176229e-04 timers/set-immediate-breadth-args.js millions=5 10.63 % * 4.250034e-02 timers/set-immediate-breadth.js millions=10 20.62 % *** 9.150439e-07 timers/set-immediate-depth-args.js millions=10 17.97 % *** 6.819135e-10 PR-URL: #17064 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
nodejs · Nov 18, 2017 · ae558af · ae558af
1 parent ca3f9b7
commit ae558af
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 73 deletions.
diff --git a/lib/timers.js b/lib/timers.js
@@ -47,6 +47,13 @@ const { kInit, kDestroy, kAsyncIdCounter } = async_wrap.constants;
 const async_id_symbol = Symbol('asyncId');
 const trigger_async_id_symbol = Symbol('triggerAsyncId');
 
+/* This is an Uint32Array for easier sharing with C++ land. */
+const scheduledImmediateCount = process._scheduledImmediateCount;
+delete process._scheduledImmediateCount;
+/* Kick off setImmediate processing */
+const activateImmediateCheck = process._activateImmediateCheck;
+delete process._activateImmediateCheck;
+
 // Timeout values > TIMEOUT_MAX are set to 1.
 const TIMEOUT_MAX = 2147483647; // 2^31-1
 
@@ -742,15 +749,9 @@ function processImmediate() {
     else
       immediate = next;
   }
-
-  // Only round-trip to C++ land if we have to. Calling clearImmediate() on an
-  // immediate that's in |queue| is okay. Worst case is we make a superfluous
-  // call to NeedImmediateCallbackSetter().
-  if (!immediateQueue.head) {
-    process._needImmediateCallback = false;
-  }
 }
 
+process._immediateCallback = processImmediate;
 
 // An optimization so that the try/finally only de-optimizes (since at least v8
 // 4.7) what is in this smaller function.
@@ -762,13 +763,17 @@ function tryOnImmediate(immediate, oldTail) {
     runCallback(immediate);
     threw = false;
   } finally {
-    // clearImmediate checks _onImmediate === null for kDestroy hooks.
     immediate._onImmediate = null;
     if (!threw)
       emitAfter(immediate[async_id_symbol]);
-    if (async_hook_fields[kDestroy] > 0 && !immediate._destroyed) {
-      emitDestroy(immediate[async_id_symbol]);
+
+    if (!immediate._destroyed) {
       immediate._destroyed = true;
+      scheduledImmediateCount[0]--;
+
+      if (async_hook_fields[kDestroy] > 0) {
+        emitDestroy(immediate[async_id_symbol]);
+      }
     }
 
     if (threw && immediate._idleNext) {
@@ -870,10 +875,9 @@ function createImmediate(args, callback) {
   immediate._argv = args;
   immediate._onImmediate = callback;
 
-  if (!process._needImmediateCallback) {
-    process._needImmediateCallback = true;
-    process._immediateCallback = processImmediate;
-  }
+  if (scheduledImmediateCount[0] === 0)
+    activateImmediateCheck();
+  scheduledImmediateCount[0]++;
 
   immediateQueue.append(immediate);
 
@@ -884,18 +888,16 @@ function createImmediate(args, callback) {
 exports.clearImmediate = function(immediate) {
   if (!immediate) return;
 
-  if (async_hook_fields[kDestroy] > 0 &&
-      immediate._onImmediate !== null &&
-      !immediate._destroyed) {
-    emitDestroy(immediate[async_id_symbol]);
+  if (!immediate._destroyed) {
+    scheduledImmediateCount[0]--;
     immediate._destroyed = true;
+
+    if (async_hook_fields[kDestroy] > 0) {
+      emitDestroy(immediate[async_id_symbol]);
+    }
   }
 
   immediate._onImmediate = null;
 
   immediateQueue.remove(immediate);
-
-  if (!immediateQueue.head) {
-    process._needImmediateCallback = false;
-  }
 };
diff --git a/src/env-inl.h b/src/env-inl.h
@@ -283,6 +283,7 @@ inline Environment::Environment(IsolateData* isolate_data,
       abort_on_uncaught_exception_(false),
       emit_napi_warning_(true),
       makecallback_cntr_(0),
+      scheduled_immediate_count_(isolate_, 1),
 #if HAVE_INSPECTOR
       inspector_agent_(new inspector::Agent(this)),
 #endif
@@ -512,6 +513,11 @@ inline void Environment::set_fs_stats_field_array(double* fields) {
   fs_stats_field_array_ = fields;
 }
 
+inline AliasedBuffer<uint32_t, v8::Uint32Array>&
+Environment::scheduled_immediate_count() {
+  return scheduled_immediate_count_;
+}
+
 inline performance::performance_state* Environment::performance_state() {
   return performance_state_;
 }

diff --git a/src/env.h b/src/env.h
@@ -622,6 +622,8 @@ class Environment {
   inline double* fs_stats_field_array() const;
   inline void set_fs_stats_field_array(double* fields);
 
+  inline AliasedBuffer<uint32_t, v8::Uint32Array>& scheduled_immediate_count();
+
   inline performance::performance_state* performance_state();
   inline std::map<std::string, uint64_t>* performance_marks();
 
@@ -731,6 +733,8 @@ class Environment {
   size_t makecallback_cntr_;
   std::vector<double> destroy_async_id_list_;
 
+  AliasedBuffer<uint32_t, v8::Uint32Array> scheduled_immediate_count_;
+
   performance::performance_state* performance_state_ = nullptr;
   std::map<std::string, uint64_t> performance_marks_;
 

diff --git a/src/node.cc b/src/node.cc
@@ -400,25 +400,6 @@ static void PrintErrorString(const char* format, ...) {
   va_end(ap);
 }
 
-
-static void CheckImmediate(uv_check_t* handle) {
-  Environment* env = Environment::from_immediate_check_handle(handle);
-  HandleScope scope(env->isolate());
-  Context::Scope context_scope(env->context());
-  MakeCallback(env->isolate(),
-               env->process_object(),
-               env->immediate_callback_string(),
-               0,
-               nullptr,
-               {0, 0}).ToLocalChecked();
-}
-
-
-static void IdleImmediateDummy(uv_idle_t* handle) {
-  // Do nothing. Only for maintaining event loop.
-  // TODO(bnoordhuis) Maybe make libuv accept nullptr idle callbacks.
-}
-
 const char *signo_string(int signo) {
 #define SIGNO_CASE(e)  case e: return #e;
   switch (signo) {
@@ -3019,39 +3000,40 @@ static void DebugEnd(const FunctionCallbackInfo<Value>& args);
 
 namespace {
 
-void NeedImmediateCallbackGetter(Local<Name> property,
-                                 const PropertyCallbackInfo<Value>& info) {
-  Environment* env = Environment::GetCurrent(info);
-  const uv_check_t* immediate_check_handle = env->immediate_check_handle();
-  bool active = uv_is_active(
-      reinterpret_cast<const uv_handle_t*>(immediate_check_handle));
-  info.GetReturnValue().Set(active);
+bool MaybeStopImmediate(Environment* env) {
+  if (env->scheduled_immediate_count()[0] == 0) {
+    uv_check_stop(env->immediate_check_handle());
+    uv_idle_stop(env->immediate_idle_handle());
+    return true;
+  }
+  return false;
 }
 
+void CheckImmediate(uv_check_t* handle) {
+  Environment* env = Environment::from_immediate_check_handle(handle);
+  HandleScope scope(env->isolate());
+  Context::Scope context_scope(env->context());
 
-void NeedImmediateCallbackSetter(
-    Local<Name> property,
-    Local<Value> value,
-    const PropertyCallbackInfo<void>& info) {
-  Environment* env = Environment::GetCurrent(info);
+  if (MaybeStopImmediate(env))
+    return;
 
-  uv_check_t* immediate_check_handle = env->immediate_check_handle();
-  bool active = uv_is_active(
-      reinterpret_cast<const uv_handle_t*>(immediate_check_handle));
+  MakeCallback(env->isolate(),
+               env->process_object(),
+               env->immediate_callback_string(),
+               0,
+               nullptr,
+               {0, 0}).ToLocalChecked();
 
-  if (active == value->BooleanValue())
-    return;
+  MaybeStopImmediate(env);
+}
 
-  uv_idle_t* immediate_idle_handle = env->immediate_idle_handle();
 
-  if (active) {
-    uv_check_stop(immediate_check_handle);
-    uv_idle_stop(immediate_idle_handle);
-  } else {
-    uv_check_start(immediate_check_handle, CheckImmediate);
-    // Idle handle is needed only to stop the event loop from blocking in poll.
-    uv_idle_start(immediate_idle_handle, IdleImmediateDummy);
-  }
+void ActivateImmediateCheck(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  uv_check_start(env->immediate_check_handle(), CheckImmediate);
+  // Idle handle is needed only to stop the event loop from blocking in poll.
+  uv_idle_start(env->immediate_idle_handle(),
+                [](uv_idle_t*){ /* do nothing, just keep the loop running */ });
 }
 
 
@@ -3277,12 +3259,11 @@ void SetupProcessObject(Environment* env,
                              FIXED_ONE_BYTE_STRING(env->isolate(), "ppid"),
                              GetParentProcessId).FromJust());
 
-  auto need_immediate_callback_string =
-      FIXED_ONE_BYTE_STRING(env->isolate(), "_needImmediateCallback");
-  CHECK(process->SetAccessor(env->context(), need_immediate_callback_string,
-                             NeedImmediateCallbackGetter,
-                             NeedImmediateCallbackSetter,
-                             env->as_external()).FromJust());
+  auto scheduled_immediate_count =
+      FIXED_ONE_BYTE_STRING(env->isolate(), "_scheduledImmediateCount");
+  CHECK(process->Set(env->context(),
+                     scheduled_immediate_count,
+                     env->scheduled_immediate_count().GetJSArray()).FromJust());
 
   // -e, --eval
   if (eval_string) {
@@ -3408,6 +3389,9 @@ void SetupProcessObject(Environment* env,
                              env->as_external()).FromJust());
 
   // define various internal methods
+  env->SetMethod(process,
+                 "_activateImmediateCheck",
+                 ActivateImmediateCheck);
   env->SetMethod(process,
                  "_startProfilerIdleNotifier",
                  StartProfilerIdleNotifier);