Remove attributes on Future

[mounirlamouri]

The Future interface currently has .value, .state and .error. It doesn't seem that those attributes are common in the Javascript libraries/definitions of promises [1][2][3]. Indeed, those values are simply useless because in a success or error callback, they are all known because of the context.

For example:

asyncCall().then(function(result) {
  /* We know that:
    - .value == result;
    - .error == undefined;
    - .state == "accepted" */
}, function(error) {
  /* We know that:
    - .value == undefined;
    - .error == error;
    - .state == "rejected" */
});

There is also:

var future = asyncCall();
/* At that point, we know that:
  - .value == undefined;
  - .error == undefined;
  - .state == "pending". */

The only case where we might need to know what .value, .error and .state values are is this one:

var future = asyncCall();
methodThatWillProduceMultipleTripToTheEventLoopBeforeReturning();
// We no longer know the state of |future|, we might want to check with .state.

However, I believe that .then() should behave so that this is working:

var future = asyncCall();
methodThatWillProduceMultipleTripToTheEventLoopBeforeReturning();
future.then(function(result) {
}, function(error) {
});

The .then() method should work even if |future| was no longer pending. I we agree on that behaviour (I am not sure if the documentation specify that and, honestly, it is hard to read un-formated documentation), I think we could simply remove those three attributes and make the Future interface slimmer.

[1] http://wiki.commonjs.org/wiki/Promises/A [2] https://github.com/kriskowal/q [3] http://promises-aplus.github.com/promises-spec/

CC: @annevk @sicking @slightlyoff

[mounirlamouri]

BTW, I would gladly do a PR if we agree on that change. I thought that the PR before the discussion would be a bit aggressive...

[mounirlamouri]

The attributes would still be needed for EventedFuture but I filed issue 35 to remove EventedFuture so if we agree that the attributes are not needed in Future they might need to move to EventedFuture or not depending on the resolution of issue 35.

[DavidBruant]

+1

[sicking]

The advantage with having the attributes is that it allows some performance optimizations when you have a Future which may have been resolved already.

Always using the .then() function means that you have to take a trip back to the event loop, even if the Future has already been resolved. Going back to the event loop is significantly slower than synchronously grabbing a value if the value accessor is simple (as is the case here).

Another advantage of the attribute is that if you have code which is waiting for a number of asynchronous conditions to become true, then you can use the promise object itself to track if that particular asynchronous conditions has been fulfilled. I.e. you can do something like:

var clickTarget = null; myElement.onclick = function(e) { clickTarget = e.target; checkIfDone(); }; var idbResult = null; loadDataFromIDB(..., function(result) { idbResult = result; checkIfDone(); }); asyncActionFuture = doAsyncAction(...); asyncActionFuture.then(checkIfDone);

function checkIfDone() { if (!clickTarget || !idbResult || asyncActionFuture.state == "pending") return;

doStuffWith(clickTarget, idbResult, asyncActionFuture.value); }

If all asynchronous APIs used futures, it would be easy to use Future.all(asyncAction1, asyncAction2, ...). However it we're far from having all APIs using futures. And some APIs likely aren't appropriate to ever expose through futures, like checking for clicks.

[sicking]

Let me try that again

var clickTarget = null;
myElement.onclick = function(e) {
  clickTarget = e.target;
  checkIfDone();
};

var idbResult = null;
loadDataFromIDB(..., function(result) {
  idbResult = result;
  checkIfDone();
});

asyncActionFuture = doAsyncAction(...);
asyncActionFuture.then(checkIfDone);

function checkIfDone() {
  if (!clickTarget || !idbResult || asyncActionFuture.state == "pending")
    return;

  doStuffWith(clickTarget, idbResult, asyncActionFuture.value);
}

[DavidBruant]

An attempt to solve the same problem with pure future to start a discussion:

var clickTargetF = new DOMFuture(function({resolve}){
  myElement.onclick = function(e) {
    resolve(e.target);
  };
})

var idbResultF = new DOMFuture(function({resolve}){
  loadDataFromIDB(..., function(result) {
    resolve(result);
  });
});

asyncActionFuture = doAsyncAction(...);

DOMFuture.when(clickTargetF, idbResultF, asyncActionFuture).then(doStuffWith)

Although all APIs are suitable to be expressed with Futures, it's always possible to wrap the native call with a future if it makes sense for your use case (and that's the case here since you want only the first click target here). I feel the burden of future-wrapping is pretty much equivalent to what you've done with the checkIfDone definition and repeated calls to it and the amount of runtime work is equivalent, so I don't see a good reason not to do it.

Always using the .then() function means that you have to take a trip back to the event loop, even if the Future has already been resolved. Going back to the event loop is significantly slower than synchronously grabbing a value if the value accessor is simple (as is the case here).

I appreciate this argument, but from my experience with promises, if you get to a point where the code that needs the value runs after the resolution of a promise, it's a sign that this code needs a refactoring. If people have had a different experience with promises, that'd be interesting to share it.

