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

node: improve nextTick performance #5092

Closed
wants to merge 2 commits into from
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
121 changes: 26 additions & 95 deletions src/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,26 @@
scheduleMicrotasks();
}

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.apply(null, args);
}
}
}

// Run callbacks that have no domain.
// Using domains will cause this to be overridden.
function _tickCallback() {
Expand All @@ -383,27 +403,10 @@
tock = nextTickQueue[tickInfo[kIndex]++];
callback = tock.callback;
args = tock.args;
// Using separate callback execution functions helps to limit the
// scope of DEOPTs caused by using try blocks and allows direct
// Using separate callback execution functions allows direct
// callback invocation with small numbers of arguments to avoid the
// performance hit associated with using `fn.apply()`
if (args === undefined) {
nextTickCallbackWith0Args(callback);
} else {
switch (args.length) {
case 1:
nextTickCallbackWith1Arg(callback, args[0]);
break;
case 2:
nextTickCallbackWith2Args(callback, args[0], args[1]);
break;
case 3:
nextTickCallbackWith3Args(callback, args[0], args[1], args[2]);
break;
default:
nextTickCallbackWithManyArgs(callback, args);
}
}
_combinedTickCallback(args, callback);
Copy link
Contributor

Choose a reason for hiding this comment

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

To remove a line in the call stack, think we could just move the if/else back into here? Open to why it should be kept in another function.

Copy link
Member Author

Choose a reason for hiding this comment

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

For me I had two reasons to split it up:

  1. It's less duplicated code
  2. The functions are smaller and the compiler might optimize them faster but I did not check that

I do not have any strong feelings about that though

Copy link
Contributor

Choose a reason for hiding this comment

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

oop. missed that it's called in multiple locations. sounds good.

if (1e4 < tickInfo[kIndex])
tickDone();
}
Expand All @@ -424,27 +427,10 @@
args = tock.args;
if (domain)
domain.enter();
// Using separate callback execution functions helps to limit the
// scope of DEOPTs caused by using try blocks and allows direct
// Using separate callback execution functions allows direct
// callback invocation with small numbers of arguments to avoid the
// performance hit associated with using `fn.apply()`
if (args === undefined) {
nextTickCallbackWith0Args(callback);
} else {
switch (args.length) {
case 1:
nextTickCallbackWith1Arg(callback, args[0]);
break;
case 2:
nextTickCallbackWith2Args(callback, args[0], args[1]);
break;
case 3:
nextTickCallbackWith3Args(callback, args[0], args[1], args[2]);
break;
default:
nextTickCallbackWithManyArgs(callback, args);
}
}
_combinedTickCallback(args, callback);
if (1e4 < tickInfo[kIndex])
tickDone();
if (domain)
Expand All @@ -456,61 +442,6 @@
} while (tickInfo[kLength] !== 0);
}

function nextTickCallbackWith0Args(callback) {
var threw = true;
try {
callback();
threw = false;
} finally {
if (threw)
tickDone();
}
}

function nextTickCallbackWith1Arg(callback, arg1) {
var threw = true;
try {
callback(arg1);
threw = false;
} finally {
if (threw)
tickDone();
}
}

function nextTickCallbackWith2Args(callback, arg1, arg2) {
var threw = true;
try {
callback(arg1, arg2);
threw = false;
} finally {
if (threw)
tickDone();
}
}

function nextTickCallbackWith3Args(callback, arg1, arg2, arg3) {
var threw = true;
try {
callback(arg1, arg2, arg3);
threw = false;
} finally {
if (threw)
tickDone();
}
}

function nextTickCallbackWithManyArgs(callback, args) {
var threw = true;
try {
callback.apply(null, args);
threw = false;
} finally {
if (threw)
tickDone();
}
}

