-
Notifications
You must be signed in to change notification settings - Fork 159
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
Label Allocation Optimization #395
Comments
This is definitely something I've thought about and even have an old, stale branch for trying to handle: https://github.com/metrics-rs/metrics/tree/cow-experiments Essentially, I envisioned the copy-on-write type that we use to handle either borrowed, owned, or shared ( I just never got as far as I wanted with it: lack of time/motivation, more or less. Dropping the bit for doing small string optimization would make it trivial to include support for shared pointers, and could be a good incremental step to revive the effort. |
Oh that sounds like it would solve our problem! Sounds like a nice solution. Re: small string optimization; are you saying you'd drop support for it? |
I would only drop the requirement of getting it out at the same time as support for smart pointers, for the purpose of trying to incrementalize improve things rather than doing it in one fell swoop. |
Related to #273 Another problem with dynamic labels is let a = "some_value";
metrics::gauge!("some_metric", 42., "label" => a); is expanded into static METRIC_NAME: &'static str = "some_metric";
if let Some(recorder) = ::metrics::try_recorder() {
let key = ::metrics::Key::from_parts(
METRIC_NAME,
(<[_]>::into_vec(
#[rustc_box]
$crate::boxed::Box::new([(::metrics::Label::new("label", a))]),
)),
);
let handle = recorder.register_gauge(&key);
handle.set(42.);
} It could be avoided by separating metadata (static usually) and values in the same way as the |
Taking the approach that I opened #405 as a placeholder to write down some thoughts on what your idea might look like. |
When using dynamic labels, it's necessary to either allocate a string each time or "forget" it to obtain a &'static str reference. The latter can in some cases (esp a naive solution) result in unbound memory growth, and the former results in a bunch of wasted time dealing with allocating and dropping Strings. For hot code paths, this can result in a lot of wasted CPU and degrade application performance.
A simple solution to this might be supporting Arc for labels, and a more sophisticated version might be some sort of label value registration / string interning managed by the metrics crate.
Not sure if this is a feature request exactly, but it's something I keep bumping into when working with the metrics crate, and I wanted to at least start a discussion about it. Is this something you've thought about at all? Is it a problem you're interested in addressing internally, or would you prefer users of the crate to handle it themselves?
Thanks for your time (and for the great package!)
The text was updated successfully, but these errors were encountered: