node: improve performance of nextTick

[trevnorris]

Couple micro optimizations to improve performance of process.nextTick().
Removes ~80ns of execution time.

Also added small threshold to test that allows timer to fire early on
the order if microseconds.

R=@bnoordhuis

[vkurchatkin]

What's the point? It is checked inside already

[trevnorris]

Test for yourself. It removes a third of execution time:

'use strict';
const ITER = 1e6;
let i = ITER;
let t = process.hrtime();

(function runner() {
  if (--i > 0) process.nextTick(runner);
  else printTime(process.hrtime(t));
}());

function printTime(t) {
  console.log(((t[0] * 1e9 + t[1]) / ITER).toFixed(1) + ' ns/op');
}

[vkurchatkin]

Oh, I thought it was if (!microtasksScheduled). So it's basically the same as if you remove scheduleMicrotasks

[vkurchatkin]

I wonder why no tests break. This does though

setTimeout(function() {
  process.nextTick(function(){});
  Promise.resolve().then(function() {
    console.log('hi');
  });
}, 10);

setTimeout(function(){}, 1000);

[trevnorris]

Yeah. I thought V8 would have been a little better at inlining the conditional of the function call, but it does seem to make enough difference to require placing the conditional here.

[trevnorris]

@vkurchatkin How is it breaking? Like, 'hi' doesn't display?

[trevnorris]

ah, wow. totally missed what you were saying about the conditional. let me fix it and push up a fix. one moment.

[vkurchatkin]

Displays after 1000ms timeout. I'm going to add tests for this branch of execution.

[trevnorris]

@vkurchatkin I've updated the name of PObj and fixed the other issue. Have reduced execution time from ~120ns to ~60ns.

[vkurchatkin]

This is still wrong:

if (microtasksScheduled)
       scheduleMicrotasks();

[vkurchatkin]

Maybe it's better to use nextTick instead of process.nextTick? don't like when we rely on globals being untouched

[trevnorris]

good idea. done.

[trevnorris]

@vkurchatkin thanks. forgot to force push my latest change. also fixed using process.nextTick().

[trevnorris]

@vkurchatkin Just made a change to move scheduling the microtask queue processor to the end of the tick callback. makes a small functional change in the way it works, but all callbacks will still be executed. mind verifying?

[vkurchatkin]

Can't verify it right now but it looks like it breaks intended behavior completely. When we are done with tick queue it's too late to schedule microtaks because: a) they should run on nextTick queue b) we should be preparared to go on with nextTicks scheduled inside microtasks

[trevnorris]

@vkurchatkin hm. all tests are currently passing. i'll try to write up additional tests to show what you're talking about, but I'll most likely need some help to make sure they're correct.

[vkurchatkin]

@trevnorris tests probably don't cover these cases. When there are no nextTick callbacks microtasks are executed directly from MakeCallback. My oversight (that there are no tests for that).

[trevnorris]

@vkurchatkin I've updated the PR to execute the microtask queue only when the queue is totally depleted, but will also return and continue execution if additional things were added. This is in line with what my prior attempt meant to do. PTAL.

[vkurchatkin]

LTGM. this is something I wanted to do in the first place, but decided to choose safer solution

[trevnorris]

Heh. Then I'll be happy to take the blame in the future if it breaks. :)

[trevnorris]

@vkurchatkin Thanks for the review. Landed in e0835c9.

[rvagg]

Do we have any benchmark numbers on this? it's always good to communicate via numbers, even if they are very rough, keep that in mind whenever adding perf changes because people love seeing it but when it's just a vague "improve performance" then it's hard to make an assessment about how it impacts you.

[trevnorris]

@rvagg Here's the script I used to test:

'use strict';
const ITER = 1e6;
let i = ITER;
let t = process.hrtime();

(function runner() {
  if (--i > 0) process.nextTick(runner);
  else printTime(process.hrtime(t));
}());

function printTime(t) {
  console.log(((t[0] * 1e9 + t[1]) / ITER).toFixed(1) + ' ns/op');
}

Similar to benchmark/misc/next-tick-depth.js.