function TickObject(c, args) {
this.callback = c;
this.domain = process.domain || null;
Expand All @@ -526,9 +457,9 @@

var args;
if (arguments.length > 1) {
args = [];
args = new Array(arguments.length - 1);
for (var i = 1; i < arguments.length; i++)
args.push(arguments[i]);
args[i - 1] = arguments[i];
}

nextTickQueue.push(new TickObject(callback, args));
Expand Down
8 changes: 4 additions & 4 deletions test/message/eval_messages.out
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ SyntaxError: Strict mode code may not include a with statement
at Object.<anonymous> ([eval]-wrapper:*:*)
at Module._compile (module.js:*:*)
at node.js:*:*
at nextTickCallbackWith0Args (node.js:*:*)
at _combinedTickCallback (node.js:*:*)
at process._tickCallback (node.js:*:*)
42
42
Expand All @@ -20,7 +20,7 @@ Error: hello
at Object.<anonymous> ([eval]-wrapper:*:*)
at Module._compile (module.js:*:*)
at node.js:*:*
at nextTickCallbackWith0Args (node.js:*:*)
at _combinedTickCallback (node.js:*:*)
at process._tickCallback (node.js:*:*)
[eval]:1
throw new Error("hello")
Expand All @@ -31,7 +31,7 @@ Error: hello
at Object.<anonymous> ([eval]-wrapper:*:*)
at Module._compile (module.js:*:*)
at node.js:*:*
at nextTickCallbackWith0Args (node.js:*:*)
at _combinedTickCallback (node.js:*:*)
at process._tickCallback (node.js:*:*)
100
[eval]:1
Expand All @@ -43,7 +43,7 @@ ReferenceError: y is not defined
at Object.<anonymous> ([eval]-wrapper:*:*)
at Module._compile (module.js:*:*)
at node.js:*:*
at nextTickCallbackWith0Args (node.js:*:*)
at _combinedTickCallback (node.js:*:*)
at process._tickCallback (node.js:*:*)
[eval]:1
var ______________________________________________; throw 10
Expand Down
2 changes: 1 addition & 1 deletion test/message/nexttick_throw.out
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
^
ReferenceError: undefined_reference_error_maker is not defined
at *test*message*nexttick_throw.js:*:*
at nextTickCallbackWith0Args (node.js:*:*)
at _combinedTickCallback (node.js:*:*)
at process._tickCallback (node.js:*:*)
at Function.Module.runMain (module.js:*:*)
at startup (node.js:*:*)
Expand Down
8 changes: 4 additions & 4 deletions test/message/stdin_messages.out
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ SyntaxError: Strict mode code may not include a with statement
at Object.<anonymous> ([stdin]-wrapper:*:*)
at Module._compile (module.js:*:*)
at node.js:*:*
at nextTickCallbackWith0Args (node.js:*:*)
at _combinedTickCallback (node.js:*:*)
at process._tickCallback (node.js:*:*)
42
42
Expand All @@ -22,7 +22,7 @@ Error: hello
at Object.<anonymous> ([stdin]-wrapper:*:*)
at Module._compile (module.js:*:*)
at node.js:*:*
at nextTickCallbackWith0Args (node.js:*:*)
at _combinedTickCallback (node.js:*:*)
at process._tickCallback (node.js:*:*)

[stdin]:1
Expand All @@ -34,7 +34,7 @@ Error: hello
at Object.<anonymous> ([stdin]-wrapper:*:*)
at Module._compile (module.js:*:*)
at node.js:*:*
at nextTickCallbackWith0Args (node.js:*:*)
at _combinedTickCallback (node.js:*:*)
at process._tickCallback (node.js:*:*)
100

Expand All @@ -47,7 +47,7 @@ ReferenceError: y is not defined
at Object.<anonymous> ([stdin]-wrapper:*:*)
at Module._compile (module.js:*:*)
at node.js:*:*
at nextTickCallbackWith0Args (node.js:*:*)
at _combinedTickCallback (node.js:*:*)
at process._tickCallback (node.js:*:*)

[stdin]:1
Expand Down