Consider adding a fn to Sink that can be used to check if the sink is ready to accept a value without actually sending one.

[carllerche]

I propose Sink::poll_ready (in part so this issue shows up in searches).

I remember discussing this at the December 16 work week, but I don't entirely remember all the details. I didn't find another issue tracking this, so I am opening one (please close if this is a dup). This function can help in cases where it is useful to know if a start_send will succeed before creating the value. Imagine that storing the value is has a cost compared to generating & sending it immediately with no buffered storage.

Relates to #256.

[alexcrichton]

I'd be ok adding this, but I think it's important to default to Ready(()) to be ergonomic

[carllerche]

Ok, so you have to help me remember the conclusion we reached in Hawaii.. maybe @aturon has notes!

So, it's important for poll_ready() in this case to not return any false positives (otherwise it defeats the purpose of avoiding storage).

I also agree that ergonomics are important, so maybe the conclusion was that this shouldn't be a fn on Sink?

Maybe it was a separate trade w/ some sort of combinator that provided poll_ready?

[carllerche]

Basically, you can always make any Sink be able to provide poll_ready by wrapping it with a 1 slot buffer, but you want to avoid that storage in the case that the Sink knows the answer, like mpsc::Sender.

[aturon]

Here's what I have in my notes:

• Add combinator for poll_ready, basically follow the fuse pattern, use the code in proto
  • This guarantees that if poll_ready succeeds then it will accept a message

[carllerche]

Thanks... now hopefully that jogs someones memory :P

[alexcrichton]

Hm so "like the fuse pattern" may imply:

• A separate PollReady trait
• A Sink::poll_ready combinator which returns a SinkPollReady<T>
• Implement a SinkPollReady::poll_ready function
• Specialize SinkPollReady to do nothing for types that implement PollReady and have a one-slot storage for all other types

(modulo all the naming conflicts I guess)

[carllerche]

So, after talking a bunch about this in IRC, the conclusion is that this is tricky.

• Sink cannot have a poll_ready function because it doesn't make sense for all Sink implementations.
• Adding a new trait PollReady doesn't make sense, because, Sink + Stream + PollReady would be confusing. Does PollReady apply to the sink or stream half?
• Introducing this new concept is difficult to do in a backwards compatible way as false positives cannot be permitted. This means it needs to be punted to 0.2.
• The "fusing" feature is a nice to have, but is tricky as to how to implement.

The best option that came up was to introduce a new trait (naming TBD):

trait ReadySink: Sink {
   fn poll_ready(&mut self) -> Async<()>,
}

The problem here is that, this is yet another trait to implement and all implementations of Sink / Stream now have to add and forward. This is getting a bit crazy.

So, the exact strategy by which to introduce this is still TBD and will need more consideration.

For the immediate, structs can implement poll_ready directly.

[carllerche]

Another question, should poll_ready return Poll instead of Async?

The behavior of poll_ready on Sink seems to be a subset of the start_send behavior. Specifically:

This method returns AsyncSink::Ready if the sink was able to start sending item. In that case, you must ensure that you call poll_complete to process the sent item to completion. Note, however, that several calls to start_send can be made prior to calling poll_complete, which will work on completing all pending items.

In which case, start_send impls should be implemented as:

fn start_send(&mut self, item: Self::Item) -> StartSend<Self::SinkItem, Self::SinkError> {
    if !try!(self.poll_ready()).is_ready() {
        return Ok(AsyncSink::NotReady(item));
    }

    // Rest of the implementation
}

poll_ready is a pretty critical piece of functionality for Sink decorators that perform a one way mapping of an item. Aka, the decorator must be able to know that the inner Sink can accept the item before mapping it because if the inner Sink returns NotReady(item) the decorator has no way to map the item back to the original type.

[carllerche]

On the topic of false positives:

In the "one way mapping" case, if poll_ready is permitted to return false positives, then how should the decorator handle this false positive? The implementation is unable to return AsyncSink::NotReady, so what should it do?

[carllerche]

Sink cannot have a poll_ready function because it doesn't make sense for all Sink implementations.

Is there a known case? I'm sure there is, but nobody wrote it down!

[alexcrichton]

If we disallow false positives does that mean we allow false negatives? In other words, how would we impement poll_ready for the mpsc channel type?

[carllerche]

Well, the mpsc channel type is able to guarantee that if poll_ready returns Ready, the next call to start_send on that handle will be accepted. This is because each Sender handle has a dedicated slot.

[alexcrichton]

Hm perhaps yeah. In general I feel like trying to disallow false positives is too strong of a guarantee we can provide here. IIRC we came up with a number of situations where such a guarantee would impose a cost to provide where it otherwise wouldn't be necessary. You can always write a wrapper which provides the guarantee, no?

[carllerche]

@alexcrichton well, the problem is that if poll_ready can return false positives, I don't see much of a purpose to have it. In fact, allowing false positives will mean it isn't useable in the case that I described above (one way mappings).

Maybe the answer is if you can't guarantee no false positives, you don't implement ReadySink?

[alexcrichton]

@carllerche hm this is all sounds very familiar like we've discussed this a few times... This sounds, though, like a "fused iterator" style problem? ReadySink could be a trait implemented for all sinks (maybe?) with a combinator to make it "correct"?

[carllerche]

That goes back to "The "fusing" feature is a nice to have, but is tricky as to how to implement." :) I'm not really sure what the "fused iterator" style would look like in this case.

[aturon]

This happened in 0.2.