fred.rs
fred.rs copied to clipboard
RedisKey::from(&str) constraint to 'static lifetimes
Typically, when callers pass a key of type &str
, it most of the time from a &'static str
. This pull request takes this into account and avoid an allocation.
If the caller has constructed a string, for example with format!("users/{}", user.id)
, then we'll take this as a String
, and there's no allocation needed.
If the caller constructs a string, but wants to reuse it, the caller will have to invoke .clone()
on the string.
Hi @nviennot,
Thanks for the PR. A few things:
I'm not sure I agree with this:
Typically, when callers pass a key of type &str, it most of the time from a &'static str.
In my experience, especially with keys, these are almost never static. However, at the end of the day this is subjective and the library needs to support both use cases no matter what.
In an ideal world we could specialize the Into
impl's so that 'static
types went through a happy path to avoid allocations, but unfortunately that's not on the table. I opted to follow the same pattern the bytes
crate uses with respect to from_static, which is to say that the "easy" path (via Into
) is the most ergonomic, but not necessarily the most performant. For the callers that want the added performance of avoiding allocations, they can use the from_static
variant on RedisKey
. The bytes
crate is far more popular than this library, so I'm also piggybacking on their design decisions here a bit too.
All things considered I think I'm going to hold off on this unless anybody can give a compelling argument for how this moves the needle on ergonomics or use cases with this library in general. At the end of the day it's possible to avoid allocations if needed with either approach, but this change would be a breaking change, so for that reason alone I'd like to hold off.