I however understand the value of being able to fix something quickly now and refactor later. That's why although I think it's possible to do without state attributes, I'm not fully against the idea of keeping them. Allowing short-term fixed in some circumstances would be the only valid reason I see to keep the state attributes.

[mounirlamouri]

Always using the .then() function means that you have to take a trip back to the event loop, even if the Future has already been resolved. Going back to the event loop is significantly slower than synchronously grabbing a value if the value accessor is simple (as is the case here).

I feel that a normal usage of Futures wouldn't imply to call .then() in weird places. Keeping attributes just for that seems a bit overkill. Shouldn't we wait to know if there is a real need for them? @DavidBruant, is this a common pattern in node.js?

Another advantage of the attribute is that if you have code which is waiting for a number of asynchronous conditions to become true, then you can use the promise object itself to track if that particular asynchronous conditions has been fulfilled. I.e. you can do something like:

I was going to say what David said: if you really need to do that, you can always wrap the async call inside a Future.

Regarding API design in general, those attributes seem to be inherited from Web-y stuff like IDBRequest or DOMRequest but Future doesn't want to keep retro-compatibility with those interfaces. But by using those attributes, Future doesn't follow the common pattern used by Promises in other Javascript librairies/frameworks.

[DavidBruant]

@mounirlamouri Node.js has made the decision to go with callback-based APIs (every async function last argument is a function which takes (error, value) as argument). Some (rare) people in the Node community seem to even despise promises. I've never really understood why. As I said, anytime I've had to use the .result, a legitimate refactoring (usually involving future-wrapping) could remove the need.

Maybe @erights @domenic @kriskowal or others have relevant experience to share with promises and the need for state/result/error attributes.

[kriskowal]

@davidbruant It is sometimes necessary for performance optimizations to have some means of synchronously checking a promise’s state. In Q, this is made possible with isPromise, isFulfilled, isPending, isRejected, valueOf() (for the fulfillment value of a fulfilled promise) and valueOf().exception (for the exception of a rejected promise). We will probably change valueOf() to some other name. Exposing these as accessor properties is consistent with the spirit of the DOM, but in Q we could not depend on the availability of property descriptors and I did not feel any need to make the interface convenient, particularly to deliberately discourage premature optimization.

[domenic]

I am +1 on removing state just because it's the only place in the public API that the term "accepted" (instead of "fulfilled") surfaces; otherwise the DOMFuture promise library uses the same terminology as all others.

That said, synchronous inspection can be useful for a small performance gain in cases like templating or assimilation. See https://github.com/promises-aplus/synchronous-inspection-spec/issues/3. @lukehoban in particular has expressed that it's useful for templating, IIRC.

As an aside, Promises/A+ is maybe settling around https://github.com/promises-aplus/synchronous-inspection-spec/issues/6, although not many implementors have chimed in with whether or not they like it, so /shrug.

[kriskowal]

Oh, yes. Having observable state properties is very convenient for binding a promise to a view, for example a substitution that switches between a determinate progress indicator, an indeterminate progress indicator, an error indicator, or the appropriate view for the fulfillment value, based on property change listeners on "state", "progress", "error", and "value".

In Montage, we have a PromiseController that accepts a Q promise property for that purpose.

[sicking]

I guess I buy the argument that if you have multiple async callbacks you can always wrap futures around them. Though it's not clear that it's always less code. But it does create a nice pattern.

However I don't buy the argument that if you find yourself needing to check state arguments that "legitimate refactoring" would get rid of that need. First of all, I would say that that's a fairly bold statement and so requires some backing up.

Second, telling people to rewrite their code in certain ways generally tends to backfire in my experience. People just end up having to do crazy hacks because the coding patterns that you envisioned doesn't actually fit with their architecture. People's code often end up quite crazy and suboptimal and if our APIs doesn't work within that environment, we're likely just going to make their code even crazier and more suboptimal, rather than cause them to refactor it.

Another reason is that it's likely not always going to be obvious to developers how to refactor the code in order to make it fit with state-less promises. This is something that I see with IndexedDB a lot. People often write much more complex code than needed simply because they don't see the simpler way of doing things.

[DavidBruant]

I guess I buy the argument that if you have multiple async callbacks you can always wrap futures around them. Though it's not clear that it's always less code. But it does create a nice pattern.

Don't make me say what I didn't. I talked about "equivalent burden", not "less code". In your case, I was able to remove lines. Maybe in another, I'll be adding one or two lines. My point was that, doing both is possible and the effort (cognitive or number of lines) to do one or the other is equivalent.

First of all, I would say that that's a fairly bold statement and so requires some backing up.

