FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

timers: cross JS/C++ border less frequently · nodejs/node@fd501b3 · GitHub

/ node Public

Commit fd501b3

Browse files
addaleax
authored and
MylesBorins
committed
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>
1 parent 9f282dd commit fd501b3

4 files changed

Lines changed: 69 additions & 73 deletions

File tree

‎lib/timers.js‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ const { kInit, kDestroy, kAsyncIdCounter } = async_wrap.constants;
4747
const async_id_symbol = Symbol('asyncId');
4848
const trigger_async_id_symbol = Symbol('triggerAsyncId');
4949

50+
/* This is an Uint32Array for easier sharing with C++ land. */
51+
const scheduledImmediateCount = process._scheduledImmediateCount;
52+
delete process._scheduledImmediateCount;
53+
/* Kick off setImmediate processing */
54+
const activateImmediateCheck = process._activateImmediateCheck;
55+
delete process._activateImmediateCheck;
56+
5057
// Timeout values > TIMEOUT_MAX are set to 1.
5158
const TIMEOUT_MAX = 2147483647; // 2^31-1
5259

@@ -742,15 +749,9 @@ function processImmediate() {
742749
else
743750
immediate = next;
744751
}
745-
746-
// Only round-trip to C++ land if we have to. Calling clearImmediate() on an
747-
// immediate that's in |queue| is okay. Worst case is we make a superfluous
748-
// call to NeedImmediateCallbackSetter().
749-
if (!immediateQueue.head) {
750-
process._needImmediateCallback = false;
751-
}
752752
}
753753

754+
process._immediateCallback = processImmediate;
754755

