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

nextTick Now #3709

Closed
wants to merge 5 commits into from
Closed

nextTick Now #3709

wants to merge 5 commits into from

Conversation

isaacs
Copy link

@isaacs isaacs commented Jul 14, 2012

Run nextTick callbacks right away.

$ ./node benchmark/next-tick-2.js 
nextTick callbacks per second: 4545455

# master:
nextTick callbacks per second: 1700680

Less dramatic speedup on http_simple, but still an improvement:

$ bash benchmark/http.sh
Requests per second:    3420.11 [#/sec] (mean)

# master
Requests per second:    3260.39 [#/sec] (mean)

Event loop starvation is prevented responsibly, but it IS possible if you set the process.maxTickDepth to a very high number (like Infinity).

Tests and docs included.

@Mithgol
Copy link

Mithgol commented Jul 14, 2012

After the nextTick()'s priority is raised, how do I register a _really idle_ worker that runs _after_ IO is completed?

(That's the most frequently asked question from #3335.)

For example, would setTimeout(someFunction, 0) be a good replacement for the old behaviour of nextTick(someFunction)?

process.nextTick(function () {
console.log('This will not run');
});
setTimeout(function () {
Copy link
Member

Choose a reason for hiding this comment

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

This is rather wired, would it not be an idea to prevent process.nextTick after the exitevent has emitted.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, that's a good idea. exit should make nextTick a no-op.

@isaacs
Copy link
Author

isaacs commented Jul 16, 2012

@Mithgol nextTick was never a suitable idle listener. It'd be nice to add a process.on('idle', fn) API, but for now, setTimeout(fn,0) is pretty close.

@bnoordhuis Replied to two comments. Otherwise all good uncontroversial stuff, thanks. I'll update with that feedback. (Also, I think the maxTickDepth ought to be much higher, like 1000. It's just a loop, it's not THAT bad until it's really super excessive.)

@isaacs
Copy link
Author

isaacs commented Jul 16, 2012

@bnoordhuis Fixed up with your comments.

It'd be good to get another reviewer on this. @piscisaureus?

}
enter = Local<Function>::Cast(domain->Get(enter_symbol));
enter->Call(domain, 0, NULL);
Local<Function> cb = Local<Function>::Cast(cb_v);
Copy link
Member

Choose a reason for hiding this comment

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

.As<Function>()?

@indutny
Copy link
Member

indutny commented Jul 16, 2012

lgtm. Though it was hard to review since you've pushed many unrelated commits in one place :)

@isaacs
Copy link
Author

isaacs commented Jul 16, 2012

@indutny You can review them individually at https://github.com/joyent/node/pull/3709/commits. They're all part of the same feature.

I'll pull in teh .As<Function> change and land. Thanks :)

@isaacs
Copy link
Author

isaacs commented Jul 16, 2012

Actually, I spoke too soon. It seems like this causes some huge problems with tls and domains, as a result of pulling MakeCallback into JS. Investigating.

@isaacs
Copy link
Author

isaacs commented Jul 17, 2012

This needs to happen in smaller steps.

Closing this. First step is to move MakeCallback to JS without breaking anything.

@isaacs isaacs closed this Jul 17, 2012
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.

None yet

5 participants