Following an usual non-blocking pattern implementation adds extra time. #195
Comments
|
That's roughly what we do. Though we can't remove |
|
Thanks for the answer! I've taken a look, and until I could see, this "last" call to the underlying I know that I'm moving at the microseconds level. For a big message, it has no impact. But if the message is small, the overhead the library produces in reading it is almost double. In a use case where a client sends a lot of several small messages, the receiver of those messages will be considerably impacted. Maybe an easy solution for this could consist of starting reading from the network inside Is there any reason to do the writing part before the reading one inside the |
Your considerations make sense, but if you're using the sockets in a non-blocking mode the proper approach would be to not call
You mean trying to read from the socket and in case it returns |
Of course, the problem is not knowing when to call
So minimum, in order to read only one message, you need two calls to
Yes. When you receive a Of course, this change wouldn't apply to blocking sockets since a read call never returns a |
Your reasoning is correct, I have nothing to add to it. In fact we, at Snapview, initially designed our own scheduler based on
We do return If I got your point right, you basically would like to have the possibility to call read and write separately, so that read only reads and write only writes, i.e. going one level lower (in this case it would be up to the user to ensure adhering to the RFC). Currently the pseudocode is: Was your suggestion about having |
|
My suggestion is moving It's true what you say about reading before writing. As a first view, if you read before write the pending data and receive
In pseudocode, the loop {
let maybe_message = read()?;
write_pending();
if let Some(message) = maybe_message {
return Ok(m);
}
}In this way, you save from call |
I think I did not quite get the point. Being ready to read and to write are 2 types of readiness. But maybe it's all about the steps that you summarized, see below.
Ok, I think I got what you mean. The thing is that if the socket is woken up and we know that we can read does not imply that
It is possible, see above. |
Of course, but you only call to
I agree with it, and I support the idea of calling
Why this distinction among 2. and 3.? because when you successfully read a message (from 1.) you are forced to call again Also, note that the socket never wakes up for read if in the first call to Sorry if my English is not the best and there is some lost information on it 😝 |
What do you mean by "there is no data in the first call to stream.read"? That we read 0 bytes (connection closed) or that we read not enough bytes to compose the message? The thing is that the (3) can only happen if you call If we remove the |
Neither, I mean that we get a
Suppose the following:
What I mean is that a socket ready for reading implies 0 or more calls to
Yes, the first time I call it. There is a potential last call that will not have any data to read and a
I think that there is a third option. Only when no bytes had been read and |
|
I think that code change would more visual, I will try to write a draft and share it with you. |
|
As a draft: pub fn read_message<Stream>(&mut self, stream: &mut Stream) -> Result<Message>
where
Stream: Read + Write,
{
// Do not read from already closed connections.
self.state.check_active()?;
loop {
match self.read_message_frame(stream) {
Ok(maybe_message) => {
// Since we may get ping or close,
// we need to reply to the messages even during read.
// Thus we call write_pending() but ignore its blocking.
self.write_pending(stream).no_block()?;
// Thus if read blocks, just let it return WouldBlock.
if let Some(message) = maybe_message {
trace!("Received message {}", message);
return Ok(message);
}
}
Err(Error::Io(err))
if err.kind() == IoErrorKind::WouldBlock && self.frame.is_empty() =>
{
// If there was nothing read and a WouldBlock was returned, avoid write.
// This means the user call this function but the socket was not prepared to
// read.
return Err(Error::Io(err));
}
Err(e) => {
self.write_pending(stream).no_block()?;
return Err(e);
}
}
}
}
|
Ok, I see. And your concern is that in such case calling
Ok, thanks for the code, that gives a bit more context. So the problem with the approach that you suggested is that we don't even have access to the |
Exactly, this is the point :)
If this is the case, the example I shared should manage this correctly, because when you read a message the method will do the call to
Maybe I do not explain well what the hypothetical Sorry if this question is taken so many messages 😅 . Only a last doubt or concern. Why did you decide a design for |
Ok, so let's check if this is really the case i.e. if there is a safe way to drop this without unexpected side effect, see below.
Yes, but there is no guarantee that
But the buffer will contain bytes even if we did not read any data (the
Well for the sake of simplicity I would merge steps "prepare input buffer" and "read from stream" in one, because these are actually implementation details, i.e. we need to prepare some buffer to be able to read into it from the stream. So the actual question is why do we do (and this is likely to work wrong as well, looks like a very brittle code for me) |
This problem could happen either, you put the read_message() {
loop {
write_pending().no_block() //not enough bytes, the pong is not sending yet.
if Some(message) = read_message_frame()? { //Return a successful message without send the previous pong.
return Ok(message)
}
}
}Anyway, I understand the complexities arising from reading and writing inside `read_message(), and maybe I can not expect this performance increment in the non-blocking case. Thanks for your answers and time! |
|
You're welcome! If the questions have been cleared, feel free to close the issue ;) |
Hi, first of all, thank you for your amazing work in tungstenite-rs!
I am using your library with non-blocking sockets, and all is working fine. Nevertheless, when I compare times among reading 1-byte in blocking and non-blocking ways I noticed that the non-blocking was around twice as slow:
8usfor reading 1-byte in a blocking schema and around15usreading in a non-blocking schema.Investigating about it, this double increment comes from in non-blocking schema I'm "force" to call
WebSocket::read_message()twice. Once to receive the byte I sent, and other call to receive theWouldBlockthat notify me that there are no more messages:Currently, I fixed this to avoid the second call to
read_message()by checking by my self if there is data in the stream or the socket would block:With the above code, I correctly read a 1-byte message in
8usinstead of15us, but is far to be obvious for a non-blocking user, that is used to perform this kind of reading pattern until gettingWouldBlock.Taking a look inside
read_message()I saw that there is a lot of things done inside. Maybe these things should be done only in the caseread_message()has data to read and if not, perform an early exit withWouldBlock.The text was updated successfully, but these errors were encountered: