-
Notifications
You must be signed in to change notification settings - Fork 38
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
use reference counted prefix for less lifetimes on params? #28
Comments
Wouldn't most people allocate again anyways (if its used in a Path extractor for example)? Unless it used the bytes crate maybe. |
On a further look a zero copy value is also possible in form like this: struct Param {
key: BytesStr,
value: BytesStr
}
impl Node {
fn at(&self, path: impl Into<ByteStr>) {}
} This would hit the perf even further when given a str to match with one extra copy and a couple of reference counted slicing on it. But in real world |
I like that The only problem I've noticed is the one described in #40 and describe there a way to delay any allocations to the point where actual path arguments are known to exist. In the (I assume common case) where there is None, no allocations need to happen.I use |
http/hyper already uses Bytes internally, though not fully exposed in it's public API, so we might still be able to be zero-copy in the common use case. |
It's when you use it in a complicated web framework abstraction the lifetime pollution becomes apparent. This is how matchit is used in axum: https://github.com/tokio-rs/axum/blob/368c3ee08fc3896358d3bd2bfc8cc67f2925c6ef/axum/src/routing/url_params.rs#L24
There is a related issue: hyperium/http#484 though it's unlikely to happen before some form of widely used ref count string type exists (either in std or 3rd party). |
I guess the most generic way of supporting this would be to return a |
That doesn't look bad. Seems it comes down to " Anyway, I agree that I thought that maybe These two effort could be combined. Matching could support internally some conversions to representations that are more useful for general purpose code at the cost of extra allocations etc, and then also enable developers converting to whatever they want as well. If |
A trade off between less lifetime and reference counted overhead + an extra public type. Current Param(s) type are pretty hard to use if not consumed inline. From what I see most people would do an extra heap allocation to erase the lifetime(s) so in that case this would likely be cheaper.
The text was updated successfully, but these errors were encountered: