Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new Future() is "slower" than new Future.value() #11911

Closed
nex3 opened this issue Jul 19, 2013 · 6 comments
Closed

new Future() is "slower" than new Future.value() #11911

nex3 opened this issue Jul 19, 2013 · 6 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-as-intended Closed as the reported issue is expected behavior library-async

Comments

@nex3
Copy link
Member

nex3 commented Jul 19, 2013

Consider the following code:

    void main() {
      new Future.value().then((_) {
        print("lion");
        new Future(() => print("giraffe"));
      });

      new Future.value().then(() {
        print("elephant");
        new Future.value().then((
) => print("baboon"));
      });
    }

I'd expect this to print

    lion
    elephant
    giraffe
    baboon

since each asynchronous event is added to the event queue and then run in FIFO order. Instead, it prints

    lion
    elephant
    baboon
    giraffe

apparently because new Future.value() takes precedence in the event loop over new Future(). This is extremely confusing behavior, and contrary to how the event loop seems to work everywhere else.

Note that replacing "new Future()" with "new Future.value()" makes the code work as expected.

@floitschG
Copy link
Contributor

We recently changed the behavior.
The reason is that the main use-case for new Future(callback) is to give the DOM time to react. Otherwise there is a good chance you would have already executed the callback immediately. This means that new Future(callback) should not go into the runAsync queue as that one starves the DOM.
Future.value, on the other hand, behaves like a Future that was completed earlier (and it is completely valid to keep these Future.values around instead of creating a new one every time). As such it executes callbacks with runAsync (thus before new Future(X)).


Added AsDesigned label.

@nex3
Copy link
Member Author

nex3 commented Jul 19, 2013

The reason is that the main use-case for new Future(callback) is to give the DOM time to react.

This seems backwards to me. That's like saying the main use-case for [Iterable.where] is finding specific DOM elements. [new Future] is a core Dart library API; its main use-case is anyone who wants to wrap a block of code in a Future.

If the desire is to provide a way to use Futures with this unusual event-loop semantic, add an explicit constructor. Right now, it's both undocumented and unintuitive that this constructor -- which looks like it exists to ensure that errors are wrapped in a Future -- has different semantics than [new Future.value] and [new Future.error].

If this behavior doesn't change, we'll be forced to remove all references to [new Future] in pub and associated packages, because its interaction with other asynchronous events is so difficult to reason about. We'll likely replace it with a utility function that does the same thing, but forwards to [new Future.value] behind the scenes.

@floitschG
Copy link
Contributor

The constructor to wrap errors in a future is Future.sync.

@nex3
Copy link
Member Author

nex3 commented Jul 19, 2013

[new Future.sync], as the name implies, is synchronous, which is often not what you want.

The problem here is that [new Future] looks like the default way to construct a Future if you want to wrap errors or run the code inside it asynchronously. But if you use it for those things, you open yourself up to any number of subtle bugs caused by its idiosyncratic event loop behavior.

new Future(() => val) looks like it should be the same as new Future.value(val), the documentation implies that it's the same as new Future.value(val) and it should be the same as new Future.value(val).

@DartBot
Copy link

DartBot commented Mar 17, 2015

This comment was originally written by @stevenroose


It's been 8 months. Assuming the current behaviour won't be changed, can anyone at least confirm so and/or provide the rationale for the current behaviour and the conflicting documentation?

@floitschG
Copy link
Contributor

My comment (#­1) still stands.

I'm not aware of conflicting documentation. Could you please point us to it, so we can fix it.


cc @lrhn.

@nex3 nex3 added Type-Defect area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-async closed-as-intended Closed as the reported issue is expected behavior labels Mar 18, 2015
natebosch added a commit to dart-archive/barback that referenced this issue Sep 16, 2017
Based on the discussion in dart-lang/sdk#11911
this utility was introduced to allow using the microtask queue rather
than the event loop. That used to be what `async` methods did, but soon
they will be able to start synchronously. `new Future.microtask` should
keep the old behavior.
natebosch added a commit to dart-archive/barback that referenced this issue Oct 5, 2017
Based on the discussion in dart-lang/sdk#11911
this utility was introduced to allow using the microtask queue rather
than the event loop. That used to be what `async` methods did, but soon
they will be able to start synchronously. `new Future.microtask` should
keep the old behavior.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-as-intended Closed as the reported issue is expected behavior library-async
Projects
None yet
Development

No branches or pull requests

4 participants