nodejs.org icon indicating copy to clipboard operation
nodejs.org copied to clipboard

Possible incorrect description of pool phase in event-loop-timers-and-nexttick guide(english language)

Open nikosson opened this issue 3 years ago • 4 comments

Hello. I guess description of the poll phase is wrong a bit, or at least it's not clear - in underlined places, there should be not timers but callbacks from check phase, which are setImmediate. While reading this phase it seems that it's about timers phase, mentioned above, while according to the real implementation after the pool phase a check phase must be executed, and setImmediate callbacks must be handled. Maybe I'm wrong, but I didn't find a place where there is such a description of a pool phase, also in libuv doc such statement is also missing. Знімок екрана 2022-02-13 о 22 51 38

Could you verify, please, where this is an invalid description?

nikosson avatar Feb 13 '22 21:02 nikosson

@nodejs/timers @nodejs/libuv

Trott avatar Feb 15 '22 01:02 Trott

This is a good question, I re-read this part and searched for different doc to get this clear in my mind. I believe the statements are correct but illustration here is not intuitive. I could be wrong but here is what I understand after deep diving.

Assume following code was run, (to make it very simple, I just use pseudo and take away a lot of details/assumptions)

// time t = 0
setTimeout(() => {
  statement 0;
}, 150);
statement 1;

// New loop, time t = 100
statement 2;

// New loop, time t= 200
statement 3;

At t = 0, setTimeout placed statement 0 to the queue with timeout 150ms for example. Then statement 1 is run. statement 0 was not run because timer scheduled for 150ms but currently 0ms passed.

Now the 2nd cycle, timers are checked we see 1 in the queue which is statement 0. Unfortunately t is only 100ms < 150ms. statement 0 is "ignored" and statement 2 is run.

In the 3rd cycle, timers are checked we see 1 in the queue which is statement 0. This time, since 200ms > 150ms, statement 0 needs to be queued to the timers phase to execute. So we put statement 0 to timers phase and run statement 3.

============================================================

Now back to the question. The 1st underlined statement no timer scheduled simply saying that there are no setTimeout, setImmediate, nextTick is in the queue. So the case is simple, nothing needs to be checked / scheduled to timers phase. Happily run whatever in the poll queue.

And then the 2nd underlined statement, it means after poll queue is cleared, poll phase will not end immediately but DOUBLE CHECKING if there are really no timers remains (because some timers might be added when processing the poll queue). If there is 1 or more timers and its time thresholds have been reached (3rd + 4th underlined statement), put them into timers phase and run in that timer queue.

============================================================

Again, I am also trying to learn, and learning through digesting+explaining. Please correct me if what I answered was wrong. Hope it helps kickstart the discussion of this great question.

source

jeffreyleeon avatar Jul 23 '22 16:07 jeffreyleeon

Yes, that part is really confused.It only tells us what event loop will do when there are no timers scheduled, but what's going on when there are some timers scheduled?

To clarify it, let's look at a simple example, and analysis the result according to that part of doc:

const fs = require("fs");

// no timers scheduled!

// size of test.txt is really small!
// so, in Poll Phase, queue is not empty
fs.stat("./test.txt", () => {
   console.log("hello")
});

setImmediate(() => console.log("world"))

// according to the doc, the result will be:
// hello
// world
//
// in my machine, node v12.22.12, it's right
const fs = require("fs");

// no timers scheduled!

// size of test.txt is not really small!
// so, in Poll Phase, queue is empty
fs.readFile("./test.txt", () => {
   console.log("hello")
});

setImmediate(() => console.log("world"))

// according to the doc, the result will be:
// world
// hello
//
// in my machine, node v12.22.12, it's right
const fs = require("fs");

// timers scheduled
setTimeout(() => console.log("sunshine beach"), 50);

// size of test.txt is not really small!
// so, in Poll Phase, queue is empty
fs.readFile("./test.txt", () => {
   console.log("hello")
});

setImmediate(() => console.log("world"))

// oh, this case is ignored in doc!
// and the actual result is:
// world
// hello
// sunshine beach

So, nodejs official should better talk about the event loop more clearly in doc.

Good documents make a good day.

zhangzhuang15 avatar Jan 05 '23 02:01 zhangzhuang15

Just to clarify: we're open for contributions here :)

ovflowd avatar Jan 07 '23 16:01 ovflowd