Make setTimeout and setImmediate timers run consecutively

[bnoordhuis]

Continuing from libuv/libuv#1035 (comment):

Node.js uses a uv_idle_t handle to implement setImmediate() timers but I think it could - and should - hang them off the big setTimeout() timer.

Right now you have this weird situation where:

a) setTimeout timers run,
b) some other callbacks run (mostly close callbacks), then
c) setImmediate timers run.

[Fishrock123]

implement setImmediate() timers

What are these? Just Immediates or timers scheduled in Immediates?

Immediates should fire after I/O before Timeouts, right?

[bnoordhuis]

Just Immediates or timers scheduled in Immediates?

Immediates.

Immediates should fire after I/O before Timeouts, right?

That's right, the actual callbacks are made from a check handle.

I realize what tripped me up: we call uv_run(UV_RUN_ONCE) in a loop, causing the big setTimeout timer to run twice per loop tick sometimes.

The order of events is approximately: start tick -> timeouts -> I/O -> immediates -> close callbacks -> timeouts again -> end tick.

[Trott]

Should this stay open?

If so, @Fishrock123: Are you likely to get to this soon? If not, should we unassign you and/or add mentor available and help wanted labels?

[Fishrock123]

I realize what tripped me up: we call uv_run(UV_RUN_ONCE) in a loop, causing the big setTimeout timer to run twice per loop tick sometimes.

The uv_run(UV_RUN_ONCE) part was changed in 9e08695 ... @bnoordhuis any idea if the ordering issues are still present? (Although we may want to make the change regardless.)

[bnoordhuis]

9e08695 should have fixed the ordering, yes. The issue of timer callbacks and immediate callbacks getting interleaved with other callbacks still exists.

[j0t3x]

Is this taken? I would like to work on this one.

[apapirovski]

Hi @j0t3x, unfortunately I don't know if there's a clear direction here even... It's a strong possibility that there might be code out there relying on immediates running after I/O.

(For reference http://docs.libuv.org/en/v1.x/design.html)

Also, the fact that setImmediate will run within the same iteration of the loop for everything but close callbacks is IMO a good thing.

[j0t3x]

The issue of timer callbacks and immediate callbacks getting interleaved with other callbacks still exists.
@apapirovski how can I help investigate this one?

[apapirovski]

I've removed the help-wanted label. I'm not sure anyone truly knows what's best here.

I would be in favour of sticking with the current behaviour but of course there is some weirdness associated with it. If we hang them off the timer handle then scheduling them seems to become a bit more problematic — I'm not 100% certain how @bnoordhuis envisions that working, but he also knows libuv a lot better than I so I trust that it's possible in an elegant manner.

[bnoordhuis]

If we hang them off the timer handle then scheduling them seems to become a bit more problematic

How so?

[apapirovski]

As far as I can tell they would need to be timers scheduled for 1ms, right? (Scheduling for 0 while in the timer handling process seems to cause those timers to fire again before running the loop again.)

Or is there some other way to hang them off the timer? As I said, only mildly knowledgeable about libuv.

[Fishrock123]

I'm not sure part of this issue exists anymore, since we no longer use uv_run(UV_RUN_ONCE).

[Fishrock123]

Actually, I'm not sure there is an issue at all anymore.

The definition of setImmediate is, according to the docs:

Schedules the "immediate" execution of the callback after I/O events' callbacks.

The OP mentions that close handles can fire before immediates after timeouts, but the code of uv_run seems to indicate that isn't true: https://github.com/libuv/libuv/blob/7284adfa7a833264fc67d68d39610da27ecbab7c/src/unix/core.c#L350-L399

The uv loop executes callbacks in the following order each loop:

1. timers
2. pending
3. idle
4. prepare
5. anything that goes to i/o polling
6. check
7. closing

[apapirovski]

This should just be closed at this point. There's nothing actionable here, at least nothing that anyone is realistically willing to do.