755756
// An optimization so that the try/finally only de-optimizes (since at least v8
756757
// 4.7) what is in this smaller function.
@@ -762,13 +763,17 @@ function tryOnImmediate(immediate, oldTail) {
762763
runCallback(immediate);
763764
threw = false;
764765
} finally {
765-
// clearImmediate checks _onImmediate === null for kDestroy hooks.
766766
immediate._onImmediate = null;
767767
if (!threw)
768768
emitAfter(immediate[async_id_symbol]);
769-
if (async_hook_fields[kDestroy] > 0 && !immediate._destroyed) {
770-
emitDestroy(immediate[async_id_symbol]);
769+
770+
if (!immediate._destroyed) {
771771
immediate._destroyed = true;
772+
scheduledImmediateCount[0]--;
773+
774+
if (async_hook_fields[kDestroy] > 0) {
775+
emitDestroy(immediate[async_id_symbol]);
776+
}
772777
}
773778

774779
if (threw && immediate._idleNext) {
@@ -870,10 +875,9 @@ function createImmediate(args, callback) {
870875
immediate._argv = args;
871876
immediate._onImmediate = callback;
872877

873-
if (!process._needImmediateCallback) {
874-
process._needImmediateCallback = true;
875-
process._immediateCallback = processImmediate;
876-
}
878+
if (scheduledImmediateCount[0] === 0)
879+
activateImmediateCheck();
880+
scheduledImmediateCount[0]++;
877881

878882
immediateQueue.append(immediate);
879883

@@ -884,18 +888,16 @@ function createImmediate(args, callback) {
884888
exports.clearImmediate = function(immediate) {
885889
if (!immediate) return;
886890

887-
if (async_hook_fields[kDestroy] > 0 &&
888-
immediate._onImmediate !== null &&
889-
!immediate._destroyed) {
890-
emitDestroy(immediate[async_id_symbol]);
891+
if (!immediate._destroyed) {
892+
scheduledImmediateCount[0]--;
891893
immediate._destroyed = true;
894+
895+
if (async_hook_fields[kDestroy] > 0) {
896+
emitDestroy(immediate[async_id_symbol]);
897+
}
892898
}
893899

894900
immediate._onImmediate = null;
895901

896902
immediateQueue.remove(immediate);
897-
898-
if (!immediateQueue.head) {
899-
process._needImmediateCallback = false;
900-
}
901903
};

‎src/env-inl.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ inline Environment::Environment(IsolateData* isolate_data,
267267
abort_on_uncaught_exception_(false),
268268
emit_napi_warning_(true),
269269
makecallback_cntr_(0),
270+
scheduled_immediate_count_(isolate_, 1),
270271
#if HAVE_INSPECTOR
271272
inspector_agent_(new inspector::Agent(this)),
272273
#endif
@@ -486,6 +487,11 @@ inline void Environment::set_fs_stats_field_array(double* fields) {
486487
fs_stats_field_array_ = fields;
487488
}
488489

490+
inline AliasedBuffer<uint32_t, v8::Uint32Array>&
491+
Environment::scheduled_immediate_count() {
492+
return scheduled_immediate_count_;
493+
}
494+
489495
inline performance::performance_state* Environment::performance_state() {
490496
return performance_state_;
491497
}

‎src/env.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,8 @@ class Environment {
601601
inline double* fs_stats_field_array() const;
602602
inline void set_fs_stats_field_array(double* fields);
603603

604+
inline AliasedBuffer<uint32_t, v8::Uint32Array>& scheduled_immediate_count();
605+
604606
inline performance::performance_state* performance_state();
605607
inline std::map<std::string, uint64_t>* performance_marks();
606608

@@ -696,6 +698,8 @@ class Environment {
696698
size_t makecallback_cntr_;
697699
std::vector<double> destroy_async_id_list_;
698700

701+
AliasedBuffer<uint32_t, v8::Uint32Array> scheduled_immediate_count_;
702+
699703
performance::performance_state* performance_state_ = nullptr;
700704
std::map<std::string, uint64_t> performance_marks_;
701705

‎src/node.cc‎

Lines changed: 35 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -399,25 +399,6 @@ static void PrintErrorString(const char* format, ...) {
399399
}
400400

401401

402-
static void CheckImmediate(uv_check_t* handle) {
403-
Environment* env = Environment::from_immediate_check_handle(handle);
404-
HandleScope scope(env->isolate());
405-
Context::Scope context_scope(env->context());
406-
MakeCallback(env->isolate(),
407-
env->process_object(),
408-
env->immediate_callback_string(),
409-
0,
410-
nullptr,
411-
{0, 0}).ToLocalChecked();
412-
}
413-
414-
415-
static void IdleImmediateDummy(uv_idle_t* handle) {
416-
// Do nothing. Only for maintaining event loop.
417-
// TODO(bnoordhuis) Maybe make libuv accept nullptr idle callbacks.
418-
}
419-
420-
421402
static inline const char *errno_string(int errorno) {
422403
#define ERRNO_CASE(e) case e: return #e;
423404
switch (errorno) {
@@ -3274,39 +3255,40 @@ static void DebugEnd(const FunctionCallbackInfo<Value>& args);
32743255

32753256
namespace {
32763257

3277-
void NeedImmediateCallbackGetter(Local<Name> property,
3278-
const PropertyCallbackInfo<Value>& info) {
3279-
Environment* env = Environment::GetCurrent(info);
3280-
const uv_check_t* immediate_check_handle = env->immediate_check_handle();
3281-
bool active = uv_is_active(
3282-
reinterpret_cast<const uv_handle_t*>(immediate_check_handle));
3283-
info.GetReturnValue().Set(active);
3258+
bool MaybeStopImmediate(Environment* env) {
3259+
if (env->scheduled_immediate_count()[0] == 0) {
3260+
uv_check_stop(env->immediate_check_handle());
3261+
uv_idle_stop(env->immediate_idle_handle());
3262+
return true;
3263+
}
3264+
return false;
32843265
}
32853266

3267+
void CheckImmediate(uv_check_t* handle) {
3268+
Environment* env = Environment::from_immediate_check_handle(handle);
3269+
HandleScope scope(env->isolate());
3270+
Context::Scope context_scope(env->context());
32863271

3287-
void NeedImmediateCallbackSetter(
3288-
Local<Name> property,
3289-
Local<Value> value,
3290-
const PropertyCallbackInfo<void>& info) {
3291-
Environment* env = Environment::GetCurrent(info);
3272+
if (MaybeStopImmediate(env))
3273+
return;
32923274

3293-
uv_check_t* immediate_check_handle = env->immediate_check_handle();
3294-
bool active = uv_is_active(
3295-
reinterpret_cast<const uv_handle_t*>(immediate_check_handle));
3275+
MakeCallback(env->isolate(),
3276+
env->process_object(),
3277+
env->immediate_callback_string(),
3278+
0,
3279+
nullptr,
3280+
{0, 0}).ToLocalChecked();
32963281

3297-
if (active == value->BooleanValue())
3298-
return;
3282+
MaybeStopImmediate(env);
3283+
}
32993284

3300-
uv_idle_t* immediate_idle_handle = env->immediate_idle_handle();
33013285

3302-
if (active) {
3303-
uv_check_stop(immediate_check_handle);
3304-
uv_idle_stop(immediate_idle_handle);
3305-
} else {
3306-
uv_check_start(immediate_check_handle, CheckImmediate);
3307-
// Idle handle is needed only to stop the event loop from blocking in poll.
3308-
uv_idle_start(immediate_idle_handle, IdleImmediateDummy);
3309-
}
3286+
void ActivateImmediateCheck(const FunctionCallbackInfo<Value>& args) {
3287+
Environment* env = Environment::GetCurrent(args);
3288+
uv_check_start(env->immediate_check_handle(), CheckImmediate);
3289+
// Idle handle is needed only to stop the event loop from blocking in poll.
3290+
uv_idle_start(env->immediate_idle_handle(),
3291+
[](uv_idle_t*){ /* do nothing, just keep the loop running */ });
33103292
}
33113293

33123294

@@ -3533,12 +3515,11 @@ void SetupProcessObject(Environment* env,
35333515
FIXED_ONE_BYTE_STRING(env->isolate(), "ppid"),
35343516
GetParentProcessId).FromJust());
35353517

3536-
auto need_immediate_callback_string =
3537-
FIXED_ONE_BYTE_STRING(env->isolate(), "_needImmediateCallback");
3538-
CHECK(process->SetAccessor(env->context(), need_immediate_callback_string,
3539-
NeedImmediateCallbackGetter,
3540-
NeedImmediateCallbackSetter,
3541-
env->as_external()).FromJust());
3518+
auto scheduled_immediate_count =
3519+
FIXED_ONE_BYTE_STRING(env->isolate(), "_scheduledImmediateCount");
3520+
CHECK(process->Set(env->context(),
3521+
scheduled_immediate_count,
3522+
env->scheduled_immediate_count().GetJSArray()).FromJust());
35423523

35433524
// -e, --eval
35443525
if (eval_string) {
@@ -3664,6 +3645,9 @@ void SetupProcessObject(Environment* env,
36643645
env->as_external()).FromJust());
36653646

36663647
// define various internal methods
3648+
env->SetMethod(process,
3649+
"_activateImmediateCheck",
3650+
ActivateImmediateCheck);
36673651
env->SetMethod(process,
36683652
"_startProfilerIdleNotifier",
36693653
StartProfilerIdleNotifier);

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL