Futures-rs: Investigate compat for transforming 0.1 Sink into 0.3 Sink

[Nemo157]

The PR that added Stream and Sink to the compat layers (#1162) only added support for converting a futures(0.3)::Sink into a futures(0.1)::Sink because the API differences make the inverse transform non-straightforward. We should investigate whether this is possible at all, even if it requires some sort of extra runtime cost.

[Nemo157]

@LucioFranco, since you mentioned you’d be interested in helping with this I’ll try and write up a bit more detail on what issues I encountered with it, tomorrow.

[LucioFranco]

@Nemo157 sounds good! I'll be waiting for it.

[Nemo157]

Basically the issue is that in 0.1 Sink has a single method Sink::start_send that checks for capacity then either consumes the incoming element or returns NotReady if there wasn't enough capacity. In 0.3 Sink has two separate methods for this, Sink::poll_ready that checks whether there is capacity and returns NotReady if there isn't, and Sink::start_send which takes in an element to consume and presumably drops it and returns an error if you did not check poll_ready and there was no capacity available.

Implementing futures(0.1)::Sink for Compat<impl futures(0.3)::Sink> is then trivial, you can just call the two methods one after another to get the correct behaviour. Going the other way would require you to somehow call futures(0.1)::Sink::start_send from futures(0.3)::Sink::poll_ready to check for capacity, even though you don't have an item to send.

My first thought on a way to support this with a small runtime cost is to add a custom Compat struct with a single element buffer to let you claim the futures(0.3)::Sink is ready without having to check the underlying futures(0.1)::Sink. futures(0.3)::Sink::poll_ready can first attempt to push any buffered element into the sink, then return Ready if the buffer is empty. futures(0.3)::Sink::start_send can attempt to push the incoming element into the sink and fallback to just putting it in the buffer if there's no capacity.

[zargony]

The tarpc crate seems to have come up with similar code: https://github.com/google/tarpc/blob/master/bincode-transport/src/compat.rs (just FYI, found it coincidentally)

I'd be happy to see 0.1 Sink compat in futures-rs since it's not always easy to work around using 0.1 sinks.

[Nemo157]

Good note from the tarpc implementation, the extra Sink compact struct will need to also delegate Stream to support dual Sink + Stream types.

[manuels]

Would it be a big problem to just "steal" the code from tarpc? The licenses of both projects are the same.

[Nemo157]

Glancing at the tarpc code it has a bug in its close handling (similar to what I mention here). I'll try and take a look at #1364 again this weekend, I think it's pretty much ready to merge.

[Nemo157]

This was implemented in #1364