use immediate instead of setImmediate

[nolanlawson]

Using setTimeout is slower than using immediate, which defaults to using MutationObserver to make microtasks. In the browser, I notice a lot of time is just spent idle because it uses setTimeout. So we should avoid that.

[coveralls]

Coverage remained the same at 93.13% when pulling c9b48fce3c4fde6d6a729a1bd3ec9a33b056e8e3 on immediate into 8cf887082218e72b946ceb7194ef98b5e73b3658 on master.

[juliangruber]

👍

[calvinmetcalf]

I would suggest replacing only process.nextTick with immediate and keeping setImmediate as a first choice as immediate with block io during recursive calls (in fact the current implementation of process.nextTick is based on immediate so they are very similar except for initial latency of non recursive calls)

[nolanlawson]

So you would say to use global.setImmediate || require('immediate')? Isn't this basically going to give one experience to IE/Edge and another experience to every other browser? Or should we just use setImmediate in Node but not the browser?

[nolanlawson]

Also it's unclear to me why we should worry about the I/O blocking – I wouldn't expect MemDOWN to block any less than a regular synchronous in-memory database, which of course would block a lot. My primary desire for MemDOWN is just to be fast; if it blocks, then folks should move it to a web worker (maybe in Node though the concerns are different).

[calvinmetcalf]

I don't know, did they fix the IE bug where setImmediate could bloke the event loop? if not then they'll ~ the same thing.

As for blocking the thing is if you do quite a lot of calls recursively, like write 100 things but one at a time and then make the next call in the call back then something truly async would allow other callbacks to fire between the async calls. Or put another way I'm suggesting you use a macrotask not a microtask

Also this is a discussion of latency not performance

[nolanlawson]

You don't have to say "they;" the person who implemented setImmediate works like one floor up from me and I could just ask them. :wink:

I tested out the demo page linked from that GitHub issue, and I'm not sure what I'm supposed to be seeing, but I see the same effect in Edge 14 desktop, Edge 14 mobile, and IE 11 desktop. I assume in the broken case that it would stop outputting logs? The test case is not super clear.

As for allowing other callbacks to interleave, my main concern is that process.nextTick() under the hood seems to contribute to a lot of idle time when running memdown in the browser, which we can eliminate by using microtasks. If you compare memdown to something like LokiJS (which is synchronous), then the overall runtime is larger because of the timeouts. (In fact right now the in-memory adapter for pouchdb-server is actually slower than CouchDB even though it doesn't write to disk, just because of all the idle time (I think)).

To be fair though I need to actually run benchmarks instead of just making wild claims. Maybe there's a better way to speed up memdown.

[calvinmetcalf]

your benchmark is probably testing latency more then performance, if you parallelize it by running like running 1000 or 10000 at once you might get a more representative demo for a real world app where other things are happening at the same time

[nolanlawson]

Sure, it's latency, not perf. Just trying to figure out when to use setTimeout and when to use setImmediate/immediate; it's really not clear to me. Yes microtasks block I/O, but so do literally every other thing in JavaScript except setTimeout/setImmediate, and in the browser setTimeout has basically a built-in 4ms latency cost. For memdown it seems super conservative to use setTimeout in the browser, but maybe I'm wrong.

if you parallelize it by running like running 1000 or 10000 at once you might get a more representative demo for a real world app where other things are happening at the same time

I'll try to come up with some numbers for PouchDB, but in the test suite alone I see a huge amount of idle time, because it doesn't parallelize everything (since it can't, since it's writing to disk).

[nolanlawson]

Related: https://github.com/pouchdb/pouchdb/pull/5645

[nolanlawson]

Gonna defer to @calvinmetcalf's judgment here, at least until I can prove a big perf diff and that the I/O blocking is worth it.

[calvinmetcalf]

by the way I was suggesting global.setImmediate || require("immediate"); instead of just require("immediate"); not scuttle it entirely

[nolanlawson]

setImmediate makes sense for Node, but for the browser I worry about putting IE/Edge down different code paths from other browsers. This has actually come up a lot in my web compat work at Edge and it's surprising how often this can have a negative impact on Edge.