5-50% speed up setTimeout()

[AlexeyKupershtokh]

Main points are:

1. Do not set Timeout._when property at all -> save a relatively slow Date.now() call & smaller memory footprint.
2. Set Timeout._idleStart and Timeout._onTimeout in the constructor due to these recommendations: http://s3.mrale.ph/nodecamp.eu/#55. Many runs of benchmark/settimeout.js verify this gives about 1-2% gain too.

Benchmark results are the following:
node 0.8.12 + timers.js from 0.9.2:

wait...
smaller is better
startup: 11257
done: 999

node 0.8.12 + optimized timers.js from 0.9.2:

wait...
smaller is better
startup: 7610
done: 999

node 0.9.2 reproduces these results as well.

[AlexeyKupershtokh]

This time the changes are minimalistic and come from a topic branch instead of master.

[AlexeyKupershtokh]

This issue is related only to some systems though.

Node's Date.now() performance gained ~15 times on my laptop after upgrading from Ubuntu 12.04 32 bit + PAE to Ubuntu 12.10 64 bit.
There are more details in this thread:
https://groups.google.com/d/msg/nodejs/A4CQx8t5MgQ/AV2Z1VyFB0UJ

So this patch will usually give only 5-6% performance to the setTimeout, and only in some cases it will be 1.5x gain.

[bnoordhuis]

Cache the result of Date.now() in a variable?

[AlexeyKupershtokh]

Unref is going to be used rarely.
Also it should use actual Date.now in the "var delay = ..." statement, but it should be not set only once in the _idleStart.
So caching in this case is not needed and will be likely harmful.

[bnoordhuis]

I don't understand what you mean. I'm suggesting this:

var now = Date.now();
if (!this._idleStart) this._idleStart = now;
var delay = this._idleStart + this._idleTimeout - now;

Which could be simplified further.

[AlexeyKupershtokh]

Ah, I completely agree with you.

[akzhan]

@AlexeyKupershtokh Simply push additional commit to your topic branch to apply @bnoordhuis' update.

[AlexeyKupershtokh]

@bnoordhuis I've committed your proposed change with --author=bnoordhuis

[AlexeyKupershtokh]

more details: #4225 (comment)
clearTimeout seems have HUGE performance gain with this patch.

[AlexeyKupershtokh]

@bnoordhuis is this pull request good enough for merging? what else should I do?

[isaacs]

Yeah, this does make a pretty big impact on benchmark/misc/timers.js.

# with the patch
misc/timers.js thousands=500 type=depth: 562.05
misc/timers.js thousands=500 type=breadth: 1459.8
# v0.10
misc/timers.js thousands=500 type=depth: 318.45
misc/timers.js thousands=500 type=breadth: 1030.8

Running tests and other benchmarks now. Will land if there aren't any other problems.

[isaacs]

No discernible effect on http or net benchmarks. I was sort of hoping for some improvement there, since we do spend a lot of our time in gettimeofday due to calling timers.active(socket) whenever we send or receive. (If anyone is feeling ambitious, that might be a good spot to try to shave off some ms.)

But anyway, this helps a little and appears to do no harm. @AlexeyKupershtokh if you would be so kind as to sign the CLA, I will land this with pleasure. http://nodejs.org/cla.html

[Nodejs-Jenkins]

Can one of the admins verify this patch?

[AlexeyKupershtokh]

@isaacs done.

[isaacs]

@AlexeyKupershtokh You should probably set up your git config to use your github name and email address. It's a bit confusing in the commit history to see wicked <wicked@alawar.com> as the author.

Landed on v0.10. Thanks!

[AlexeyKupershtokh]

@isaacs yeah. It was one of my first PRs so I hadn't configured git by the time. Sorry for this.