-
Notifications
You must be signed in to change notification settings - Fork 1.2k
core/muxing: Generalise StreamMuxer::poll_address_change to poll
#2797
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
aa1a242
febd518
ce958fc
9a0eed2
fc61a68
360857b
55c0da0
d9c2e6a
1479271
8f8fa91
ee0a76c
94ae705
28a2fb7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,14 +86,6 @@ pub trait StreamMuxer { | |
| cx: &mut Context<'_>, | ||
| ) -> Poll<Result<Self::Substream, Self::Error>>; | ||
|
|
||
| /// Poll for an address change of the underlying connection. | ||
| /// | ||
| /// Not all implementations may support this feature. | ||
| fn poll_address_change( | ||
| self: Pin<&mut Self>, | ||
| cx: &mut Context<'_>, | ||
| ) -> Poll<Result<Multiaddr, Self::Error>>; | ||
|
|
||
| /// Closes this `StreamMuxer`. | ||
| /// | ||
| /// After this has returned `Poll::Ready(Ok(()))`, the muxer has become useless. All | ||
|
|
@@ -105,6 +97,22 @@ pub trait StreamMuxer { | |
| /// > properly informing the remote, there is no difference between this and | ||
| /// > immediately dropping the muxer. | ||
| fn poll_close(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>>; | ||
|
|
||
| /// Poll for an event of the underlying connection. | ||
| /// | ||
| /// In addition to returning an event, this function may be used to perform any kind of background | ||
| /// work that needs to happen for the muxer to do its work. Implementations can rely on this | ||
| /// function to be called regularly and unconditionally. | ||
| fn poll_event( | ||
thomaseizinger marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| self: Pin<&mut Self>, | ||
| cx: &mut Context<'_>, | ||
| ) -> Poll<Result<StreamMuxerEvent, Self::Error>>; | ||
| } | ||
|
|
||
| /// An event produced by a [`StreamMuxer`]. | ||
| pub enum StreamMuxerEvent { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any thoughts on just naming this I think there was a loose consensus around #2217 at some point but we haven't really made progress on this front.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 Sounds good to me.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am in favor of just |
||
| /// The address of the remote has changed. | ||
| AddressChange(Multiaddr), | ||
| } | ||
|
|
||
| /// Extension trait for [`StreamMuxer`]. | ||
|
|
@@ -131,15 +139,15 @@ pub trait StreamMuxerExt: StreamMuxer + Sized { | |
| Pin::new(self).poll_outbound(cx) | ||
| } | ||
|
|
||
| /// Convenience function for calling [`StreamMuxer::poll_address_change`] for [`StreamMuxer`]s that are `Unpin`. | ||
| fn poll_address_change_unpin( | ||
| /// Convenience function for calling [`StreamMuxer::poll_event`] for [`StreamMuxer`]s that are `Unpin`. | ||
| fn poll_event_unpin( | ||
| &mut self, | ||
| cx: &mut Context<'_>, | ||
| ) -> Poll<Result<Multiaddr, Self::Error>> | ||
| ) -> Poll<Result<StreamMuxerEvent, Self::Error>> | ||
| where | ||
| Self: Unpin, | ||
| { | ||
| Pin::new(self).poll_address_change(cx) | ||
| Pin::new(self).poll_event(cx) | ||
| } | ||
|
|
||
| /// Convenience function for calling [`StreamMuxer::poll_close`] for [`StreamMuxer`]s that are `Unpin`. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| use crate::muxing::StreamMuxerEvent; | ||
| use crate::StreamMuxer; | ||
| use futures::{AsyncRead, AsyncWrite}; | ||
| use multiaddr::Multiaddr; | ||
| use pin_project::pin_project; | ||
| use std::error::Error; | ||
| use std::fmt; | ||
|
|
@@ -38,11 +38,6 @@ where | |
| type Substream = SubstreamBox; | ||
| type Error = io::Error; | ||
|
|
||
| #[inline] | ||
| fn poll_close(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { | ||
| self.project().inner.poll_close(cx).map_err(into_io_error) | ||
| } | ||
|
|
||
| fn poll_inbound( | ||
| self: Pin<&mut Self>, | ||
| cx: &mut Context<'_>, | ||
|
|
@@ -65,14 +60,16 @@ where | |
| .map_err(into_io_error) | ||
| } | ||
|
|
||
| fn poll_address_change( | ||
| #[inline] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why inline this method but not the other ones, and why only inline it in this muxer? Just asking out of curiosity because I am not really familiar with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not on purpose to be honest. I thought I left everything as it was before. I am not too familiar with inlining either but the rough advice I got was that the compiler tends to be smarter on when it is needed :)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the many
Yes. Unless proven through a benchmark, let the compiler make the decision and thus don't use
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The method was marked as |
||
| fn poll_close(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { | ||
| self.project().inner.poll_close(cx).map_err(into_io_error) | ||
| } | ||
|
|
||
| fn poll_event( | ||
| self: Pin<&mut Self>, | ||
| cx: &mut Context<'_>, | ||
| ) -> Poll<Result<Multiaddr, Self::Error>> { | ||
| self.project() | ||
| .inner | ||
| .poll_address_change(cx) | ||
| .map_err(into_io_error) | ||
| ) -> Poll<Result<StreamMuxerEvent, Self::Error>> { | ||
| self.project().inner.poll_event(cx).map_err(into_io_error) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -109,11 +106,6 @@ impl StreamMuxer for StreamMuxerBox { | |
| type Substream = SubstreamBox; | ||
| type Error = io::Error; | ||
|
|
||
| #[inline] | ||
| fn poll_close(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { | ||
| self.project().poll_close(cx) | ||
| } | ||
|
|
||
| fn poll_inbound( | ||
| self: Pin<&mut Self>, | ||
| cx: &mut Context<'_>, | ||
|
|
@@ -128,11 +120,16 @@ impl StreamMuxer for StreamMuxerBox { | |
| self.project().poll_outbound(cx) | ||
| } | ||
|
|
||
| fn poll_address_change( | ||
| #[inline] | ||
| fn poll_close(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { | ||
| self.project().poll_close(cx) | ||
| } | ||
|
|
||
| fn poll_event( | ||
| self: Pin<&mut Self>, | ||
| cx: &mut Context<'_>, | ||
| ) -> Poll<Result<Multiaddr, Self::Error>> { | ||
| self.project().poll_address_change(cx) | ||
| ) -> Poll<Result<StreamMuxerEvent, Self::Error>> { | ||
| self.project().poll_event(cx) | ||
| } | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.