util: Add BoxLayer
#569
util: Add BoxLayer
#569
Conversation
| /// } | ||
| /// ``` | ||
| pub struct BoxLayer<In, T, U, E> { | ||
| boxed: Arc<dyn Layer<In, Service = BoxService<T, U, E>> + Send + Sync + 'static>, |
There was a problem hiding this comment.
kinda ironic that this is called BoxLayer but it's actually ArcLayer :P
i think this is probably the best name for this, for consistency & because it produces BoxServicees, but i just thought it was funny...
|
|
||
| impl<In, T, U, E> fmt::Debug for BoxLayer<In, T, U, E> { | ||
| fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { | ||
| fmt.debug_struct("BoxLayer").finish() |
There was a problem hiding this comment.
here's a thought; not a blocker for this PR: something we could do, and that we might want to think about doing for BoxService as well, is "remembering" the type name of the erased layer, like this:
pub struct BoxLayer<In, T, U, E> {
boxed: Arc<dyn Layer<In, Service = BoxService<T, U, E>> + Send + Sync + 'static>,
name: &'static str,
}
impl<In, T, U, E> BoxLayer<In, T, U, E> {
/// Create a new [`BoxLayer`].
pub fn new<L>(inner_layer: L) -> Self
where
L: Layer<In> + Send + Sync + 'static,
L::Service: Service<T, Response = U, Error = E> + Send + 'static,
<L::Service as Service<T>>::Future: Send + 'static,
{
let layer = /* ... */
Self {
boxed: Arc::new(layer),
name: std::any::type_name::<L>(),
}
}
}
// ...
impl<In, T, U, E> fmt::Debug for BoxLayer<In, T, U, E> {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
fmt.debug_struct("BoxLayer")
.field("boxed", &format_args!("Box<{}>", self.name))
.finish()
}
}This would let us see what the original, un-erased type was in the Debug impl, which could be useful for debugging in some cases.
The main downside to this is that it makes the one-word BoxLayer struct suddenly three words long (a &'static str is a ptr+len, so two words). This might be meaningful if you're cloning a really large number of BoxLayers.
Alternatively, we could stick the type name inside the Arc, so it's not part of each clone of the struct...but then we'd need some
struct Inner<In, T, U, E> {
boxed: Box<dyn Layer<In, Service = BoxService<T, U, E>> + Send + Sync + 'static>,
name: &'static str,
}which is also not great — now we're dereferencing two heap pointers to call self.inner.layer, rather than one.
A third idea is to do the first proposal, but stick #[cfg(debug_assertions)] on the name field and the code that uses it. That way, type names are included in debug mode, but we remove the extra two words from BoxLayer in release mode for performance. IMO, that might be the best choice if we decide to do something like this.
Anyway, just an idea...probably out of scope for this PR and best done in a follow up, if we decide this is worth it.
| mod sync; | ||
| mod unsync; | ||
|
|
||
| #[allow(unreachable_pub)] // https://github.com/rust-lang/rust/issues/57411 | ||
| pub use self::{sync::BoxService, unsync::UnsyncBoxService}; | ||
| pub use self::{layer::BoxLayer, sync::BoxService, unsync::UnsyncBoxService}; |
There was a problem hiding this comment.
do we think there's much of a use-case for an UnsyncBoxLayer as well? I think what we might want is a Layer that is Sync (and still Arced), but that produces UnsyncBoxServices instead of BoxServices.
but, i guess that's just something we can easily add later if it turns out anyone needs it...
|
Went ahead and made the remaining docs tweaks, gonna merge this pending CI 👍 We can look into improving debug output in a follow-up...or, it may not be worth the effort... |
Deprecated - **util**: Deprecated `ServiceExt::ready_and` (renamed to `ServiceExt::ready`). ([#567]) - **util**: Deprecated `ReadyAnd` future (renamed to `Ready`). ([#567]) Added - **builder**: Add `ServiceBuilder::layer_fn` to add a layer built from a function. ([#560]) - **builder**: Add `ServiceBuilder::map_future` for transforming the futures produced by a service. ([#559]) - **builder**: Add `ServiceBuilder::service_fn` for applying `Layer`s to an async function using `util::service_fn`. ([#564]) - **util**: Add example for `service_fn`. ([#563]) - **util**: Add `BoxLayer` for creating boxed `Layer` trait objects. ([#569]) [#567]: #567 [#560]: #560 [#559]: #559 [#564]: #564 [#563]: #563 [#569]: #569
Deprecated - **util**: Deprecated `ServiceExt::ready_and` (renamed to `ServiceExt::ready`). ([#567]) - **util**: Deprecated `ReadyAnd` future (renamed to `Ready`). ([#567]) Added - **builder**: Add `ServiceBuilder::layer_fn` to add a layer built from a function. ([#560]) - **builder**: Add `ServiceBuilder::map_future` for transforming the futures produced by a service. ([#559]) - **builder**: Add `ServiceBuilder::service_fn` for applying `Layer`s to an async function using `util::service_fn`. ([#564]) - **util**: Add example for `service_fn`. ([#563]) - **util**: Add `BoxLayer` for creating boxed `Layer` trait objects. ([#569]) [#567]: #567 [#560]: #560 [#559]: #559 [#564]: #564 [#563]: #563 [#569]: #569
Deprecated - **util**: Deprecated `ServiceExt::ready_and` (renamed to `ServiceExt::ready`). ([#567]) - **util**: Deprecated `ReadyAnd` future (renamed to `Ready`). ([#567]) Added - **builder**: Add `ServiceBuilder::layer_fn` to add a layer built from a function. ([#560]) - **builder**: Add `ServiceBuilder::map_future` for transforming the futures produced by a service. ([#559]) - **builder**: Add `ServiceBuilder::service_fn` for applying `Layer`s to an async function using `util::service_fn`. ([#564]) - **util**: Add example for `service_fn`. ([#563]) - **util**: Add `BoxLayer` for creating boxed `Layer` trait objects. ([#569]) [#567]: #567 [#560]: #560 [#559]: #559 [#564]: #564 [#563]: #563 [#569]: #569 Signed-off-by: Eliza Weisman <eliza@buoyant.io>
I've run into a use case where I need to return a
Layerwith a complex type from a function. I previously usedimpl Layer<S, Service = impl Service<...>>but that impacted compile times quite significantly. Boxing theLayerfixed it. Thought it made sense to upstream.The
Send + Sync + 'staticbounds on the innerdyn Layerwere necessary to get it working with hyper.