win,unix: change execution order of timers by trevnorris · Pull Request #3927 · libuv/libuv

trevnorris · 2023-03-16T18:51:09Z

The maximum number of times timers should run when uv_run() is called with UV_RUN_ONCE and UV_RUN_NOWAIT is 1. Do that by conditionally calling timers before entering the while loop when called with UV_RUN_DEFAULT.

The reason to always run timers at the end of the while loop, instead of at the beginning, is to help enforce the conceptual event loop model. Which starts when entering the event provider (e.g. calling poll).

Other than only allowing timers to be processed once per uv_run() execution, the only other noticeable change this will show is if all the following are true:

uv_run() is called with UV_RUN_NOWAIT or UV_RUN_ONCE.
An event is waiting to be received when poll is called.
Execution time between the call to uv_timer_start() and entering the while loop is longer than the timeout.

If all these are true, then timers that would have executed before entering the event provider will now be executed afterward.

@bnoordhuis I've also tested this in node.js and all tests passed. Want your feedback if this approach is appropriate, and if you think it's v1.x compatible.

vtjnash · 2023-03-16T19:51:36Z

This sounds sane to me, and easy to update in v2 to drop the first check. However, it means this diagram will need to be updated:

(included on this page https://github.com/libuv/libuv/blob/v1.x/docs/src/design.rst)

mmomtchev · 2023-03-16T19:56:08Z

This is simply running the timers after the polling - which was @vtjnash original proposal

I don't think that running the timers before entering the event loop will have any effect - there are usually no timers waiting when the event loop is started

I did test this back when the issue was initially discussed and the conclusion was that it didn't break anything in Node.js

Still, it appeared a risky change despite its elegance - but it is probably the best solution

santigimeno

LGTM. Not 100% it won't break anyone but if the Node.js test suite is not complaining we could give it a chance (we can always revert, right?)

trevnorris · 2023-03-16T21:13:41Z

@mmomtchev I must have missed that original proposal by @vtjnash.

I agree that it shouldn't make a difference, and Node.js has even stated that users shouldn't depend on the execution order of each event loop phase, but it's been outlined in documents such as https://nodejs.org/en/docs/guides/event-loop-timers-and-nexttick and it's what is expected at this point.

The following shows a crazy example, but one that would change execution order if we removed the first call to timers:

require('fs').open('/tmp/tmp.file', 'w', () => console.log('opened'));

setTimeout(() => console.log('setTimeout'), 10);

process.nextTick(() => {
  console.log('nextTick');
  const t = Date.now();
  while (Date.now() - t < 10);
});

Call order now is nextTick, setTimeout, opened. Whereas it would be nextTick, opened, setTimeout removing the initial timer processing call (if the file system is fast enough). Basically, it prioritizes running timers before processing events. Should it matter? No. Will it matter? Possibly. At least enough that I think it's necessary to always run timers before entering the loop in the case of UV_RUN_DEFAULT.

trevnorris · 2023-03-16T21:26:02Z

@vtjnash Is there a diagram file I can update to regenerate the loop_iteration.png file?

The maximum number of times timers should run when uv_run() is called with UV_RUN_ONCE and UV_RUN_NOWAIT is 1. Do that by conditionally calling timers before entering the while loop when called with UV_RUN_DEFAULT. The reason to always run timers at the end of the while loop, instead of at the beginning, is to help enforce the conceptual event loop model. Which starts when entering the event provider (e.g. calling poll). Other than only allowing timers to be processed once per uv_run() execution, the only other noticeable change this will show is if all the following are true: * uv_run() is called with UV_RUN_NOWAIT or UV_RUN_ONCE. * An event is waiting to be received when poll is called. * Execution time between the call to uv_timer_start() and entering the while loop is longer than the timeout. If all these are true, then timers that would have executed before entering the event provider will now be executed afterward. Fixes: libuv#3686 Co-authored-by: Momtchil Momtchev <momtchil@momtchev.com>

trevnorris · 2023-03-16T23:02:05Z

@vtjnash I tried my best to regenerate the graphic for the event loop. Let me know if you want any changes.

- linux: introduce io_uring support libuv/libuv#3952 - src: add new metrics APIs libuv/libuv#3749 - unix,win: give thread pool threads an 8 MB stack libuv/libuv#3787 - win,unix: change execution order of timers libuv/libuv#3927 - Fixes: nodejs#43931 - Fixes: nodejs#42496

- linux: introduce io_uring support libuv/libuv#3952 - src: add new metrics APIs libuv/libuv#3749 - unix,win: give thread pool threads an 8 MB stack libuv/libuv#3787 - win,unix: change execution order of timers libuv/libuv#3927 Fixes: nodejs#43931 Fixes: nodejs#42496 Fixes: nodejs#47715 Fixes: nodejs#47259 Fixes: nodejs#47241

- linux: introduce io_uring support libuv/libuv#3952 - src: add new metrics APIs libuv/libuv#3749 - unix,win: give thread pool threads an 8 MB stack libuv/libuv#3787 - win,unix: change execution order of timers libuv/libuv#3927 Fixes: #43931 Fixes: #42496 Fixes: #47715 Fixes: #47259 Fixes: #47241 PR-URL: #48078 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Jiawen Geng <technicalcute@gmail.com>

There should be no need to run timers prior to entering the event loop. Instead allow them to run normally at the end after the poll phase is complete. Ref: libuv#3927 Ref: libuv#4017

- linux: introduce io_uring support libuv/libuv#3952 - src: add new metrics APIs libuv/libuv#3749 - unix,win: give thread pool threads an 8 MB stack libuv/libuv#3787 - win,unix: change execution order of timers libuv/libuv#3927 Fixes: #43931 Fixes: #42496 Fixes: #47715 Fixes: #47259 Fixes: #47241 PR-URL: #48078 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Jiawen Geng <technicalcute@gmail.com>

- linux: introduce io_uring support libuv/libuv#3952 - src: add new metrics APIs libuv/libuv#3749 - unix,win: give thread pool threads an 8 MB stack libuv/libuv#3787 - win,unix: change execution order of timers libuv/libuv#3927 Fixes: nodejs#43931 Fixes: nodejs#42496 Fixes: nodejs#47715 Fixes: nodejs#47259 Fixes: nodejs#47241 PR-URL: nodejs#48078 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Jiawen Geng <technicalcute@gmail.com>

- linux: introduce io_uring support libuv/libuv#3952 - src: add new metrics APIs libuv/libuv#3749 - unix,win: give thread pool threads an 8 MB stack libuv/libuv#3787 - win,unix: change execution order of timers libuv/libuv#3927 Fixes: #43931 Fixes: #42496 Fixes: #47715 Fixes: #47259 Fixes: #47241 PR-URL: #48078 Backport-PR-URL: #49591 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Jiawen Geng <technicalcute@gmail.com>

trevnorris requested a review from bnoordhuis March 16, 2023 18:51

trevnorris self-assigned this Mar 16, 2023

trevnorris mentioned this pull request Mar 16, 2023

loop: avoid double calling the timers in UV_RUN_ONCE mode #3791

Closed

santigimeno approved these changes Mar 16, 2023

View reviewed changes

trevnorris force-pushed the move-timer-exec branch from f516c47 to 225d34a Compare March 16, 2023 22:58

bnoordhuis approved these changes Mar 17, 2023

View reviewed changes

trevnorris merged commit 6600954 into libuv:v1.x Mar 20, 2023

trevnorris deleted the move-timer-exec branch March 20, 2023 16:05

santigimeno mentioned this pull request May 19, 2023

deps: upgrade to libuv 1.45.0 nodejs/node#48078

Closed

vtjnash mentioned this pull request May 23, 2023

new timer_from_check test seems pretty flaky #4017

Open

trevnorris mentioned this pull request May 24, 2023

src: don't run timers before entering event loop #4026

Merged

This was referenced Jun 28, 2025

Unexpected server incoming requests queue after Node.js 20 version (possible performance regression) nodejs/node#57364

Open

Update event loop guide nodejs/nodejs.org#7914

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

win,unix: change execution order of timers#3927

win,unix: change execution order of timers#3927
trevnorris merged 1 commit intolibuv:v1.xfrom
trevnorris:move-timer-exec

trevnorris commented Mar 16, 2023

Uh oh!

vtjnash commented Mar 16, 2023

Uh oh!

mmomtchev commented Mar 16, 2023

Uh oh!

santigimeno left a comment

Uh oh!

trevnorris commented Mar 16, 2023

Uh oh!

trevnorris commented Mar 16, 2023

Uh oh!

trevnorris commented Mar 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

trevnorris commented Mar 16, 2023

Uh oh!

vtjnash commented Mar 16, 2023

Uh oh!

mmomtchev commented Mar 16, 2023

Uh oh!

santigimeno left a comment

Choose a reason for hiding this comment

Uh oh!

trevnorris commented Mar 16, 2023

Uh oh!

trevnorris commented Mar 16, 2023

Uh oh!

trevnorris commented Mar 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants