Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Guarantee future-turn resolutions #12

Closed
briancavalier opened this Issue · 15 comments

6 participants

@briancavalier

Currently, when.js doesn't guarantee future-turn promise resolutions. Callbacks might run immediately, in the current stack frame. The Promises/A draft spec leaves it unspecified.

Here are some pros and cons I can think of:

Pros

  1. Removes all doubt as to whether a callback will run immediately or in a future turn. Currently, a callback registered with when() or .then() might execute immediately or in a future turn depending on the state of the promise (and potentially on the implementation if there are other promise implementations besides when.js in the app)
  2. Protects variables in the current stack frame from being modified by a callback that captures them and might execute immediately. Related to 1, but wanted to call it out, specifically.

Cons

  1. Performance. This is the big downside in my mind. Even in environments that provide a fast alternative to setTimeout(), there will be a hit. For environments that only provide setTimeout(), the hit will be even more significant, due to low timer resolution.
  2. Implementing this in an optimized, cross-environment way to take advantage of faster alternatives where available will obviously make the lib larger--not by a huge amount, but worth noting.

I've been using promises in JS heavily for over a year and honestly, I've never been bitten by, or even surprised by, a callback being called in the current turn when I didn't expect it to. I've also never run into a situation where I felt like I needed future-turn guarantees to implement something correctly.

All of my work has been in browsers, though, so I don't have a good feel for how important this issue is in a server environment, or how important the performance issue is there.

I'd love to hear what people think about this.

@unscriptable

I have had problems with code that executed callbacks too soon in the past -- but those times were when using callbacks, not promises/deferreds. I've probably only hit it once since using formal promises, if at all. Devs who aren't accustomed to async programming will probably hit this problem, too.

Related ticket on curl.js: https://github.com/unscriptable/curl/issues/48

It would be great to have an option. Even better: a when decorator module that guarantees next-turn semantics.

define(/* when/next-turn */, ['when'], function (when) {
    // override when's methods somehow. i know this isn't trivial
});

AMD doesn't (yet) define a method for passing configuration information through to packages. I've been toying with the idea in curl.js. This would allow devs to specify a parameter in the loader's config:

curl({
    packages: {
        when: {
            config: { nextTurnGuarantee: true },
            path: 'support/when'
        }
    }
});

and then when could ask for the config in one of these ways:

define(/* when */, ['config'], function (config) {
    if (config.nextTurnGuarantee) { /* ... */ }
});


define(/* when */, ['module'], function (module) {
    if (module.config.nextTurnGuarantee) { /* ... */ }
});


define(/* when */, ['require'], function (require) {
    if (require.config.nextTurnGuarantee) { /* ... */ }
});

The latter two are safer. I wonder if CommonJS Modules/2.0 has tackled this issue.

-- J

@briancavalier

Very interesting ideas, John! I've also been thinking about ways to allow it to be configurable. Being able to pass options thru the loader config looks really interesting--I was wondering if something like that existed--I'd definitely be interested to hear if CommonJS Modules/2.0 deals with it. I think a decorator might be impossible without exposing some promise internals, but I'll think through it a bit more.

Whatever the mechanism, I'm thinking it needs to be some sort of "one shot" configuration. Once you set when.js to operate in either "allow immediate" or "guarantee future-turn" mode, there's no way to change it for the lifetime of the app execution. Otherwise, other code could change the setting and potentially break the app. Am I being too paranoid? :)

@johnyanarella

Given the significant cons, it seems like a situation where you'd want to opt-in to future turn resolution only in the scenarios where you really need it.

I like the hierarchy of when,when.all, when.some, when.any, etc. methods you've invented in this project. I think they read well. Why not continue that pattern here?

How about:

when.later( computeResult() ).then( handleResult );
@unscriptable

Hey @johnyanarella, I like your idea. We've thought of calling it when.soon() or when.asap() as well. It's a bit tricky to do it that way, iirc. Still an option, I hope. -- J

@briancavalier

Hi @johnyanarella, thanks for the input.

Have you run into any situations where you were bitten/surprised by a promise callback being executed immediately? Or a situation where you felt like you needed a future-turn guarantee to implement something correctly? Just trying to get a feel for whether this is even an issue or not. I've had at least one when.js user indicate that they would rather have the faster performance than the future-turn guarantees.

But yeah, we had talked about that kind of approach. I really like the simplicity and explicitness of it.

tl;dr I'm definitely open to something like when.later() if it makes sense and doesn't cause additional confusion.

I think there are some things to consider with it. I really don't know if these are problems, or if anyone will care, but I want to throw them out there for discussion.

  1. As a whole, it doesn't let when.js make any stronger guarantees than it does now. For example, if someone hands you a promise, you can't assume it will resolve in a future turn.

  2. There still won't be a way to guarantee a future turn resolution using promise.then() without changing the promise implementation itself. So a consumer of a when.js promise must use when.js's when.later(), or their own future-turn-guaranteeing when(). Does this make interop with other promises a bit more vague?

  3. How to also provide future turn resolutions for all the other methods, like when.all, when.reduce, etc? One option would be to add a .later() to every method, e.g. when.all.later(<same args as when.all>). I'm not so crazy about that, but it'd be fairly intuitive for people to learn. Another option is forcing people to nest: when.later(when.all([..]), doSomething). Other options?

  4. It may create some interesting situations with ordering of callbacks. Right now, when.js guarantees that callbacks will be invoked in the order they are registered. For example:

// doStuff will always execute before doMoreStuff
when(promise, doStuff);
when(promise, doMoreStuff);

Now consider this:

// which callback runs first now depends on the implementation of the promise, which could
// vary if there are multiple promise producers in play (dojo, Q, jQuery, etc.).  If the promise itself
// doesn't guarantee future-turn, these may run in "reverse" order
when.later(promise, doStuff);
when(promise, doMoreStuff);

This doesn't seem too bad when you are the one making both those calls, but consider if another library uses when.js to do this:

function makeAPromise() {
    var promise = doSomethingThatReturnsAPromise();
    when.later(promise, doStuff);
    return promise;
}

And someone using that library does:

var p = makeAPromise();
p.then(doCoolStuff);

Now it's possible, depending on the implementation of the promise (which may not be a when.js promise, again e.g. jQuery, dojo, Q), for the callbacks to run in either order, and it's less obvious to either party as to why. So, allowing people to mix and match immediate and future turn resolutions may create more confusion.

If when.js's promise implementation, rather than when.later() guaranteed future-turn, then all when.js methods would become future-turn. Again, for better or for worse :)

On the other hand, the situation right now might not be much better. Since when.js doesn't force future-turn resolutions, you can't assume anything about when your callback will be called. So maybe something like when.later doesn't make that situation any worse :)

I wonder if reversing the when.later() logic makes more sense--having when() guarantee future-turn, and having a when.asap() that allows (but doesn't guarantee) immediate resolution, if the promise implementation allows it (since ultimately when() delegates to promise.then()).

Again, all these are just things for us to discuss. I'm definitely open to something like when.later() if it makes sense and doesn't cause additional confusion.

@briancavalier

I'm currently leaning toward the idea of having when(), and promise.then() always guarantee future turn, but when.asap() allows a callback to be called immediately (but possibly can't guarantee it, not sure yet).

That said, I'm still not 100% convinced this needs to go into 2.0. I've still not run into a compelling use case, but I'm keeping it in the 2.0 milestone for now.

@briancavalier

After much discussion, we've decided async resolutions is the right thing to do. You can read more in the cujojs google group. In limited initial performance testing, the performance has been acceptable in node+v8 and chrome. Barring any performance disasters in other VMs, when.js 2.0 will be async.

We're considering offering an option to opt in to sync resolutions, if you absolutely-positively need the best possible performance. Not sure what that option will look like yet, but we'll be discussing more before 2.0.

@medikoo

@briancavalier I think aside of performance, we should also discuss logical reasoning behind that.

Promise is an object (not a function per se) that can be in one of two different states, resolved or unresolved. It seems logical to me that on resolved promise callbacks are invoked immediately.

Thing that we should definitely obey is that asynchronous operation that we just initialized must not return immediately.
So asynchronous function that returns promise, should return only unresolved promises and I'd say this is the rule we should obey.

If above is guaranteed I'd say that in about 80% of cases in algorithm we know whether promise that we have is resolved or not, and there are no surprises, at least I worked out already a few really complex flows and fact that resolved promise calls callbacks immediately was definitely not an issue on my side.

What's also important: I think this decision should be based on real world experiences and not just theory which happen to fail until put into practice.

@medikoo

