futures-rs: Current 0.3 Sink interface inflicts complexity on implementers
In my existing futures 0.1 Sink implementations, the single start_send method was able to access the SinkItem (it needs the item buffer’s length), calling tokio_threadpool::blocking if necessary and depending on that result, consuming the item buffer and returning Ready, or giving the item back via NotReady:
I’m now trying to port these Sinks to futures 0.3-alpha. With the current Sink interface, it is necessary to signal any potential for Poll::Pending (for Sink meaning lack of readiness to receive) in the trait method poll_ready, before the Item buffer is given.
The futures compatibility layer already deals with this interface difference, in the Sink01As03 shim (#1364), by maintaining an additional buffer of one Item, and deferring the actual consumption of that Item to subsequent calls to poll_ready and poll_flush.
I couldn’t find an original rationale for this change to the Sink interface? Intuitively I would think it should simplify aspects of the Forward combinator, but at the expense of pushing the issue to most Sink implementers, where the complexity risks bugs. As we are still alpha, is there any willingness to reconsider this interface change? Alternatively, would it be reasonable to introduce here some alternative trait, e.g. TardySink:
/// A sink which doesn't know it's readyiness until it tries to consume an Item
trait TardySink<Item> {
type SinkError;
fn poll_send(self: Pin<&mut Self>, cx: &mut Context, item: Item)
-> (Poll<Result<(), Self::SinkError>>, Option<Item>);
fn poll_flush(self: Pin<&mut Self>, cx: &mut Context)
-> Poll<Result<(), Self::SinkError>>;
fn poll_close(self: Pin<&mut Self>, cx: &mut Context)
-> Poll<Result<(), Self::SinkError>>;
}
…and impl Sink<Item> for TardySink<Item> using the same 1-item buffering strategy as in Sink01As03, but not depending on anything from futures 0.1? Note, the tuple return type of poll_send above is awkward. An alternative would be a TardyPoll<Item> enum with TardyPoll::Pending(Item) variant and some easy conversion to Poll.
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Comments: 15 (15 by maintainers)
@dekellum
How about SinkExt::buffer?
I managed to implement the pin-projections and removed the
Unpinbounds:https://github.com/dekellum/tardy-sink/blob/master/src/lib.rs