To tick or not to tick?

[overlookmotel]

1. What version of bluebird is the issue happening on?

v3.5.1

2. What platform and version? (For example Node.js 0.12 or Google Chrome 32)

Node v6.12.2, Mac OS 10.12.6

3. Did this issue happen with earlier version of bluebird?

Not sure.

---

I was interested by @benjamingr's comment on nodejs/promises#31 (comment) stating that Bluebird avoids an extra tick before calling a .then() handler if the promise it's chained onto has already taken a tick to resolve.

new Promise(r => setTimeout(r, 1)).then(x => console.log("Got here"))

In this case, bluebird sees that new Promise did not resolve synchronously and thus does not defer the console.log to the next tick (it was already deferred). It also does this optimization when promisifying.

Here is an example which seems to demonstrate the opposite:

const Promise = require('bluebird');

let resolve;
const p = new Promise( r => resolve = r );

p.then( () => console.log('then handler called') );

setTimeout( () => {
	console.log('resolving');
	resolve();
	console.log('resolved');
}, 1000 );

The promise is resolved asynchronously, and many ticks after .then() is called. So, if I understood what @benjamingr was saying right, I'd have expected the then handler to be called synchronously when the promise is resolved. But it's not. The output is:

resolving
resolved
then handler called

I also can't see a call to ._setAsyncGuaranteed() anywhere on the code path for the Promise constructor - which as far as I can see is what would prevent an additional tick being introduced.

Promise.promisify(), however, does achieve this optimisation:

let resolve;
const f = Promise.promisify( cb => resolve = cb );

const p = f();
p.then( () => console.log('then handler called') );

setTimeout(() => {
	console.log('resolving');
	resolve();
	console.log('resolved');
}, 1000);

outputs:

resolving
then handler called
resolved

Did I miss the point here?

[overlookmotel]

Incidentally, does the following break the Promises A+ spec, as the then callback is called before any tick has occured?

let resolve;
const f = Promise.promisify( cb => resolve = cb );

const p = f();
p.then( () => console.log('then handler called') );

console.log('resolving');
resolve();
console.log('resolved');

This outputs:

resolving
then handler called
resolved

[benjamingr]

You didn't miss any point, this optimization is performed for promisify and not for regular new Promise calls.

I've been hoping we figure out a way to do this in Node by the way, @bmeurer - what sort of work do you think would need to be done in util.promisify from the Node side so V8 can optimize the tick away like bluebird?

[overlookmotel]

@benjamingr Thanks for swift reply.

Ah OK. Is there a reason why the same optimisation isn't performed for new Promise calls? Looks like it'd be easy to implement, and would make the logic consistent.

And, yes I guess bluebird is shuffling slowly towards retirement, but until async/await gets a lot closer to Bluebird.coroutine() on speed, I'll be sticking with bluebird. ;)

Please also see my previous comment which crossed with yours about whether promisify (strictly speaking) breaks the Promises A+ contract.

[bmeurer]

TBH I don't know how this works under the hood in bluebird, and I'm also not entirely sure I understand how this currently works in Node with native promises, but looking at this naively it doesn't seem to be spec compliant.

[overlookmotel]

@bmeurer Please don't be put off by my point above. I think I may have found an edge case where bluebird is optimising too aggressively and breaking the spec. But this is an implementation error, not a demonstration that there's anything wrong with the optimisation in principle.

For what it's worth, I totally agree with @benjamingr's stance. There's a big performance gain to be had if this optimisation was implemented in native Promises, and in my opinion it can be implemented so it's completely compliant with the Promises A+ spec.

Here's an example:

// Promisified version of `fs.readFile()`
function readFileAsync(path, options, cb) {
    return new Promise( (resolve, reject) => {
        fs.readFile( path, options, (err, value) => {
            if (err) return reject(err);
            resolve(value);
        } );
    } );
}

readFileAsync('foo.txt').then( v => console.log(v) );

We know for sure that fs.readFile() will never call its callback synchronously. So the promise will necessarily be resolved after a tick has passed. Therefore, it is unnecessary to introduce an additional tick after the promise resolves, before calling the then handler (console.log(v)). That extra turn of the event loop is just a slow-down with no gain.

