-
Notifications
You must be signed in to change notification settings - Fork 634
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
Redefine Stream/Sink/AsyncRead/AsyncWrite/etc on top of Future #1365
Comments
Having the core traits return futures directly are probably blocked on GATs (or all the way to trait Stream {
type Output;
type Next<'a>: Future<Item = Option<Self::Output>> + 'a;
fn next(self: Pin<&mut self>) -> Self::Next<'_>;
} I reintroduced |
I was afraid of that. I don't know yet what GATs exactly are but will try read up on them. Can you elaborate how a signature for a stream would need to look like in order to make it implementable by future composition as outlined above? I would say as long as that is not possible, we would maybe face the general issue that it's not possible to define abstractions around types that are implemented by async functions. And e.g if I built an async powered HTTP client, I can't built an interface/trait around it that I use for unit-testing an dependency-injection in other code.
You are right, most streams will only support producing a single item at a time. Supporting more would require internal queuing (which can easily be implemented through I think that aspect could be most cleanly solved by the type system, if reading from the stream produces a
I can see that some |
GATs are generic associated types, that's the feature that allows parameterizing the
This is where having GATs is useful, rather than consuming the stream and receiving it back once complete you can borrow the stream, and have that borrow last until the future is complete/dropped.
My use case was reading a stream of values from a radio peripheral in a microcontroller. By using pinning the stream can have an internal buffer that is written to by the radio using DMA, without pinning this buffer would need to be externally allocated and passed in as a reference (or heap allocated, but this was for a |
Ah, that makes sense. So the following wouldn't work, because without GAT we can't put the lifetime on the item? trait Stream {
type Output;
type Next: Future<Item = Self::Output>;
fn next(self: &'a mut) -> Self::Next<'a>; // Or is that Self::Next + 'a ?
} After reading GATs I understand it makes lifetimes more flexible, but I wasn't sure whether something is possible without them. It should be OK for this use-case if the lifetime of the returned Future is bound to the stream.
Ah, cool idea to represent it as a stream. I think however your requirement is more the typical pain point of an embedded system: One shouldn't dynamically allocate, and therefore ideally all buffers/etc. should be contained inside the type. However without e.g. const generics that's already painful, and even with I don't find the embedding approach super appealing (there's an endless forwarding of type parameters). |
I tried things out. I seems to be only possible to define this at the moment: trait Stream<'a> {
type Output;
type Next: Future<Item = Self::Output> + 'a;
fn next(&'a mut self) -> Self::Next;
} which is arguably not nice due to the lifetime which needlessly is on the trait. |
I discussed this with @erickt elsewhere and explained my thoughts. As the ecosystem stands, it's not possible to make this work without making all of these traits non-object-safe, so I'm not interested in pursuing this at the moment. |
One thing that come to mind with regards to |
@cramertj: would you mind sharing these publicly at some point? I guess that would be interesting for others too. The implications on the ecosystem are totally understood. Although the current changes from 0.1 already are lots of those smaller renaming things. And this would mostly be another. We can still have the old things around, with adapters to the new variants. This direction is possible to bridge. However it’s not possible to move future based thingies into the current signatures without costs (eg extra allocations) @seanmonstar: I think this should still be possible as all types that support read and write should also support a split method for getting individual halves in order to make them more flexible. Then you can get a future from the reader and one from the writer. |
@Matthias247 Oh sorry, I don't have a large writeup or anything, I just mean't we had discussed it. If there were a way to return a |
Ok. Let's focus on this issue, instead of spreading the discussion out. I think e.g.
which is mostly a copy of the well-known synchronous variant, and behaves just like it when combined with The obvious problem is that this won't work yet without GATs. For Streams/Sinks we can work around this by requiring them to be implemented through I am aware that the inability to create trait objects is an issue. But not being able to compose Futures and other async traits seems like an even bigger one to me currently. Just as a motivating example: I have written a completely async HTTP/2 library a while ago, which I still think has a reasonably simple API and implementation. The API provides users of the library some byte stream based interfaces, which are similar to The implementation of this interface is heavily based on top of Future/Task composition, as it can be seen here: https://github.com/Matthias247/http2dotnet/blob/master/Http2/StreamImpl.cs#L520-L591 Writing data includes blocking on async mutexes for synchronization of long-lived tasks, sending data asynchronously to a background task which performs all IO multiplexing and more async waiting until the data has been actually flushed. This can't really be done with the the current Rust traits. It seems like it would be back to manually implementing state-machines for all those operations. Therefore I would really like to see a mechanism where we can implement those common async base traits on top of Futures - ideally in a zero cost fashion. Since it seems like this isn't yet possible I'm totally ok with having some intermediate solutions in between. But I wouldn't like the whole ecosystem to commit already on those. Or even to build a completely separate and incompatible world on top of those. |
@Matthias247 The "Ext" traits allow you to easily treat
Given that the style of writing code that you want want is already possible thanks to the "Ext" traits, it seems a shame to remove the ability to have zero cost async read + write. |
The Ext traits allow to use those other traits as Writing code in the style I shared is not possible using the current state of the traits. |
I feel like we could prepare a more modern api already. Async trait methods can be done by returning When GAT's or async trait methods come around, we can change the method signature, remove the feature gate and deprecate the poll_* methods. In the mean time if people need high performance, they can continue using the current api. |
@najamelan It's critically important to continue allowing these traits to be object-safe and non-allocating. |
@cramertj As far as I can tell, returning Pin box dyn future from a trait method is object safe. For the allocating of a boxed future, well that will be a temporary problem until async trait methods become possible. Do you feel that having such an api behind a feature gate whilst keeping the current api for people who need the performance is unacceptable? I mean having an unstable api behind a feature gate that does allocation temporarily is unacceptable? The problem I'm running into at least, but maybe there's a better solution, is that implementing Im not sure either that it's more performant to spawn a new task than to box a future... |
It's not a temporary problem-- there's no solution that has been proposed so far which would allow non-allocating object-safe async methods. |
I think the object-safety is a good argument. The allocations are not necessarily. We can start with defining things like pub trait Stream {
type Output;
type Next: Future<Output = Self::Output>;
fn next(&self) -> Self::Next;
}
pub trait Sink {
type Input;
type Error;
type Next: Future<Output = Result<(), Self::Error>>;
fn send(&self, value: Self::Input) -> Self::Next;
} That is zero-allocation for all implementations where the source/sink internally uses an Object safety is obviously a harder discussion, and also a bit of a question of prioritization. Is it more important for users that those types are object safe than being able to compose them from Also since the current Futures ecosystem isn't really that dynamic-dispatch friendly (due to enormous generated types), it's not obvious to me why we prioritize it here over the ability to write easy-to-understand async code. |
Regarding object-safety we might think whether that could be regained for the most common use-cases by some custom allocator or boxing that type-erases the returned Then we would could have an additional pub trait DynamicDispatchableStream {
type Output;
fn next(&self) ->FixedSizeFuture<Output = Self::Output>;
} |
I don't believe these are changes we're going to make for reasons I've explained above (the structure of the state machines, object safety). Thanks for the discussion, though, and I'm sure there'll be more to discuss here in the future. For those interested in more, http://smallcultfollowing.com/babysteps/blog/2019/10/26/async-fn-in-traits-are-hard/ has a great explanation of the complications here |
I think in the async/await world it might be preferable to redefine the traits and types like the ones mentioned in the title on top of
Future
s, and e.g. move frompoll_something()
signatures to methods that returnFuture
s.E.g.
The important reason for this, that it should be possible to implement those types on top of Futures as primitives, which isn't possible today.
As a motivating example I'm thinking about a MPMC channel, which can be built around some async primitives as shown below:
I'm not yet sure if the current state of async functions and traits allow all this (please educate me!), but I think this should be one of the goals that async programming in Rust should enable.
This kind of implementation is however currently not wrappable in a
Stream
orSink
implementation.We can't stuff this kind of code inside a
poll_xyz
method of aStream
/etc, since it requires temporaryFuture
s that all carry their own poll state to be instantiated in the task. ThoseFuture
s must maintain their state between calls topoll()
. E.g. polling an asynchronous mutex the first time creates a registration in the waiting list, and dropping it cancels the registration. If we would try to use an asynchronous mutex inside apoll_next
method, we must always need to drop it beforepoll_next()
returns, which would cancel the registration and never allow use to retrieve the mutex.I think the more general observation might be, that
Future
s allow for persisting some additional temporary state per operation, and provide additional lifecycle hooks per operation (Future
gets polled for the first time, andFuture
gets dropped) compared to the currentStream
definitions.Another side effect is that currently
Streams
/etc must all support pinning. In Streams that returnFuture
s that wouldn't be the case, since theFuture
s get pinned. They might indirectly get pinned because the respectiveFuture
s might reference theStream
through a reference and a certain lifetime, but that relationship is already directly checked through Rusts normal means, and e.g. doesn't put special requirements regarding pinning on the implementation of the type. And often we wouldStream
s being moveable after they have been partly consumed (e.g. a Tcp Socket), which is not allowed if they are pinned. Of course some of them might beUnpin
, but this kind of change would allow moves again for allStream
s.The text was updated successfully, but these errors were encountered: