Futures-rs: Having both "sync" and "sink" in the API makes verbal discussion v. confusing

[jsgf]

The other day I had a conversation in which we were a few mins in before we realized we were talking at cross purposes because of a sync/sink confusion.

Maybe rename futures::sync? Or "sink" -> "drain" (or something)?

[alexcrichton]

Heh, agreed :)

FWIW I currently disambiguate with "sink with an I" and "sync with a Y", although it's not great.

Agreed we should probably rename Sink (or at least consider doing so) for 0.2

[carllerche]

Pretty sure @alexcrichton suggested Push before... I like it as the analog of Poll.

[alexcrichton]

Oh right!

[aturon]

Closing -- this is addressed in 0.2, https://github.com/rust-lang-nursery/futures-rfcs/pull/1

[carllerche]

@aturon how is this addressed? Afaik, the trait is still named Sink.

[aturon]

The sync module is no more.

[carllerche]

@aturon FWIW, the struggle is real when talking about Sink vs. Sync as the Sync concept applies everywhere you talk about Rust.

I now always say "Sink with an I" and "Sync with a Y" when talking about futures.

[aturon]

@carllerche heh, indeed! futures-channel should make this much simpler.

[aturon]

Ah sorry, I missed your point re: the Sync trait. Hm. Reopening.

Do we have any contenders for a different name for Sink?

[carllerche]

@aturon Alex originally proposed Push, and I like that...

We obviously can't use Send, so there are limited other options besides Push :)

[jsgf]

(I really don't like Sink conceptually and we'd lose nothing except complexity by removing it entirely.)

[aturon]

@jsgf yeesh! I'm curious why you feel so strongly, and what you think would be a better replacement?

[jsgf]

@aturon The beauty of the futures model is that it's all pull based - it's the act of calling poll() that drives everything along. Adding Sink adds a push element to the design that causes an impedance mismatch whenever it turns up.

So, for example, unbounded and oneshot channels are fine, because their senders can never block. But the bounded channels are a complete pain to use because the Sink requirements of the sender can't be reconciled with the surrounding async environment.

Every time I've used Sink it's ended up being a big increase in complexity, and in each case I could turn it around to a simpler pull-based model.

It does mean that we need to move away from thinking about the "sending side" of a channel/pipe/socket/etc. Instead they all become the "consuming side" - they take ownership of a Future or Stream instance and poll it for the next thing to transport.

The main value of a sink-like endpoint is when interfacing blocking code into an async environment across threads via a channel. In that case a sync sink is fine, but that doesn't need a complex interface like Sink.

[carllerche]

@jsgf the Sink trait has been reworked and I think it addresses your concern. The async components are now poll based.

[jsgf]

OK, I'll take a look. When did those changes happen?

[carllerche]

@jsgf hasn't been merged yet. https://github.com/rust-lang-nursery/futures-rs/pull/765

[aturon]

@carllerche I don't think the recent changes to Sink dramatically change the things that @jsgf is talking about, though it's possible I'm misunderstanding.

@jsgf When we were first designing Sink, we did consider doing something like what you said instead: modeling everything as streams, where a "sink" is just a "stream consumer". However, strictly following that approach led to some painful ownership issues, IIRC.

That said, the traits do allow you to consume a Stream and a Sink -- is there a reason the forward API isn't enough to let you treat sinks in the way you describe?

[carllerche]

@aturon I think it does :)

@jsgf originally said:

So, for example, unbounded and oneshot channels are fine, because their senders can never block.

My interpretation of that is that Sink has historically been difficult to use because send can fail. This has also been my experience and why I personally have never ended up using it.

However, with the new trait, you can reconcile sinks with the surrounding async nature of things.

Imagine a ready combinator on Sink that yields the sink back once poll_ready completes.

Now, getting a value and sending it becomes:

my_value.join(sink.ready()).and_then(|(value, sink)| {
    sink.start_send(value) // or send if you want to include flush as part of the logic.
})

Anyway, I am only hypothesizing based on the OP & my personal experience, so

[aturon]

@carllerche I see, thanks!

One thing I still worry a bit about: in general, after a complete send, you also need to ensure that you are continually polling to flush, even if you don't want to block on flushing. That's not hard to do when you're writing tasks manually, but I don't actually know how this is going to work with async/await notation...

[carllerche]

@aturon is this significantly different than io::Write which has a flush?

Another question, can async / await have async drop handlers? That would allow ensuring flush in "async drop".

[carllerche]

If an async fn converts to a future, it seems like it should be possible to have trait AsyncDrop that must complete before the returned future from the async fn realizes.

/cc @withoutboats

[jsgf]

@aturon The specific case related to sync::mpsc::channel. The Sender implements Sink, so you can either use start_send/poll_complete, or use send which returns Future-implementing Send. However the start_send/poll_complete can't be used in an async way (that I can see), and send consumes the Sender until it completes - making the m part of mpsc moot. We tried to work around it by cloning the Sender but it turns out that the channel bound is applied to each individual Sender instance, so we could easily blow out the memory budget by having too many things in flight. I think @alexcrichton and @kulshrax have more context on this.

In the end we ended up using an unbounded channel and tried to limit the number of in-flight things manually.

But I think a channel wasn't really the right thing to use at all - all we really wanted was the ability to poll a stream/future across threads which would have avoided the notion of sending something at all.

I'm curious about what the lifetime problems are with "stream consumer" model, and whether they're worse than with Sink. Do you have a pointer to more details?

[jsgf]

Hm, I guess I'd overlooked that start_send returning Ok(AsyncSink::NotReady(x)) will reschedule the task when the channel has space again, and is therefore logically equivalent to Async::NotReady. So I guess its possible to implement bounded mpsc in an async environment, but it's definitely not pretty.

[seanmonstar]

I don't think it was clear at all in the RFC that unsync was going away and sync was just what would become futures-channel.

[aturon]

@seanmonstar Oy, you're right. To be clear, unsync will stay around in some form, but not be included in the futures facade. The need for it will decrease quite a bit once internal borrowing is possible, which is making fast progress.

[aturon]

I'm going to go ahead and close this out; we're not going to rename Sink at this point.