From ae558af7bc1db217eb1c368c8b5e5773910232c3 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 16 Nov 2017 00:43:12 +0100 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/17064 Reviewed-By: Refael Ackermann Reviewed-By: Minwoo Jung Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis --- lib/timers.js | 46 ++++++++++++++------------- src/env-inl.h | 6 ++++ src/env.h | 4 +++ src/node.cc | 86 +++++++++++++++++++++------------------------------ 4 files changed, 69 insertions(+), 73 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index ce27be69450249..56f02fe1ea644c 100644 --- 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 index 4dadf5ea2c1978..f7a7559b3f2818 100644 --- 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& +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 index 3276a4b9a75ada..862431df2ddab4 100644 --- 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& scheduled_immediate_count(); + inline performance::performance_state* performance_state(); inline std::map* performance_marks(); @@ -731,6 +733,8 @@ class Environment { size_t makecallback_cntr_; std::vector destroy_async_id_list_; + AliasedBuffer scheduled_immediate_count_; + performance::performance_state* performance_state_ = nullptr; std::map performance_marks_; diff --git a/src/node.cc b/src/node.cc index a1d5ad7e254469..81ea92b7e20b1d 100644 --- 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& args); namespace { -void NeedImmediateCallbackGetter(Local property, - const PropertyCallbackInfo& info) { - Environment* env = Environment::GetCurrent(info); - const uv_check_t* immediate_check_handle = env->immediate_check_handle(); - bool active = uv_is_active( - reinterpret_cast(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 property, - Local value, - const PropertyCallbackInfo& 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(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& 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);