Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

timers: run nextTicks after each immediate and timer #22842

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions benchmark/timers/timers-timeout-nexttick.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use strict';
const common = require('../common.js');

// The following benchmark measures setting up n * 1e6 timeouts,
// as well as scheduling a next tick from each timeout. Those
// then get executed on the next uv tick.

const bench = common.createBenchmark(main, {
n: [5e4, 5e6],
});

function main({ n }) {
let count = 0;

// Function tracking on the hidden class in V8 can cause misleading
// results in this benchmark if only a single function is used —
// alternate between two functions for a fairer benchmark.

function cb() {
process.nextTick(counter);
}
function cb2() {
process.nextTick(counter);
}
function counter() {
count++;
if (count === n)
bench.end(n);
}

for (var i = 0; i < n; i++) {
setTimeout(i % 2 ? cb : cb2, 1);
}

bench.start();
}
18 changes: 5 additions & 13 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -1461,13 +1461,11 @@ changes:
* `callback` {Function}
* `...args` {any} Additional arguments to pass when invoking the `callback`

The `process.nextTick()` method adds the `callback` to the "next tick queue".
Once the current turn of the event loop turn runs to completion, all callbacks
currently in the next tick queue will be called.

This is *not* a simple alias to [`setTimeout(fn, 0)`][]. It is much more
efficient. It runs before any additional I/O events (including
timers) fire in subsequent ticks of the event loop.
`process.nextTick()` adds `callback` to the "next tick queue". This queue is
fully drained after the current operation on the JavaScript stack runs to
completion and before the event loop is allowed to continue. As a result, it's
possible to create an infinite loop if one were to recursively call
`process.nextTick()`.

```js
console.log('start');
Expand Down Expand Up @@ -1542,11 +1540,6 @@ function definitelyAsync(arg, cb) {
}
```

The next tick queue is completely drained on each pass of the event loop
**before** additional I/O is processed. As a result, recursively setting
`nextTick()` callbacks will block any I/O from happening, just like a
`while(true);` loop.

## process.noDeprecation
<!-- YAML
added: v0.8.0
Expand Down Expand Up @@ -2162,7 +2155,6 @@ cases:
[`require()`]: globals.html#globals_require
[`require.main`]: modules.html#modules_accessing_the_main_module
[`require.resolve()`]: modules.html#modules_require_resolve_request_options
[`setTimeout(fn, 0)`]: timers.html#timers_settimeout_callback_delay_args
[`v8.setFlagsFromString()`]: v8.html#v8_v8_setflagsfromstring_flags
[Android building]: https://github.com/nodejs/node/blob/master/BUILDING.md#androidandroid-based-devices-eg-firefox-os
[Child Process]: child_process.html
Expand Down
11 changes: 10 additions & 1 deletion lib/internal/process/next_tick.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function setupNextTick(_setupNextTick, _setupPromises) {
const [
tickInfo,
runMicrotasks
] = _setupNextTick(_tickCallback);
] = _setupNextTick(internalTickCallback);