I've been very careful to prefix the statement with "from my experience with promises". Full disclosure: my experience was a 8 months experience working on a crawler and a backend (which included talking to a database, other servers and some command line tools). This work is closed source. I really wish it was different because sometimes I know exactly the lines I'd like to point people to. It isn't and will stay so for quite a long time, likely ever (business-sensitive code). Also, I've been calling out for more experience from other people (who've actually proved me wrong with their experience i.e. the Montage case). I admit anytime that a 8 months experience isn't 20 years, but that's the only relevant thing I have, so I use it but with the necessary precautions.

Second, telling people to rewrite their code in certain ways generally tends to backfire in my experience.

If there is no state attribute in the API initially, people won't have to rewrite their code in a certain way, but only write it in a certain way. The way is actually pretty much imposed. In general I would agree with you. For the specific future case, I can imagine so few ways to architect code around futures that I don't see how it could be a problem, but I'm willing to be told my imagination is limited.

Another reason is that it's likely not always going to be obvious to developers how to refactor the code in order to make it fit with state-less promises. This is something that I see with IndexedDB a lot. People often write much more complex code than needed simply because they don't see the simpler way of doing things.

MOAR DOCUMENTATION !!!§§§!!!!§ :-) Which is another way to say that I agree with you on that point and there is unfortunately very few that can be done beyond educating developers. I think analogies with IndexedDB have to be made carefully. Futures don't have the API surface and complexity of IndexedDB. Options are fairly limited in how you can organize yourself around futures. I understand this simplicity as one of the reason a "community" around promises has emerged; people coming from different background all saw a pattern and agreed on a simple minimalistic abstraction.

[slightlyoff]

After talking with @lukehoban at the most recent TC39 meeting, I'm leaning in favor of removing this. We can add them later, but we can't remove them later.

One of the bigger perf issues in the MSFT experience with promises seems to be that they always go to the next turn for entire chains, even when values (not futures) are returned from .then() calls. This leads to a situation when many callers might want .value/.error and .state. Yes, we'll penalize by always going to end-of-turn in this design where you might avoid it otherwise, but as I said, if it's a serious pain-point, we can add these back later.

Thoughts?

[domenic]

I like the conservatism of that last comment, and agree :).

Also, IIRC part of @lukehoban's problem was that he envisioned long chains having a number of next-turns proportional to the length of the chain. But, this is not actually a Promises/A+ requirement, and can be circumvented in practice without violating any invariants by batching all fulfillment handler calls together in one (future) turn. In other words, the number of turns for any chain can always be one.

[annevk]

I agree too. Starting out super simple, without DOM events, state, or anything, and then maybe adding features later if justified by real world usage seems like the best way forward.

[erights]

+1 on removing the attributes.

On turns, the key invariant is that each element of the chain starts when the stack is empty. This effectively makes each a turn, but the mechanism by which they are scheduled doesn't need to go via the underlying platform's event queue. Whether this addresses the performance issue you're concerned about, I don't know.

I think there should also be guarantees about the relative order in which these future turnoids are scheduled. I don't know if this effects your performance concerns either.

Perhaps you could explain more what it is that seems unoptimizable about the full turn model?

[domenic]

@erights I think the scenario is as follows:

var deferred = Q.defer();

var p = deferred.promise.then(() => 1).then(() => 2).then(() => 3).then(() => 4);

p.then(() => {
  // this code should ideally happen only one tick after the `p.then` call.
});

deferred.resolve();

[erights]

Specs only deal in observables. An implementation is free to do any optimization it wishes which is not observably different from the spec. Tick-count is not time. A spec which mandates that the above takes 5 tick counts is not worse than a spec that allows it to take one tick count if the two can easily be implemented to do all these ticks in the same amount of time. Tick counts aren't relevant. Clock cycles are.

In other words, I don't get it.

[domenic]

Sure. But in current browsers and runtimes, ticks are observable (via process.nextTick or setImmediate). And experimentation shows that promise implementations which take five ticks for the above versus those that take one tick result in proportionate times.

Thus, if a spec mandated five ticks, it would be mandating lower performance in current runtimes.

[slightlyoff]

I think the way to consider the timing invariant that we'll need to write down in prose is that you must not observably resolve the head of a then() chain before the end-of-microtask. You may fast-forward through the chain from there, particularly when values and not Future instances are returned from handlers, but you must at least go to the event loop once. This would actually be bad in the situations that @lukehoban described where WinJS programmers had many, many conditionals and async operations queued (depth == 17 in some UI). If these are each penalized with 5ms or even 1ms penalties for going around the event loop, you can easily create janky UI and drop frames. This is a Bad Thing (TM). WinJS apparently doesn't do any fast-forwarding, but it's not clear it would help.

I am honestly conflicted, as result, about leaving these properties out. I can imagine wanting them myself for just such a situation but I understand why they may lead others astray.

[sicking]

I really like the idea of defining in prose that you must not observably resolve the head of a chain before end-of-microtask (with the exception of other end-of-microtask items of course), and that you may fast-forward through the chain from there.

I might even say that UA-implemented Futures must fast-forward through the chain after that in order to ensure that different UAs behave consistently.

[slightlyoff]

Looks like @annevk is making great progress on this: http://dom.spec.whatwg.org/#futures