Avoid boxing in Connector chains #919
Conversation
algesten
force-pushed
the
fix/no-boxing
branch
from
f977ff5 to
3b63fef
Compare
algesten
changed the title
algesten
force-pushed
the
fix/no-boxing
branch
3 times, most recently
from
7fafe3a to
2c2af87
Compare
| pub struct ChainedConnector<In, First, Second>(First, Second, PhantomData<In>); | ||
|
|
||
| impl<In, First, Second> Connector<In> for ChainedConnector<In, First, Second> | ||
| where | ||
| In: Transport, | ||
| First: Connector<In>, | ||
| Second: Connector<First::Out>, | ||
| { | ||
| type Out = Second::Out; | ||
|
|
||
| fn connect( | ||
| &self, | ||
| details: &super::ConnectionDetails, | ||
| chained: Option<In>, | ||
| ) -> Result<Option<Self::Out>, crate::Error> { | ||
| let f_out = self.0.connect(details, chained)?; | ||
| self.1.connect(details, f_out) | ||
| } | ||
| } | ||
|
|
||
| impl<In, First, Second> ChainedConnector<In, First, Second> { | ||
| pub(crate) fn new(first: First, second: Second) -> Self { | ||
| ChainedConnector(first, second, PhantomData) | ||
| } | ||
| } | ||
|
|
||
| impl<In, First, Second> fmt::Debug for ChainedConnector<In, First, Second> | ||
| where | ||
| In: Transport, | ||
| First: Connector<In>, | ||
| Second: Connector<First::Out>, | ||
| { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| f.debug_tuple("ChainedConnector") | ||
| .field(&self.0) | ||
| .field(&self.1) | ||
| .finish() | ||
| } | ||
| } |
There was a problem hiding this comment.
One way would be to implement this for tuples of sizes 0 to 8 through a macro.
There was a problem hiding this comment.
You mean as an alternative syntax to .chain().chain().chain()?
There was a problem hiding this comment.
Sure. I don't know how much extra it aids clarity though?
There was a problem hiding this comment.
This avoids ugly ChainedConnector<First, ChainedConnnector<Second, ChainedConnector<...,...>>> types and would instead allow:
ChainedConnector<(First, Second, Third, ...)>
There was a problem hiding this comment.
Changing a bit the DefaultConnector code to create a type error leads to this. The full type is ChainedConnector<(), ChainedConnector<(), ChainedConnector<(), ChainedConnector<(), ChainedConnector<(), ChainedConnector<(), (), WarnOnNoSocksConnector>, TcpConnector>, RustlsConnector>, WarnOnMissingTlsProvider>, WarnOnMissingTlsProvider>, ConnectProxyConnector>
It also means that DefaultConnector boxes the inner instead of having a more explicit type (and self-documenting)
#[derive(Debug)]
pub struct DefaultConnector {
inner: ChainedConnector<(), (
#[cfg(feature = "_test")]
test::TestConnector,
#[cfg(feature = "socks-proxy")]
SocksConnector,
#[cfg(feature = "socks-proxy")]
WarnOnNoSocksConnector,
TcpConnector,
#[cfg(feature = "rustls")]
RustlsConnector,
#[cfg(feature = "native-tls")]
NativeTlsConnector,
#[cfg(feature = "_tls")]
WarnOnMissingTlsProvider,
)>,
}There was a problem hiding this comment.
Isn't that just the nature of combinators in Rust? Like every single thing on an Iterator produces some intermediary type.
There was a problem hiding this comment.
Iterators are very rarely stored and named. And given the combinators used with iterator include closures, you wouldn't be able to name them anyways. Here the we combine named types, and there are only two types of combinators: Chaining and Either. I'm not suggesting doing this for Either because it's much more annoying to generate many enum types. Chaining is pretty well suited to being made easy to name.
There was a problem hiding this comment.
Just using Iterator as example. Another is something like warp https://crates.io/crates/warp
There was a problem hiding this comment.
More recently, xilem, axum and leptos do what I'm thinking about, implementing traits on many tuple length through a macro.
I don't think this is something very important. It's still possible to build combinators in a downstream crate.
|
Thank you! I'll try find the time to see if this fits our use case this month. |
Close #890