metrics icon indicating copy to clipboard operation
metrics copied to clipboard

Label Allocation Optimization

Open jwilm opened this issue 1 year ago • 5 comments

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!)

jwilm avatar Oct 02 '23 18:10 jwilm

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 (Arc<T>) versions of the given type, as well as an optimization for small strings that could otherwise be inlined into the copy-on-write structure.

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.

tobz avatar Oct 03 '23 00:10 tobz

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?

jwilm avatar Oct 04 '23 17:10 jwilm

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.

tobz avatar Oct 04 '23 17:10 tobz

Related to https://github.com/metrics-rs/metrics/issues/273

Another problem with dynamic labels is Box::new:

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 tracing crate does it.

loyd avatar Oct 26 '23 11:10 loyd

Taking the approach that tracing takes would involve a lot more work, because it would completely change how we work with Key... but this is very different from basic problem above of not supporting Arc<str>.

I opened #405 as a placeholder to write down some thoughts on what your idea might look like.

tobz avatar Oct 29 '23 15:10 tobz