From 7e247c182ebea6a4ca2a3078a29f2edbaa2d21cc Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sat, 25 Nov 2017 11:58:53 -0500 Subject: [PATCH] process: slightly simplify next tick execution Get rid of separate function to call callback from _tickCallback as it no longer yields worthwhile performance improvement. Move some code from nextTick & internalNextTick into TickObject constructor to minimize duplication. PR-URL: https://github.com/nodejs/node/pull/16888 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Refael Ackermann Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater Reviewed-By: Timothy Gu --- benchmark/process/next-tick-breadth-args.js | 2 +- benchmark/process/next-tick-breadth.js | 2 +- benchmark/process/next-tick-exec-args.js | 25 ++++++ benchmark/process/next-tick-exec.js | 18 +++++ lib/internal/process/next_tick.js | 77 ++++++++----------- test/message/nexttick_throw.out | 1 - test/message/stdin_messages.out | 7 +- .../unhandled_promise_trace_warnings.out | 2 - 8 files changed, 81 insertions(+), 53 deletions(-) create mode 100644 benchmark/process/next-tick-exec-args.js create mode 100644 benchmark/process/next-tick-exec.js diff --git a/benchmark/process/next-tick-breadth-args.js b/benchmark/process/next-tick-breadth-args.js index cc038dd348faac..ca608f15daa743 100644 --- a/benchmark/process/next-tick-breadth-args.js +++ b/benchmark/process/next-tick-breadth-args.js @@ -2,7 +2,7 @@ const common = require('../common.js'); const bench = common.createBenchmark(main, { - millions: [2] + millions: [4] }); function main(conf) { diff --git a/benchmark/process/next-tick-breadth.js b/benchmark/process/next-tick-breadth.js index 8f8d0adc618dad..51951ce0afd645 100644 --- a/benchmark/process/next-tick-breadth.js +++ b/benchmark/process/next-tick-breadth.js @@ -2,7 +2,7 @@ const common = require('../common.js'); const bench = common.createBenchmark(main, { - millions: [2] + millions: [4] }); function main(conf) { diff --git a/benchmark/process/next-tick-exec-args.js b/benchmark/process/next-tick-exec-args.js new file mode 100644 index 00000000000000..5ff017bb29cd5b --- /dev/null +++ b/benchmark/process/next-tick-exec-args.js @@ -0,0 +1,25 @@ +'use strict'; +const common = require('../common.js'); +const bench = common.createBenchmark(main, { + millions: [5] +}); + +function main(conf) { + var n = +conf.millions * 1e6; + + bench.start(); + for (var i = 0; i < n; i++) { + if (i % 4 === 0) + process.nextTick(onNextTick, i, true, 10, 'test'); + else if (i % 3 === 0) + process.nextTick(onNextTick, i, true, 10); + else if (i % 2 === 0) + process.nextTick(onNextTick, i, 20); + else + process.nextTick(onNextTick, i); + } + function onNextTick(i) { + if (i + 1 === n) + bench.end(+conf.millions); + } +} diff --git a/benchmark/process/next-tick-exec.js b/benchmark/process/next-tick-exec.js new file mode 100644 index 00000000000000..12c9d4624a903c --- /dev/null +++ b/benchmark/process/next-tick-exec.js @@ -0,0 +1,18 @@ +'use strict'; +const common = require('../common.js'); +const bench = common.createBenchmark(main, { + millions: [5] +}); + +function main(conf) { + var n = +conf.millions * 1e6; + + bench.start(); + for (var i = 0; i < n; i++) { + process.nextTick(onNextTick, i); + } + function onNextTick(i) { + if (i + 1 === n) + bench.end(+conf.millions); + } +} diff --git a/lib/internal/process/next_tick.js b/lib/internal/process/next_tick.js index 83a833dc596c24..4c8b5f7d2d4181 100644 --- a/lib/internal/process/next_tick.js +++ b/lib/internal/process/next_tick.js @@ -60,7 +60,7 @@ function setupNextTick() { // Grab the constants necessary for working with internal arrays. const { kInit, kDestroy, kAsyncIdCounter } = async_wrap.constants; const { async_id_symbol, trigger_async_id_symbol } = async_wrap; - var nextTickQueue = new NextTickQueue(); + const nextTickQueue = new NextTickQueue(); var microtasksScheduled = false; // Used to run V8's micro task queue. @@ -99,7 +99,6 @@ function setupNextTick() { const microTasksTickObject = { callback: runMicrotasksCallback, args: undefined, - domain: null, [async_id_symbol]: 0, [trigger_async_id_symbol]: 0 }; @@ -125,26 +124,6 @@ function setupNextTick() { } } - function _combinedTickCallback(args, callback) { - if (args === undefined) { - callback(); - } else { - switch (args.length) { - case 1: - callback(args[0]); - break; - case 2: - callback(args[0], args[1]); - break; - case 3: - callback(args[0], args[1], args[2]); - break; - default: - callback(...args); - } - } - } - // Run callbacks that have no domain. // Using domains will cause this to be overridden. function _tickCallback() { @@ -152,8 +131,6 @@ function setupNextTick() { while (tickInfo[kIndex] < tickInfo[kLength]) { ++tickInfo[kIndex]; const tock = nextTickQueue.shift(); - const callback = tock.callback; - const args = tock.args; // CHECK(Number.isSafeInteger(tock[async_id_symbol])) // CHECK(tock[async_id_symbol] > 0) @@ -173,10 +150,11 @@ function setupNextTick() { if (async_hook_fields[kDestroy] > 0) emitDestroy(tock[async_id_symbol]); - // Using separate callback execution functions allows direct - // callback invocation with small numbers of arguments to avoid the - // performance hit associated with using `fn.apply()` - _combinedTickCallback(args, callback); + const callback = tock.callback; + if (tock.args === undefined) + callback(); + else + Reflect.apply(callback, undefined, tock.args); emitAfter(tock[async_id_symbol]); @@ -191,11 +169,21 @@ function setupNextTick() { class TickObject { constructor(callback, args, asyncId, triggerAsyncId) { + // this must be set to null first to avoid function tracking + // on the hidden class, revisit in V8 versions after 6.2 + this.callback = null; this.callback = callback; this.args = args; - this.domain = process.domain || null; + this[async_id_symbol] = asyncId; this[trigger_async_id_symbol] = triggerAsyncId; + + if (async_hook_fields[kInit] > 0) { + emitInit(asyncId, + 'TickObject', + triggerAsyncId, + this); + } } } @@ -220,13 +208,14 @@ function setupNextTick() { args[i - 1] = arguments[i]; } - const asyncId = ++async_id_fields[kAsyncIdCounter]; - const triggerAsyncId = initTriggerId(); - const obj = new TickObject(callback, args, asyncId, triggerAsyncId); - nextTickQueue.push(obj); + // In V8 6.2, moving tickInfo & async_id_fields[kAsyncIdCounter] into the + // TickObject incurs a significant performance penalty in the + // next-tick-breadth-args benchmark (revisit later) ++tickInfo[kLength]; - if (async_hook_fields[kInit] > 0) - emitInit(asyncId, 'TickObject', triggerAsyncId, obj); + nextTickQueue.push(new TickObject(callback, + args, + ++async_id_fields[kAsyncIdCounter], + initTriggerId())); } // `internalNextTick()` will not enqueue any callback when the process is @@ -240,10 +229,6 @@ function setupNextTick() { if (process._exiting) return; - if (triggerAsyncId === null) { - triggerAsyncId = async_hooks.initTriggerId(); - } - var args; switch (arguments.length) { case 2: break; @@ -256,11 +241,15 @@ function setupNextTick() { args[i - 2] = arguments[i]; } - const asyncId = ++async_id_fields[kAsyncIdCounter]; - const obj = new TickObject(callback, args, asyncId, triggerAsyncId); - nextTickQueue.push(obj); + if (triggerAsyncId === null) + triggerAsyncId = initTriggerId(); + // In V8 6.2, moving tickInfo & async_id_fields[kAsyncIdCounter] into the + // TickObject incurs a significant performance penalty in the + // next-tick-breadth-args benchmark (revisit later) ++tickInfo[kLength]; - if (async_hook_fields[kInit] > 0) - emitInit(asyncId, 'TickObject', triggerAsyncId, obj); + nextTickQueue.push(new TickObject(callback, + args, + ++async_id_fields[kAsyncIdCounter], + triggerAsyncId)); } } diff --git a/test/message/nexttick_throw.out b/test/message/nexttick_throw.out index 1b72ea2d3cfb69..1c9eca8405d928 100644 --- a/test/message/nexttick_throw.out +++ b/test/message/nexttick_throw.out @@ -4,7 +4,6 @@ ^ ReferenceError: undefined_reference_error_maker is not defined at *test*message*nexttick_throw.js:*:* - at _combinedTickCallback (internal/process/next_tick.js:*:*) at process._tickCallback (internal/process/next_tick.js:*:*) at Function.Module.runMain (module.js:*:*) at startup (bootstrap_node.js:*:*) diff --git a/test/message/stdin_messages.out b/test/message/stdin_messages.out index 3145d50894771b..d934523a726772 100644 --- a/test/message/stdin_messages.out +++ b/test/message/stdin_messages.out @@ -11,7 +11,6 @@ SyntaxError: Strict mode code may not include a with statement at Socket. (bootstrap_node.js:*:*) at Socket.emit (events.js:*:*) at endReadableNT (_stream_readable.js:*:*) - at _combinedTickCallback (internal/process/next_tick.js:*:*) at process._tickCallback (internal/process/next_tick.js:*:*) 42 42 @@ -29,7 +28,7 @@ Error: hello at Socket. (bootstrap_node.js:*:*) at Socket.emit (events.js:*:*) at endReadableNT (_stream_readable.js:*:*) - at _combinedTickCallback (internal/process/next_tick.js:*:*) + at process._tickCallback (internal/process/next_tick.js:*:*) [stdin]:1 throw new Error("hello") ^ @@ -44,7 +43,7 @@ Error: hello at Socket. (bootstrap_node.js:*:*) at Socket.emit (events.js:*:*) at endReadableNT (_stream_readable.js:*:*) - at _combinedTickCallback (internal/process/next_tick.js:*:*) + at process._tickCallback (internal/process/next_tick.js:*:*) 100 [stdin]:1 var x = 100; y = x; @@ -60,7 +59,7 @@ ReferenceError: y is not defined at Socket. (bootstrap_node.js:*:*) at Socket.emit (events.js:*:*) at endReadableNT (_stream_readable.js:*:*) - at _combinedTickCallback (internal/process/next_tick.js:*:*) + at process._tickCallback (internal/process/next_tick.js:*:*) [stdin]:1 var ______________________________________________; throw 10 diff --git a/test/message/unhandled_promise_trace_warnings.out b/test/message/unhandled_promise_trace_warnings.out index 410b3bf4e36d41..c9c7a5c8700b26 100644 --- a/test/message/unhandled_promise_trace_warnings.out +++ b/test/message/unhandled_promise_trace_warnings.out @@ -15,7 +15,6 @@ at * at * at * - at * (node:*) Error: This was rejected at * (*test*message*unhandled_promise_trace_warnings.js:*) at * @@ -34,7 +33,6 @@ at * at * at * - at * (node:*) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1) at getAsynchronousRejectionWarningObject (internal/process/promises.js:*) at rejectionHandled (internal/process/promises.js:*)