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: #16888 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
nodejs · Nov 28, 2017 · cbaf59c · cbaf59c
1 parent 1f5ee33
commit cbaf59c
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 53 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,35 +124,13 @@ 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() {
     do {
       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
@@ -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
@@ -11,7 +11,6 @@ SyntaxError: Strict mode code may not include a with statement
     at Socket.<anonymous> (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.<anonymous> (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.<anonymous> (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.<anonymous> (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
@@ -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:*)