Conversation
| /// An object that represents a connected peer and associated state. | ||
| /// | ||
| /// # Usage of Framed | ||
| /// [`Framed`] is used for encoding and decoding messages ("frames"). | ||
| /// Usually, [`Framed`] has its own internal buffering mechanism, that's respected | ||
| /// when calling `poll_ready` and configured by [`Framed::set_backpressure_boundary`]. | ||
| /// | ||
| /// However, we don't use `poll_ready` here, and instead we flush whenever the write buffer contains | ||
| /// data (i.e., every time we write a message to it). | ||
| pub(crate) struct PeerState<T: AsyncRead + AsyncWrite, A: Address> { |
There was a problem hiding this comment.
Reading this makes me think: do we really need Framed then? Can't we extract the encoding/decoding out of if and write to/read from the underlying transport?
There was a problem hiding this comment.
Maybe it's a just a matter of rephrasing the documentation, highlighting why we still think it's useful to have. Perhaps the polling functionality over incoming messages on the transport.
There was a problem hiding this comment.
Check the PR description above - for reading, it's nice to have, but for writing, we don't need it and it in fact results in an additional message copy (the raw message into the Framed buffer).
However, for high throughput profiles, it may actually make sense to reintroduce batched flushes to reduce syscall overhead. So we'll have to see
2 participants
Mainly ensures the
ReqDriverplays more nicely withFramed. We ignore Framed's internal backpressure mechanism (respected whenpoll_readyis called) and flush manually.I realized with this PR that we should get rid of
Framedat least for writing messages, as it needlessly copies and buffers bytes that could be written directly to an underlyingIoobject.