Other thing to think about: Is it ok, that library that helps to deal with asynchronicity introduces some extra asynchronicity on its own?

@briancavalier

Hey @medikoo, thanks for jumping in. There's plenty of additional discussion of this over in the cujojs google group, too.

I've not been bitten by any problem related to synchronous resolutions. However, after lots of discussion, I do think it's important to provide the most consistent behavior by default. We'll probably still provide an option to opt in to sync resolutions if you need the speed.

Developers who aren't as experience with promises are more likely to be tripped up in situations where a promise might call a callback immediately or might call it asynchronously because developers will tend to just "write code until it works", and may not test both cases.

Is it ok, that library that helps to deal with asynchronicity introduces some extra asynchronicity on its own?

This is a tricky one, imho. The problem is that if a promise library doesn't force nextTick, then it's impossible to guaranteed consistent behavior (i.e. callbacks might be called immediately, or might be called later). Forcing nextTick is the only way to make that situation consistent.

I agree, though, that it's not a magic bullet. I have seen some instances where forcing nextTick can cause issues with other things, like streams and event emitters, where forcing nextTick can cause a callback to miss an event. There are ways to solve that, just as there are ways to program carefully enough to avoid sync vs. async problems.

I'd def encourage you to read that cujojs discussion, and especially the Mark Miller info that @domenic points to.

@Twisol

For what it's worth, I think the default behavior should be "correct" and consistent, with as few surprising corner cases as possible. Promises, at least to.me, represent values in the future, and operations on these values necessarily also take place in the future. (A temporal 'fmap', if you will.) Allowing promises to sometimes be "present" values complicates the picture.

A when.asap() to override the default would be a nice way to bend the normal rules, and it also forces you to call out the unusual usage explicitly. I think a different model would be better conceptually, but it doesn't really hurt to have it in practice.

@briancavalier

One potential source of confusion with when.asap is that it can't guarantee synchronous resolutions, but rather can only allow them. Let me try to explain ...

For a promise to guarantee future turn resolutions, the machinery inside its then method must guarantee them. Since when and when.asap must be implemented using a promise's public API, i.e. then, they are somewhat dependent on the implementation of whatever promise is passed to them. If when and when.asap aren't implemented using the public then API, but rather some internal back doors, they become useless for any promise that doesn't support those back doors ... which defeats much of the purpose of when.

So, if an async promise (one whose then machinery guarantees future turn resolutions) is passed to when.asap, there's no way for when.asap to "un-asyncify" that promise.

That's not necessarily bad (its basically how when.js < 2.0 behaves now), but I just wanted to point out the subtle difference between guaranteeing sync and allowing it.

@briancavalier

The dev-200 branch is fully async, and will be the basis for the upcoming 2.0 release. Closing as there's no real work left to do here.

@mikaelkaron

What was the eventual performance impact?

@briancavalier

I'll collect some real numbers and post.

Until then, I can say that it tends to vary based on the characteristics of the situation. For some situations it can be almost no difference or a small constant. The reason is that 2.0 uses an extensible trampoline to run handlers, and can avoid actual next ticking a lot of the time.

For example, here's a simple test that creates 100 promise chains, each 100 promises long. When.js 2.0 can complete this test in < 50ms in Chrome on my machine using setTimeout as its tick mechanism. The tick conflation algorithm is basically able to avoid all but 1 setTimeout call in that example :)

Also, if you crank up the numbers in that test, the time will scale linearly.

It is possible to create situations where it simply can't conflate ticks. In those situations, the perf will scale with the number of ticks incurred. The particular ticking mechanism starts to matter more as well. For example, using setTimeout will become very slow, whereas using a setImmediate/nextTick polyfill will tend to stay faster longer.

There are some interesting advantages to the async-ness. Things that I knew about but am appreciating even more now that I'm using it:

  1. You can't blow the stack ... or at least, if you do, it's not when.js's fault :) Want to crank the example above up to promise chains that are 10000 or even 100000 promises deep? Sure, go for it! I've done it, 100 x 10000 promises completes in about 6 seconds in Chrome

  2. The consistency of handlers always being invoked later makes for some nice programming patterns. For example, being able to setup a scenario early in a block of code using then(), and not having to think about when it will happen (it will always happen later). I'm also finding that code can be refactored more easily and without much thought--move a then() around, and it is guaranteed not to change the behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.