Skip to content

Conversation

@matehat
Copy link

@matehat matehat commented Feb 17, 2013

Really simple improvement to use process.nextTick instead of setTimeout under Node.js. Quick benchmarks show that the incurred overhead of the current implementation (using setTimeout(fn, 0)) varies between 600 µs to 1.2 ms. With the current improvement, it varies between 200 µs to 450 µs.

After a few years in production, that could potentially save us 1 or 2 seconds! :)

@jdalton
Copy link
Contributor

jdalton commented Feb 17, 2013

setTimeout(fn, 0), and low ms in general, varies by environment. I know IE 10 and Node v0.10 have setImmediate which may be a better choice or at least covers a few more enviros (also there are shims for setImmediate).

@matehat
Copy link
Author

matehat commented Feb 17, 2013

How about this decision flow:

  1. If process.nextTick is available, use that
  2. Otherwise, if setImmediate is available, use that
  3. Otherwise, use setTimeout

@ghost
Copy link

ghost commented Feb 17, 2013

Hmm, I'd prefer setImmediate to nextTick—since the latter now executes at the end of the current tick, instead of at the beginning of the next one, it's no longer recursion-safe (so this pattern wouldn't work):

_.defer(function next() {
  // ...
  _.defer(next);
});

@matehat
Copy link
Author

matehat commented Feb 17, 2013

setImmediate being available and nextTick being executed at the end of the current tick will be effective when Node.js v0.10 comes out, and when that happens we should definitely use setImmediate. But until v0.10 is out and viable for most people nextTick is the only option better than setTimeout. So how about reversing 1 and 2:

  1. If setImmediate is available (latest browsers and Node v0.10), use that
  2. Otherwise, if process.nextTick is available (Node pre-v.10), use that
  3. Otherwise, use setTimeout (everyone else)

@matehat
Copy link
Author

matehat commented Mar 13, 2013

Is there something holding back this PR?

@jashkenas
Copy link
Owner

Just general slowness. Since the nextTick and setTimeout semantics are now different -- and they can't be guaranteed to work interchangeably, it wouldn't be wise to conflate them, no? Using setTimeout for a recursive call is a pretty common pattern.

@matehat
Copy link
Author

matehat commented Mar 14, 2013

I agree. Would it be more acceptable if we just tried to detect if setImmediate is available and use that if it is?

@jashkenas
Copy link
Owner

Probably not, given the MDN note:

This method is not expected to become standard, and is only implemented by recent builds of
Internet Explorer. It meets resistance both from Gecko (Firefox) and Webkit (Google/Apple)

I think Underscore would be wise to just stay out of it, and let you optimize further if you wish.

@jashkenas jashkenas closed this Mar 14, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants