Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc-eventLoop #1573

Closed
zzuligy opened this issue Feb 23, 2018 · 9 comments
Closed

doc-eventLoop #1573

zzuligy opened this issue Feb 23, 2018 · 9 comments
Labels

Comments

@zzuligy
Copy link

zzuligy commented Feb 23, 2018

the event loop graph is in the top down order,which make me confuse the poll phase.
if say poll phase like a moniter,which kick off the cycyle, loop and execute the callback in the phase may be most clear。

@zzuligy
Copy link
Author

zzuligy commented Feb 23, 2018

               ┌───────────────────────┐
            ┌─>│        timers         │
            │  └──────────┬────────────┘
            │  ┌──────────┴────────────┐
            │  │     I/O callbacks     │
            │  └──────────┬────────────┘
            │  ┌──────────┴────────────┐
            │  │     idle, prepare     │
         looper└──────────┬────────────┘
            │  ┌──────────┴────────────┐
            │  │        check          │
            │  └──────────┬────────────┘
            │  ┌──────────┴────────────┐
            └──┤    close callbacks    │
               └───────────────────────┘

i think this is clear

@fhemberger
Copy link
Contributor

/cc @evanlucas

@amandeepmittal
Copy link
Contributor

@zzuligy But then where will you put the retrieval of I/O events?

@dmcghan
Copy link
Contributor

dmcghan commented Mar 17, 2018

I think the confusion comes from another part of the doc being wrong. In the Phases Overview section, the I/O Callbacks bullet states:

executes almost all callbacks with the exception of close callbacks, the ones scheduled by timers, and setImmediate().

However, in the Phases Detail > I/O Callbacks section, the detail states:

This phase executes callbacks for some system operations such as types of TCP errors.

Unless I'm misunderstanding something, it's the poll phase that "executes almost all callbacks with the exception of close callbacks, the ones scheduled by timers, and setImmediate()".

This seems to be confirmed in the libuv doc on the I/O loop which says this regarding the poll phase (#8 in the doc):

All I/O related handles that were monitoring a given file descriptor for a read or write operation get their callbacks called at this point.

Perhaps the Node.js doc should change the phase names to be more in sync with the libuv doc.

  • I/O callbacks could be renamed to "pending I/O callbacks"
  • poll could be renamed to "poll for I/O"

So the Phases Overview would look more like this:

  • timers: this phase executes callbacks scheduled by setTimeout() and setInterval().
  • pending I/O callbacks: executes I/O callbacks deferred to the next loop iteration.
  • idle, prepare: only used internally.
  • poll for I/O: retrieve new I/O events; execute I/O related callbacks (almost all with the exception of close callbacks, the ones scheduled by timers, and setImmediate()); node will block here when appropriate.
  • check: setImmediate() callbacks are invoked here.
  • close callbacks: e.g. socket.on('close', ...).

@fhemberger
Copy link
Contributor

@dmcghan Sounds good, would you mind creating a PR for it?

@amandeepmittal
Copy link
Contributor

@dmcghan in pending I/O callbacks is it good to mention process.nextTick()?

@dmcghan
Copy link
Contributor

dmcghan commented Mar 26, 2018

@fhemberger Sure, will do.

@amandeepmittal I don't think so. process.nextTick() gets it's own section in the doc because it's technically not part of the event loop. It has its own queue that is processed after each phase.

@dmcghan
Copy link
Contributor

dmcghan commented Mar 26, 2018

I created PR #1603. I simplified "pending I/O callbacks" to "pending callbacks" and left the name of the "poll" phase alone since it was referred to in other places.

@ghost
Copy link

ghost commented Aug 11, 2018

Since PR #1603 is submitted now, so this issue should be over here. I'm closing it.
Thanks @dmcghan.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants