Repository navigation
MacroTask and MicroTask execution order #22257
Description
- Version: 10.8.0
- Platform: macOs High Sierra 10.13.6 / Windows 10
- Subsystem:
Hi,
I believe the topic is not new (see #17181, #15081) but nevertheless worth a new issue.
Currently there is a different in the execution order of Macro and Micro tasks between browsers and NodeJS that leads to different behaviors on shared code across both platforms.
In the past browsers had inconsistent behavior but currently all major browsers - Chrome (v68), Edge (v42), Safari (v11.1) and Firefox (v61) - behave the same.
Example:
for (let i = 0; i < 2; i++) {
setTimeout(() => {
console.log("Timeout ", i);
Promise.resolve().then(() => {
console.log("Promise 1 ", i);
}).then(() => {
console.log("Promise 2 ", i);
});
})
}Results:
Chrome / Edge / Safari / Firefox
Timeout 0
Promise 1 0
Promise 2 0
Timeout 1
Promise 1 1
Promise 2 1
Node
The result is actually inconsistent. From my tests, the most frequent result is:
Timeout 0
Timeout 1
Promise 1 0
Promise 2 0
Promise 1 1
Promise 2 1
But sometimes it matches the one given by the browsers.
The reason why I bumped into this issue is that I need to hook a callback on the end of every event loop turn. In my case, ensuring that all microtasks are executed before the start of the next macrotask would easily solve the problem, and work everywhere.
I understand the impact of such a change in the engine, and that it could only be done in a next major version. Despite of that, having an alignment with how all browsers behave seems to me at least worth considering the topic one more time.
Thank you for you time in advance,
Best,
jpsfs
PS: Before finding the issue linked above, I created a question on StackOverflow. I will leave it here for future reference.
Activity
addaleax commentedon Aug 10, 2018
@nodejs/timers
devsnek commentedon Aug 10, 2018
node doesn't use the language microtask queue for timers, it has another queue above that which runs the {timer queue, immediate queue, etc} and the microtask queue.
addaleax commentedon Aug 10, 2018
@devsnek I do think that this is a bug that should be fixed, to be clear.
devsnek commentedon Aug 11, 2018
@addaleax agreed 😄
TimothyGu commentedon Aug 11, 2018
For reference, here's how HTML defines the event loop for renderer threads and worker threads: https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-processing-model. Definition wise, a "task" in HTML parlance is a "macrotask" unless specified otherwise. A quick summary:
The way
setTimeout()works is roughly:In comparison, the way Node.js timers run is this:
(The "thread" here is idealized; libuv may or may not use threads to implement timers.)
This results in all timers of a specific timeout being run together rather than having microtasks run after each timer callback.
BTW, I realize that people who have commented here already probably knows how everything works already, but in case a summary is needed by anyone for context.
jasnell commentedon Aug 11, 2018
Before we call this a bug in Node.js, there really needs to be a clear description of what the expected behavior is. Timers in Node.js have always been different than timers in the browser and JS does not define their behavior.
/cc @Fishrock123
benjamingr commentedon Aug 11, 2018
I think we should be very careful in what timing guarantees we make about this sort of thing because every guarantee we make means we have to keep it rather than optimize if we get the chance in the future.
That said - in this particular case I believe we should align in browsers and always run microticks before timers if at all possible. This has implications on hooks people use for debugging and instrumentation and we should do it both because it aligns with browsers and because it’s right.
Fishrock123 commentedon Aug 13, 2018
It seems like there are some misconceptions here
That's not actually possible. Please re-verify that.See below
setTimeout()could, for all we know, be implemented using the same system that Node.js uses. It doesn't really matter.As far as I'm aware neither one works like this. IIRC the MTQ works roughly the same as our nextTick queue except I think it can be interrupted by browser frame renders (and then maybe RAF, not 100% sure on that).
The proper behavior, at least in node, should be to run the MTQ, then nextTicks, after "sync user code" is finished running.
As with all things adjusting the behvaior and order of timers, this is probably a moderately risky change.
It is possible the manually run the MicroTaskQueue in between timer queue items (again, the timer system does not actually matter). nextTick already has this exact behavior.
That being said, it seems like the root cause of this is that nextTicks are not called between timer invocations. I can't think of a good reason why that is the case other than that it has always been the case (literally).
I think if we make any change, we should call the nextTick queue, which would then call the MTQ. That should keep ordering consistent and the way we want. Immediates also have this issue.
I can make a patch pretty easily, but testing it's impact is likely quite difficult.
TimothyGu commentedon Aug 13, 2018
It'd be great if you could. I see it as a net plus if we could get this working without performance regressions.
Fishrock123 commentedon Aug 13, 2018
Interestingly, calling the MTQ for individual timer callbacks has no effect on the Node.js test suite, but doing it from immediates seemingly causes all sorts of explosions.
Fishrock123 commentedon Aug 13, 2018
Ok well it actually gets completely stuck.
[10:16|% 99|+ 2353|- 10]: release test-vm-ownpropertysymbols
addaleax commentedon Aug 13, 2018
@Fishrock123 I can reproduce it locally (Linux).
devsnek commentedon Aug 13, 2018
@Fishrock123 i can also reproduce that (macos)
apapirovski commentedon Aug 14, 2018
Definitely possible since the timers could get set on a different ms and thus expire 1ms apart.
Anyway, interleaving nextTick is quite easy but it will have impact on the ecosystem as there are a non-trivial number of packages that rely on the current behavior.
apapirovski commentedon Aug 14, 2018
Also worth mentioning that it has non-trivial impact on peformance of timers and immediates.
benjamingr commentedon Aug 14, 2018
Can you name one on top of your head? Should we perhaps make a PR and get a CITGM run for breakage?
timers: run nextTicks + MTQ between timers / immediates
Fishrock123 commentedon Aug 14, 2018
PR's up at #22316... idk how to stop the automated tests though... they are just gona stall forever.
Edit: started a citgm run: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1506/
Fishrock123 commentedon Sep 26, 2018
Ok so the current state of implementation is at #22842 - it works, but it is pretty costly (performance-wise) to do. (See that PR for more details & profiles.)
devsnek commentedon Sep 26, 2018
Also, if you're using setTimeout as a form of immediate as the OP is, browsers and node are working on shipping out a method called
queueMicrotaskwhich better serves the task. It is currently available in safari tech preview and node nightly builds. (chrome and firefox have bugs open to implement it, so feel free to find those and comment to push them forward)Fishrock123 commentedon Sep 28, 2018
No, that is not a correct solution to the actual problem, Gus.
And besides it won't run in between anyways.
benjamingr commentedon Sep 28, 2018
@devsnek to add to what Jeremiah said - would you mind explaining how that solves the timing issue?
devsnek commentedon Sep 28, 2018
@benjamingr maybe i just misunderstood the intention of the example the OP gave.
benjamingr commentedon Sep 28, 2018
@devsnek the way I understand it (which might be off) is that this issue is about whether or not we want to make guarantees about the order of microtasks and macrotasks (for example "no timers execute between promises and if a promise queues a nextTick that executes before any additional timers").
timers: run nextTicks after each immediate and timer
timers: run nextTicks after each immediate and timer