i.e. When a promise is known to have taken a tick to resolve, its then handlers can be resolved synchronously as soon as the promise resolves - without breaking the Promises A+ spec.

[bmeurer]

I don't think that this is a valid optimization in general:

function readFileAsync(path, options, cb) {
    return new Promise( (resolve, reject) => {
        fs.readFile( path, options, (err, value) => {
            // microtask queue is empty here, because this is called directly from the main event loop.
            // (a) First injection point
            if (err) return reject(err);
            resolve(value);
            // (b) Second injection point
        } );
    } );
}

Notice that Promises don't use the main event loop, but rather the microtask queue, so they generally don't tick. Now on to the example: As a human you know that when you enter the handler above, the microtask queue must be empty since it's called straight from the main event loop. V8 however doesn't know that, plus someone could call the handler from somewhere else. Then you need to ensure the neither the (a) nor the (b) injection points have any side effects or put stuff onto the microtask queue, otherwise the optimization will be wrong.

In general I doubt that the microtask queue is such a big issue here. Do you have concrete examples where you see the big performance gain?

[benjamingr]

In general I doubt that the microtask queue is such a big issue here. Do you have concrete examples where you see the big performance gain?

You mean specifically cases where deferring the execution an additional microtick causes a significant performance penalty?

Notice that Promises don't use the main event loop, but rather the microtask queue, so they generally don't tick.

That's not the case with bluebird by the way and as far as I know there is no guarantee about the microtick queue state when a promise handler is run other than that it depleted once before the promise ran.

Promises don't have any spec requirements for orderings like that that I am aware of. Resolution is (potentially) synchronous and then callbacks run after synchronous code has finished running (thens for the same promise run in order).

[bmeurer]

The spec doesn't leave any room wrt. order in which microtasks have to be executed. The order has to match exactly the order on which they were enqueued, and it's never valid to execute microtasks synchronously.

[benjamingr]

I'm not sure I understand, in your example:

function readFileAsync(path, options, cb) {
    return new Promise( (resolve, reject) => {
        fs.readFile( path, options, (err, value) => {
            // microtask queue is empty here, because this is called directly from the main event loop.
            // (a) First injection point
            if (err) return reject(err);
            resolve(value);
            // (b) Second injection point
        } );
    } );
}

What exactly is being injected? Promise.resolve().then calls? Maybe @domenic can weigh in on the execution order requirements and motivation?

Sorry if you feel like I'm beating a dead horse - I genuinely recall this optimization giving measurable gains when added to bluebird (which uses macrotask semantics for scheduling anyway which is allowed by A+ but would be weird in the ECMAScript spec) and I feel like it's worth exploring for JS.

[bmeurer]

Well, Promise.prototype.then, Promise.resolve, Promise.prototype.catch, await, etc. Anything that puts a microtask onto the queue essentially.

[domenic]

Whether an optimization is valid depends on whether one can write a JavaScript test demonstrating observable differences with or without the optimization. (If the test demonstrates differences, it's an invalid optimization.)

I skimmed this thread looking for such a test, and it seems like #1498 (comment) is one such case---if you implement resolve() according to the spec, the output would be resolving/resolved/then handler called, for any promise's resolve function. So the optimization that is applied to resolve() in that case is not valid.

[overlookmotel]

@domenic Just to throw my view in... Yes, I believe in the example I gave in #1498 (comment) does demonstrate bluebird optimizing too aggressively in this case.

But in my opinion that's an implementation error in bluebird in this specific edge case. I did not meant to discredit the optimization overall. I can see many other cases where this optimization can be applied and, as you say, there'd be no test which could identify whether the optimization had been used or not.

Am on the hop so can't write more at present but will try to find some time to write a more thorough explanation later.

[petkaantonov]

Promisify and promisifyAll are custom APIs that are not specced. The idea is that anything you would promisify is asynchronous anyway, so why produce further delays? It should only be observable when something you promisify is actually synchonous, as you noticed.