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

Allow asynchronous code in a synchronous context #31102

Closed
nex3 opened this issue Oct 14, 2017 · 29 comments
Closed

Allow asynchronous code in a synchronous context #31102

nex3 opened this issue Oct 14, 2017 · 29 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. customer-dart-sass type-enhancement A request for a change that isn't a bug

Comments

@nex3
Copy link
Member

nex3 commented Oct 14, 2017

Asynchronous code in Dart is currently contagious, in the sense that as soon as one portion of an API is asynchronous, every caller must be made asynchronous. We work around this in various ways, including gradually adding more and more synchronous versions of built-in asynchronous APIs, but splitting every API into two versions quickly becomes untenable as code gets more complex and has more layers.

We're running into this issue with Dart Sass today. All of Sass's compilation logic is purely synchronous, and we need to provide the ability for people to invoke it synchronously. However, it also provides extension points to allow users to write custom functions and importers, and in some cases these need to do work that's only possible asynchronously. For example, to integrate it with the build package, it needs to be able to read inputs using the asynchronous AssetReader.readAsString() method.

In the server-side JavaScript world, the fibers package provides coroutine functionality. The Dart VM could provide something similar that would be one way to make this work without violating the principle of having only one active thread of execution per isolate.

A simpler but less general possibility would be to provide a blockUntilMessage() function that blocks the current isolate until a port has an incoming message, handles that message by invoking its Dart handler, and returns. This could be used as a primitive building block for more user-friendly synchronization functions:

/// Blocks until [future] completes.
T synchronize<T>(Future<T> future) {
  T value;
  var completed = false;
  future.then((value_) }
    value = value_;
    completed = true;
  });

  while (!completed) {
    blockUntilMessage();
  }

  return value;
}

This is effectively a form of coroutines in which the execution can only be suspended and resumed in a very restricted way.

@nex3 nex3 added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. customer-dart-sass type-enhancement A request for a change that isn't a bug labels Oct 14, 2017
@mkustermann mkustermann added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. and removed area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. labels Oct 14, 2017
@mkustermann
Copy link
Member

@floitschG Isn't this first and foremost a language/library discussion. Without knowing whether and if so, what, mechanism to expose there is no VM work to do.

@lrhn
Copy link
Member

lrhn commented Oct 14, 2017

I feel the pain. Futures and explicit asynchrony is a solution to a problem that works on all the platforms that we target. It's not necessarily the best solution, but it works even when the only primitive you have is a single thread and a top-level event loop.

This request can be approached from many directions.

  • As a language issue, where we provide language support for blocking operations, like await inside a synchronous method. It's not impossible as a language design, but unlikely to be efficiently compilable to a language like JavaScript that doesn't have similar features.
  • As a library issue, where we provide platform independent guarantees that some library function can be blocking for the entire isolate in a user-controlled way. That is a pretty big gun, toes will be lost, and still hard to compile to JavaScript.
  • As a platform specific issue, where a single platform provides a custom library that does ... something (anything and everything that platform can possibly support can be exposed).

As a language designer, I don't see language changes in this direction happening any time soon. Compiling to JavaScript is hard enough already.

As a core platform library designer, I don't think any isolate-blocking operations are likely either. Again, compiling to JavaScript, which doesn't have any good way to block the current execution better than alert() , means that it won't work in practice.

This leaves the platform specific solution. If the stand-alone VM wants to provide such functionality, then I'm not opposed to it, and wouldn't mind helping with the design, but it would mean that any code using the feature will be locked to that one platform. That does diminish the value of the feature significantly.

@matanlurey
Copy link
Contributor

matanlurey commented Oct 15, 2017

FWIW, I primarily work on Dart web products, and I don't care if this doesn't work in JavaScript.

The idea of trying to only maintain and create language features and support them iff the browser can support them has already has thrown out the window. We don't support dart:io at all, we will soon not support dart:isolate or dart:mirrors at all, and if, for example, Flutter needs a better concurrency model I'd hope that "it doesn't work in JavaScript" isn't the blocking factor.

In fact, we've had multiple "experimental" language features (assert in initializers, super mixins) that haven't worked in any web product for probably over a year already, and I've yet to see any real issue.