// *Must* match Environment::TickInfo::Fields in src/env.h.
const kHasScheduled = 0;
Expand All @@ -39,6 +39,15 @@ function setupNextTick(_setupNextTick, _setupPromises) {
process._tickCallback = _tickCallback;

function _tickCallback() {
if (tickInfo[kHasScheduled] === 0 && tickInfo[kHasPromiseRejections] === 0)
runMicrotasks();
if (tickInfo[kHasScheduled] === 0 && tickInfo[kHasPromiseRejections] === 0)
return;

internalTickCallback();
}

function internalTickCallback() {
let tock;
do {
while (tock = queue.shift()) {
Expand Down
175 changes: 76 additions & 99 deletions lib/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,16 +254,17 @@ function processTimers(now) {
debug('process timer lists %d', now);
nextExpiry = Infinity;

let list, ran;
let list;
let ranAtLeastOneList = false;
while (list = queue.peek()) {
if (list.expiry > now) {
nextExpiry = list.expiry;
return refCount > 0 ? nextExpiry : -nextExpiry;
}
if (ran)
if (ranAtLeastOneList)
runNextTicks();
else
ran = true;
ranAtLeastOneList = true;
listOnTimeout(list, now);
}
return 0;
Expand All @@ -275,6 +276,7 @@ function listOnTimeout(list, now) {
debug('timeout callback %d', msecs);

var diff, timer;
let ranAtLeastOneTimer = false;
while (timer = L.peek(list)) {
diff = now - timer._idleStart;

Expand All @@ -288,6 +290,11 @@ function listOnTimeout(list, now) {
return;
}

if (ranAtLeastOneTimer)
runNextTicks();
else
ranAtLeastOneTimer = true;

// The actual logic for when a timeout happens.
L.remove(timer);

Expand All @@ -307,7 +314,33 @@ function listOnTimeout(list, now) {

emitBefore(asyncId, timer[trigger_async_id_symbol]);

tryOnTimeout(timer);
let start;
if (timer._repeat)
start = getLibuvNow();

try {
const args = timer._timerArgs;
if (!args)
timer._onTimeout();
else
Reflect.apply(timer._onTimeout, timer, args);
} finally {
if (timer._repeat && timer._idleTimeout !== -1) {
timer._idleTimeout = timer._repeat;
if (start === undefined)
start = getLibuvNow();
insert(timer, timer[kRefed], start);
} else {
if (timer[kRefed])
refCount--;
timer[kRefed] = null;

if (destroyHooksExist() && !timer._destroyed) {
emitDestroy(timer[async_id_symbol]);
timer._destroyed = true;
}
}
}

emitAfter(asyncId);
}
Expand All @@ -327,30 +360,6 @@ function listOnTimeout(list, now) {
}


// An optimization so that the try/finally only de-optimizes (since at least v8
// 4.7) what is in this smaller function.
function tryOnTimeout(timer, start) {
if (start === undefined && timer._repeat)
start = getLibuvNow();
try {
ontimeout(timer);
} finally {
if (timer._repeat) {
rearm(timer, start);
} else {
if (timer[kRefed])
refCount--;
timer[kRefed] = null;

if (destroyHooksExist() && !timer._destroyed) {
emitDestroy(timer[async_id_symbol]);
timer._destroyed = true;
}
}
}
}


// Remove a timer. Cancels the timeout and resets the relevant timer properties.
function unenroll(item) {
// Fewer checks may be possible, but these cover everything.
Expand Down Expand Up @@ -456,26 +465,6 @@ setTimeout[internalUtil.promisify.custom] = function(after, value) {
exports.setTimeout = setTimeout;


function ontimeout(timer) {
const args = timer._timerArgs;
if (typeof timer._onTimeout !== 'function')
return Promise.resolve(timer._onTimeout, args[0]);
if (!args)
timer._onTimeout();
else
Reflect.apply(timer._onTimeout, timer, args);
}

function rearm(timer, start) {
// Do not re-arm unenroll'd or closed timers.
if (timer._idleTimeout === -1)
return;

timer._idleTimeout = timer._repeat;
insert(timer, timer[kRefed], start);
}


const clearTimeout = exports.clearTimeout = function clearTimeout(timer) {
if (timer && timer._onTimeout) {
timer._onTimeout = null;
Expand Down Expand Up @@ -601,75 +590,63 @@ function processImmediate() {
const queue = outstandingQueue.head !== null ?
outstandingQueue : immediateQueue;
var immediate = queue.head;
const tail = queue.tail;

// Clear the linked list early in case new `setImmediate()` calls occur while
// immediate callbacks are executed
queue.head = queue.tail = null;

let count = 0;
let refCount = 0;
if (queue !== outstandingQueue) {
queue.head = queue.tail = null;
immediateInfo[kHasOutstanding] = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could leave a comment here about how this = null thing works? it's... pretty difficult to read/understand.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should disallow that in the lint rules. please split it into two.

}

let prevImmediate;
let ranAtLeastOneImmediate = false;
while (immediate !== null) {
immediate._destroyed = true;
if (ranAtLeastOneImmediate)
runNextTicks();
else
ranAtLeastOneImmediate = true;

const asyncId = immediate[async_id_symbol];
emitBefore(asyncId, immediate[trigger_async_id_symbol]);
// It's possible for this current Immediate to be cleared while executing
// the next tick queue above, which means we need to use the previous
// Immediate's _idleNext which is guaranteed to not have been cleared.
if (immediate._destroyed) {
outstandingQueue.head = immediate = prevImmediate._idleNext;
continue;
}

count++;
immediate._destroyed = true;

immediateInfo[kCount]--;
if (immediate[kRefed])
refCount++;
immediateInfo[kRefCount]--;
immediate[kRefed] = null;

tryOnImmediate(immediate, tail, count, refCount);
prevImmediate = immediate;

emitAfter(asyncId);
const asyncId = immediate[async_id_symbol];
emitBefore(asyncId, immediate[trigger_async_id_symbol]);

immediate = immediate._idleNext;
}
try {
const argv = immediate._argv;
if (!argv)
immediate._onImmediate();
else
Reflect.apply(immediate._onImmediate, immediate, argv);
} finally {
immediate._onImmediate = null;

immediateInfo[kCount] -= count;
immediateInfo[kRefCount] -= refCount;
immediateInfo[kHasOutstanding] = 0;
}
if (destroyHooksExist())
emitDestroy(asyncId);

// An optimization so that the try/finally only de-optimizes (since at least v8
// 4.7) what is in this smaller function.
function tryOnImmediate(immediate, oldTail, count, refCount) {
var threw = true;
try {
// make the actual call outside the try/finally to allow it to be optimized
runCallback(immediate);
threw = false;
} finally {
immediate._onImmediate = null;

if (destroyHooksExist()) {
emitDestroy(immediate[async_id_symbol]);
outstandingQueue.head = immediate = immediate._idleNext;
}

if (threw) {
immediateInfo[kCount] -= count;
immediateInfo[kRefCount] -= refCount;

if (immediate._idleNext !== null) {
// Handle any remaining Immediates after error handling has resolved,
// assuming we're still alive to do so.
outstandingQueue.head = immediate._idleNext;
outstandingQueue.tail = oldTail;
immediateInfo[kHasOutstanding] = 1;
}
}
emitAfter(asyncId);
}
}

function runCallback(timer) {
const argv = timer._argv;
if (typeof timer._onImmediate !== 'function')
return Promise.resolve(timer._onImmediate, argv[0]);
if (!argv)
return timer._onImmediate();
Reflect.apply(timer._onImmediate, timer, argv);
if (queue === outstandingQueue)
outstandingQueue.head = null;
immediateInfo[kHasOutstanding] = 0;
}


Expand Down
1 change: 1 addition & 0 deletions test/message/events_unhandled_error_nexttick.out
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Error
at bootstrapNodeJSCore (internal/bootstrap/node.js:*:*)
Emitted 'error' event at:
at process.nextTick (*events_unhandled_error_nexttick.js:*:*)
at internalTickCallback (internal/process/next_tick.js:*:*)
at process._tickCallback (internal/process/next_tick.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at startup (internal/bootstrap/node.js:*:*)
Expand Down
1 change: 1 addition & 0 deletions test/message/nexttick_throw.out
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
^
ReferenceError: undefined_reference_error_maker is not defined
at *test*message*nexttick_throw.js:*:*
at internalTickCallback (internal/process/next_tick.js:*:*)
at process._tickCallback (internal/process/next_tick.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at startup (internal/bootstrap/node.js:*:*)
Expand Down
Loading