Closed Fotosmile closed 4 years ago
Hi! Thanks for the detailed write-up! You’re totally right we should have a way to cancel the reservation. What about we leverage truncate
for this as well? We could have an additional .cancel
method that zeroes the length of the reservation, which effectively cancels the send operation.
What do you think of something like https://github.com/utaal/spsc-bip-buffer/pull/7/files ?
Yeah, the cancel
can be a solution for some cases. But there may be the case when you must manually cancel a reservation in all Result cases:
struct WriterFuture {
writer: BipBufferWriter,
stream: TcpStream,
}
impl Stream for WriterFuture {
type Item = usize;
type Error = io::Error;
fn poll(&mut self) -> Poll<Option<Self::Item>, Self::Error> {
loop {
if let Some(mut reserved) = self.writer.reserve(1024) {
match self.stream.poll_read(reserved.as_mut()) {
Ok(Async::Ready(Some(0))) => {
reserved.cancel(); // first cancel
return Ok(Async::Ready(None));
}
Ok(Async::Ready(Some(n))) => {
// do something with the buffer
reserved.truncate(n);
}
Ok(Async::Ready(None)) => {
reserved.cancel(); // second cancel
return Ok(Async::Ready(None));
}
Ok(Async::NotReady) => {
reserved.cancel(); // third cancel
return Ok(Async::NotReady);
}
Err(e) => {
reserved.cancel(); // fourth cancel
return Err(e);
}
}
} else {
return Ok(Async::NotReady);
}
}
}
}
So, it is not very comfortable to cancel the reservation for every case. Instead, I would like to use BipBufferWriterReservation
without auto-send-in-drop implementation. Then, the code will look like this:
struct WriterFuture {
writer: BipBufferWriter,
stream: TcpStream,
}
impl Stream for WriterFuture {
type Item = usize;
type Error = io::Error;
fn poll(&mut self) -> Poll<Option<Self::Item>, Self::Error> {
let mut bytes_written = 0;
loop {
if let Some(mut reserved) = self.writer.reserve(1024) {
match try_ready!(self.stream.poll_read(reserved.as_mut())) {
Some(0) => {
return Ok(Async::Ready(None));
}
Some(n) => {
// do something with the buffer
reserved.truncate(n);
reserved.send(); // explicit send, but only one!
}
None => {
return Ok(Async::Ready(None));
}
}
} else {
return Ok(Async::NotReady);
}
}
}
}
So, without send-in-drop we are able to not handle every possible Result with sub-options to manually cancel the reservation. Instead, we call only once send
. And in terms of optimization, with send-in-drop, the sending of bytes (even if the cancel method was called) will be called every time what processor time will be spent on, but what will not be without send-in-drop. Without send-in-drop, the code to send bytes is executed only for the real sending of bytes.
That's a good point! Thank you for the detailed explanation and code examples.
I've updated the pull request with a new method, set_manual_send
: when called, it disables the auto-send behaviour, and requires an explicit send. I think your code example would look like the following:
struct WriterFuture {
writer: BipBufferWriter,
stream: TcpStream,
}
impl Stream for WriterFuture {
type Item = usize;
type Error = io::Error;
fn poll(&mut self) -> Poll<Option<Self::Item>, Self::Error> {
let mut bytes_written = 0;
loop {
if let Some(mut reserved) = self.writer.reserve(1024) {
reserved.set_manual_send();
match try_ready!(self.stream.poll_read(reserved.as_mut())) {
Some(0) => {
return Ok(Async::Ready(None));
}
Some(n) => {
// do something with the buffer
reserved.truncate(n);
reserved.send(); // explicit send, but only one!
}
None => {
return Ok(Async::Ready(None));
}
}
} else {
return Ok(Async::NotReady);
}
}
}
}
Thoughts?
set_manual_send
looks good. Thank you for the code updates! :)
Hi! Consider we have the next case:
(the error is not explicit, and users can spend some time to figure out where is the problem) So, the reservation does not always have to be sent to the receiver, as there are cases when something went wrong with the reservation (for example, an error occurred and you need to try to reserve and fill the buffer later). Therefore, I think the code from
drop
should be moved tosend_impl
. But, of course, we should not leave users without auto-sending, as not everyone has cases where errors are possible. Therefore, I think we should move the code fromdrop
forBipBufferWriterReservation
to thesend_impl
method, and delete thedrop
method. But add anotherstruct BipBufferWriterReservationAutoSend(BipBufferWriterReservation)
(the name should be better), which can be obtained from the call for theBipBufferWriter::reserve_auto_send
method, and BipBufferWriterReservationAutoSend can look like this: