read the enitre message in one go without copies

[MaxVerevkin]

No description provided.

[MaxVerevkin]

I've run cargo clippy --fix and it fixed more stuff than I expected, probably because I'm on nightly.

[KillingSpark]

I think this needs to be if self.msg_buf_in.len() != max_buffer_size { so that msg_buf_in also gets resized to be smaller if the next message is smaller than the previous one.

[MaxVerevkin]

Why? I specifically didn't do this to avoid unnecessary memsets.

[KillingSpark]

Because the msg_buf_in should only ever contain at most as many bytes as the message that is currently being read.

I think the msg_buf_in should also be resized after the read to match how many bytes actually where received

[MaxVerevkin]

This information is stored in msg_buf_filled. Imagine you receive a sequence of messages with the following sizes: 1K, 128B, 512B, 1K. The way this PR works right now, resizes would write 1K of zeroes. With your suggestion it would write at least 2K-128B of zeroes (best case, if each message is read in one recvmsg).

[KillingSpark]

That is true but I would accept that cost for the sake of an easier invariant and making the msg_buf_filled field unnecessary. The few memsets are unlikely to be a bottleneck in the library.

Another thing I don't really like is that the code currently "reuses" the msg_buf_in by cloning (part of) it when a full message has been received. I cases where there are typically small messages received one big message will forever bloat the memory usage.

[KillingSpark]

Could you point where the cloning is?

rustbus/src/wire/unmarshal.rs:175

I think it would be better to do a swap here with a newly allocated vec of a sensible default size instead of doing it like this. Or maybe there is an even better way?

Right. Exactly as it was. .clear() does not release any memory

Yes, I am aware. I wanted to say that it's currently unsatisfying and that, while we are working on this anyways we could improve this aspect too.

[MaxVerevkin]

Could you point where the cloning is?

rustbus/src/wire/unmarshal.rs:175

I think it would be better to do a swap here with a newly allocated vec of a sensible default size instead of doing it like this. Or maybe there is an even better way?

Thanks! I completely missed this. A swap, even with an empty vector, sounds reasonable.

[MaxVerevkin]

So, should I .drain(..offset) bytes from the buffer, or keep the offset to avoid shifting?

[KillingSpark]

I think draining is fine as a first step towards improvement. Ideally the initial offset would be passed through to the unmarshalling code though. But that could be a separate PR, or you could leave it for me :)

[MaxVerevkin]

I've implemented the take+drain part, but left the msg_buf_filled for now. The annoying thing is that without it the message must be resized back in case of any error, not just partial reads.