Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit fd501b3

Browse filesBrowse files
addaleaxMylesBorins
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
Copy full SHA for fd501b3

File tree

Expand file treeCollapse file tree

4 files changed

+69
-73
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+69
-73
lines changed
Open diff view settings
Collapse file

‎lib/timers.js‎

Copy file name to clipboardExpand all lines: lib/timers.js
+24-22Lines 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
};
Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+6Lines 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
}
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+4Lines 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

Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+35-51Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.