The up-and-coming most popular cross-platform languages most similar to Dart (C#, Kotlin, Swift) don't care if every language feature works on every platform in every possible configuration. I'd hope that isn't a blocker for Dart either.

@lrhn
Copy link
Member

lrhn commented Oct 16, 2017

That is the point I'm trying to make: We have platform specific (or multi-but-not-all-platform) features already (in dart:io, dart:isolate, dart:html and dart:mirrors). We can do things there that not all platforms support. We should not add such a feature in a generally supported library (so not in dart:core or dart:async). That is, the third bullet-point case is the one that can make sense.

Then it's a matter of discussing the viability of the features with the platforms that actually support that library, and deciding whether it's useful enough that we want it, and not something we'll regret later because it gets used so much that a lot of useful libraries become platform specific.

A blocking operation seems like it would fit in dart:isolate or dart:io. Since it'll block on a receive-port, dart:isolate is probably the best location.
It has to have a clear blocking point, and a way to set up the background computation. I'm thinking something like:

abstract class BlockingReceivePort {
  /// Port for sending events to this receive port.
  SendPort get sendPort;
  /// Close the [BlockingReceivePort]. 
  /// No further events are received and further calls to [next] will throw.
  void close();
  /// Receive the next received object that has been sent on [sendPort].
  /// Blocks the entire isolate until an object is received.
  /// The blocking receive port will buffer events received before [next] is called and
  /// deliver them immediately instead of blocking.
  Object next();
}

You can call next() at any point, and the entire isolate will block until an object is sent on sendPort.

Blocking an isolate is a powerful feature. It's unclear whether the isolate can receive other port events, even control port events, while blocked (it'll probably be the same as if it's blocked on a an OS call).
At least we never promised anything about the relative order of port events on different ports, so it doesn't matter that this event is effectively delivered before other events that were received earlier.

In any case, it's not a feature that is planned for Dart 2.

@nex3
Copy link
Member Author

nex3 commented Oct 16, 2017

To be clear, I'm interested in this as a VM-only feature. I'm pretty sure that it's not just difficult to compile to JS, it's actually impossible (at least as long as we support interop). We could talk about some way to write code that's polymorphic over asynchrony, but that's out of scope for what I'm requesting here.

Unless I'm missing something, I don't think the BlockingReceivePort straw-man would work for what I'm trying to do. If it blocks the entire isolate, you can't use it to wait for a Future that's generated within the current isolate. I specifically need the ability to call a user-provided asynchronous callback in synchronous code; I think I need some sort of coroutines for that.

@lrhn
Copy link
Member

lrhn commented Oct 16, 2017

If you need to run something in the same isolate while blocking other synchronous code, then yes, you need multiple stacks (as co-routines or fibers or something else).

I'm guessing that is significantly harder to implement than plain isolate blocking - but I could be wrong. If the VM is using threads internally anyway, it might be able to do something with those, even if only one of them is running at a time. It's definitely a VM-only feature then.

@matanlurey
Copy link
Contributor

If we ever go with Web Assembly the whole "VM-only" world disappears. Just saying 😄

@nex3
Copy link
Member Author

nex3 commented Oct 16, 2017

I don't think threads are necessary; for example, Ruby doesn't support OS threads, but it does support fibers. All that's really necessary is an internal representation of the stack. My blockUntilMessage() proposal doesn't even really require that, since you can model the message-handling as being in same stack as the blockUntilMessage() call.

@lrhn
Copy link
Member

lrhn commented Oct 17, 2017

The blockUntilMessage() was originally described as

blocks the current isolate until a port has an incoming message

which the BlockingReceivePort does satisfy. If you're saying that's not sufficient, because it shouldn't block the entire isolate, then I'm not sure which blockUntilMessage() proposal you are referring to above.

There are things you can do by rerunning the event loop code on top of the "blocked" stack, see, e.g., the Java Foxtrot library. It has obvious limits, at least you have to maintain a stack discipline for the blocked operations. There's also the option of stack rewriting, copying the stack away when blocking, reinstating it on release, which has to play well with both garbage collection and on-stack-replacement optimizations. But I'm guessing, I'm sure the VM engineers have a much better idea of what is needed and possible.

@nex3
Copy link
Member Author

nex3 commented Oct 17, 2017

The blockUntilMessage() was originally described as

blocks the current isolate until a port has an incoming message

which the BlockingReceivePort does satisfy. If you're saying that's not sufficient, because it shouldn't block the entire isolate, then I'm not sure which blockUntilMessage() proposal you are referring to above.

The distinction is that blockUntilMessage() blocks the isolate until a message comes in for any port, whereas BlockingReceivePort blocks until a message comes in for that port in particular. Blocking on any port is what allows us to run it in a loop until something happens.

I chose blockUntilMessage() because it's the simplest to describe in terms of implementation, but another primitive that would provide the functionality I need would be:

/// Runs the event loop until [test] returns `false`.
void yieldWhile(bool test());

This could be implemented in terms of blockUntilMessage(), or vice versa:

void yieldWhile(bool test()) {
  do {
    blockUntilMessage();
  } while (test());
}

void blockUntilMessage() => yieldWhile(() => false);

There are things you can do by rerunning the event loop code on top of the "blocked" stack, see, e.g., the Java Foxtrot library. It has obvious limits, at least you have to maintain a stack discipline for the blocked operations.

Yeah, this is the benefit of blockUntilMessage()/yieldWhile(): as long as the event loop doesn't assume it's at the root of the stack, it doesn't require any fancy machinery.

There's also the option of stack rewriting, copying the stack away when blocking, reinstating it on release, which has to play well with both garbage collection and on-stack-replacement optimizations. But I'm guessing, I'm sure the VM engineers have a much better idea of what is needed and possible.

Yep, this is the sort of thing that's necessary to implement full fibers. Fibers are very powerful, and I think it may well ultimately be worth the effort, but I'd be happy with a simpler solution for now.

@lrhn
Copy link
Member

lrhn commented Oct 17, 2017

Blocking on any port is what allows us to run it in a loop until something happens.

Most likely that won't work. The problem is that the isolate receiving a port message is not the same as that message being delivered to the receive port handler.
If you block the isolate until any port receives a message, when you continue running, that message still hasn't been handled by any Dart code - it can't, the isolate was blocked so no Dart code could run. The message is buffered and the event loop is ready to deliver it, but no Dart state has changed.

That is, you don't get any visible effect from blocking because the state afterwards is exactly the same as the state before blocking. You need to run Dart code to change that ... unless you have some platform supported construct that changes state due to receiving the event. That's what the BlockingReceivePort could do by allowing the message to bypass the event queue and go directly into the buffer (just) before unblocking. It's a non-event-loop based message delivery system.

The yieldWhile has the same problem, you either need something to run in order to change state, or you need something that is aware of the received message, otherwise you're no better of after blocking than before.

@nex3
Copy link
Member Author

nex3 commented Oct 17, 2017

The idea is that blockUntilMessage() also handles the message by invoking the handler beneath the current stack.

@lrhn
Copy link
Member

lrhn commented Oct 18, 2017

Indeed, that would be necessary, bringing us back to the strategy of running the event loop on top of the stack. It might only be a single event being delivered, but I'll wager that you'd also want any microtasks triggered by that event to run as well. Even with that, it's strictly simpler than running the full event loop while waiting for the block to complete.
It does need to handle the case where the event handler blocks again, so we can have multiple blocked stacks on the stack at the same time (but at least at most one of them actively blocking, the rest are waiting to be released). The microtask queue needs to handle being blocked in the middle, running a different event and its microtasks, before continuing again (if only it had been a stack instead of a queue, that would have been easier!).

All in all, probably doable. It really amounts to blocking until the next event loop event, then executing that on top of the current stack, then continuing execution of that stack. That's why it requires platform support (there is no way to access the JS event queue except returning to it).

Event loop events are not just port messages, they can also be timers (non-zero timers are port messages in the VM too, zero-duration timers are not) and possibly also other things like I/O. You might want to unblock for any event (blocking for an asynchronous HTTP fetch to complete is likely to require many events, which may or may not be implemented as port messages).

With that in mind, it's probably simpler to just run the entire event loop on top of the stack, until some condition is true. Like a future having completed (which it can then do).

T waitFor<T>(Future<T> future) {
    Result<T> result = null;
    Result.capture(future).then((r) { result = r; });
    // Checks the condition between top-level events.
    // Saves and restores current microtask queue.
    _internalRunEventLoop(until: () => result != null);
    if (result.isValue) return result.asValue.value;
    // rethrow the async error synchronously.
    _internalRethrow(result.asError.error, result.asError.stackTrace);
} 

Waiting for port messages seems less general.

(I'm not saying that we want this, just that it's probably doable, with some, likely significant, amount of refactoring of the event loop code to make it reentrant).

@nex3
Copy link
Member Author

nex3 commented Oct 18, 2017

I don't think we'd need to handle microtasks any differently than any other event; I'd expect _internalRunEventLoop() to run any microtasks that are already queued up, followed by any new microtasks that are enqueued by those tasks, followed by any non-microtask events and any microtasks they enqueue.

@zanderso
Copy link
Member

zanderso commented Dec 5, 2017

Sorry for not following along with all the discussion in this issue. Please see the below CL for a primitive from the VM that I think would help with this use-case. It's not perfect, and is still being debated by the VM team, but PTAL: https://dart-review.googlesource.com/#/c/sdk/+/25281/

@nex3
Copy link
Member Author

nex3 commented Dec 5, 2017

@zanderso That looks great! It looks like that would solve this use case.

In the CL, you say that the analyzer should complain about this in a lot of configurations. Under what configurations is dart:io available where this wouldn't work?

@zanderso
Copy link
Member

zanderso commented Dec 5, 2017

Any embedder that is providing its own message loop will probably become wedged on a call to waitForEventSync. Flutter on all platforms, and the Dart content handler on Fuchsia provide their own message loops.

@nex3
Copy link
Member Author

nex3 commented Dec 5, 2017

Is there a path for platforms like that to become compatible with waitForEventSync()?

@zanderso
Copy link
Member

zanderso commented Dec 5, 2017

Even if there was, suspending the UI thread to wait for anything would cause jank.

@nex3
Copy link
Member Author

nex3 commented Dec 6, 2017

Even if there was, suspending the UI thread to wait for anything would cause jank.

I'm not sure I follow—doesn't the proposed function only suspend execution until there's an event available to handle, meaning that UI actions would still be dispatched as quickly as they would otherwise?


After thinking about the proposed semantics, I'm curious how you'd use it to implement the waitFor() function. A naïve implementation would look like @lrhn's above:

T waitFor<T>(Future<T> future) {
  Result<T> result;
  Result.capture(future).then((r) => result = r);

  while (result == null) {
    waitForEventSync();
  }

  if (result.isValue) return result.asValue.value;
  _internalRethrow(result.asError.error, result.asError.stackTrace);
}

But what happens if future completes as microtasks are being flushed in the first call to waitEventSync()? It'll still wait for an incoming event, but if none is scheduled it might just deadlock.

@zanderso
Copy link
Member

zanderso commented Dec 6, 2017

doesn't the proposed function only suspend execution until there's an event available to handle, meaning that UI actions would still be dispatched as quickly as they would otherwise?

I do think that there are various ways that the Flutter event loop could be rearchitected to achieve this, however there are no simple tweaks to the way that Flutter works today that can do this. The simple tweaks would all result in the delayed handling of requests to run the build method. This is the sort of thing that should probably be looked at only if support for new language features needs to be developed.


Right, you'd need something like:

T waitFor<T>(Future<T> future) {
  Result<T> result;
  Result.capture(future).then((r) => result = r);

  Timer.run(() {});   // Ensure that there is at least one message.
  while (result == null) {
    waitForEventSync();
  }

  if (result.isValue) return result.asValue.value;
  _internalRethrow(result.asError.error, result.asError.stackTrace);
}

@nex3
Copy link
Member Author

nex3 commented Dec 6, 2017

Right, you'd need something like:

This seems like a big pitfall, especially considering that the failure case is a deadlock that may or may not occur depending on the context of the call. If these semantics are necessary, maybe it would be better to keep waitForEventSync() SDK-private and expose waitFor() as the user-facing API?

@zanderso
Copy link
Member

zanderso commented Dec 7, 2017

A few things:

  • I’m very reluctant to make this easy to use since it is preferable for people to write idiomatic Dart code.
  • I would prefer to have the doc comment indicate that the call should only be used by folks who know what they’re doing.
  • I don’t want to put extra logic that has to be maintained by someone into dart:io.

@nex3
Copy link
Member Author

nex3 commented Dec 7, 2017

Can we set up a VC to talk about this?

@zanderso
Copy link
Member

zanderso commented Dec 7, 2017

Side question. What is Result? I can't find it in the SDK.

@natebosch
Copy link
Member

Side question. What is Result? I can't find it in the SDK.

https://www.dartdocs.org/documentation/async/1.13.3/async/Result-class.html

@whesse whesse closed this as completed in 3ea5e13 Dec 12, 2017
@whesse
Copy link
Contributor

whesse commented Dec 13, 2017

The issue is closed by landing the waitForEventSync implementation on the VM:

Reland: [dart:io] Adds waitForEventSync

The only fix needed for relanding is adding _ensureScheduleImmediate
to the list of vm entrypoints in //runtime/vm/compiler/aot/precompiler.cc

Original commit message:

Adds a top-level call waitForEventSync to dart:io that blocks the
thread an Isolate is running on until messages are available.
Before the thread blocks, the microtask queue is drained.
Before waitForEventSync returns, all messages are handled.

@natebosch
Copy link
Member

"waitForEventSync" doesn't show up when I search the CHANGELOG. Should we consider this complete if we haven't documented it in the changelog?

@zanderso
Copy link
Member

CHANGELOG update landed here: https://dart-review.googlesource.com/#/c/sdk/+/29041/

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. customer-dart-sass type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

8 participants