Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Document what setTimeout(callback, 0) does #7447

Closed
wants to merge 1 commit into from
Closed

Document what setTimeout(callback, 0) does #7447

wants to merge 1 commit into from

Conversation

dandv
Copy link

@dandv dandv commented Apr 10, 2014

CLA signed. I've edited via the GitHub UI - sorry for forgetting to add the "Docs" subsystem in the commit message.

@Nodejs-Jenkins
Copy link

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit dandv/node@352191d has the following error(s):

  • Commit message must indicate the subsystem this commit changes

The following commiters were not found in the CLA:

  • Dan Dascalescu

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

@Mithgol
Copy link

Mithgol commented Apr 10, 2014

Suggestion: recommend setImmediate along with process.nextTick.

@othiym23
Copy link

Actually, don't recommend process.nextTick at all. With the advent of setImmediate, which doesn't starve I/O, process.nextTick should be considered an internal API that's only useful in certain tightly circumscribed situations.

@trevnorris
Copy link

We should be more specific here. Recursive setImmediate doesn't starve
I/O. But queueing up a million things in the same "tick" will still require
them to be run sequentially to completion on the next round.

@tjfontaine
Copy link

Is it necessary to document setTimeout(fn, 0)? Anyway, I second the notion that nextTick should be removed from examples and use deemphasized. Similarly, I prefer to use the term turn of the event loop which I stole from @othiym23

@trevnorris
Copy link

+1 on using "event loop turn".

-1 on making process.nextTick() an internal only API. I'm for documenting that users should reconsider using process.nextTick() instead of setImmediate(), but I don't believe it should be removed from user-land.

@dandv
Copy link
Author

dandv commented Apr 10, 2014

Good to see a discussion started. I'm not a core node dev; just a user who has run into unexpected and undocumented behavior for setTimeout(fn, 0), @tjfontaine. I do believe it will help everyone to have setTimeout(fn, 0) and its preferred alternative documented, and I'll let you take over from here.

@calvinmetcalf
Copy link

this has split into 2 discussions

  • documenting that setTimeout(fn, 0); doesn't do what you think it does and setImmediate(fn); is generally what you intended.
  • general de emphasizing process.next tick in favor of setImmediate, possibly helped by adding a section to the docs with a reference to microtasks and macrotasks and how you almost always want a macrotask?

edit: added the word 'helped'

@Mithgol
Copy link

Mithgol commented May 20, 2014

In a browser using standard window.setTimeout(yourFunction, 0) does not seem very efficient because HTML5 standard defines its minimal timeout as 4 milliseconds even if 0 is given (i.e. even if you use setTimeout only 250 times, you already get a whole second of extra delay in your application).

Is it the same in Node.js?

@othiym23
Copy link

var totalNanoseconds = 0;
var count = 1e3;

var NS_TO_S = 1e-9;
var S_TO_NS = 1e9;
var S_TO_MS = 1e3;
var AVERAGE = NS_TO_S * S_TO_MS / count;

function noTimeout(cb) {
  var start = process.hrtime();
  setTimeout(function () {
    var finish = process.hrtime(start);
    cb(finish[0] * S_TO_NS + finish[1]);
  }, 0);
}

function run() {
  noTimeout(function (time) {
    totalNanoseconds += time;
    if (--count > 0) return run();

    console.log("Loop took an average of %sms per iteration.", totalNanoseconds * AVERAGE);
  });
}

run();

produces

% node timed-out.js
Loop took an average of 1.3080131550000003ms per iteration.

If you look at the code, you'll see that you're probably never going to get it to run in much less than a millisecond because of the built-in floor to the duration, which, as the comment helpfully notes, is there to provide compatibility with browsers. Try replacing the call to setTimeout with setImmediate and seeing what difference it makes. This has nothing to do with efficiency and everything to do with timing attacks. This is why modules like asap have to go to such lengths to find a faster way to do something Real Soon Now across both browsers and Node.

@jasnell
Copy link
Member

jasnell commented Aug 13, 2015

@othiym23 @trevnorris ... recognizing that we likely still need to do a better job explaining nextTick/setImmediate in the docs, is there reason to keep this PR open?

@jasnell jasnell added the doc label Aug 14, 2015
@jasnell
Copy link
Member

jasnell commented Aug 15, 2015

Related: #6725

@jasnell
Copy link
Member

jasnell commented Aug 15, 2015

New issue opened in nodejs/docs. Closing this here but still need to revisit.

@jasnell jasnell closed this Aug 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants