These two crates must be upgraded together since metrics-exporter-prometheus exposes types from metrics in its public API that we use.
However, the new version of metrics has improved its diagnostics such that unused diagnostics are shown as compiler warnings.
We have 2 such instances of these warnings.
Since CI will prevent merging with warnings, we need to either ignore the warnings or fix them.
I can see that our usages that are getting warned do have an associated performance cost that we should try to avoid.
Add the bench to shotover/benches/benches/chain.rs
Copy redis_filter or loopback as a base for the new bench.
The bench should have a chain consisting of QueryCounter followed by Loopback. This is because QueryCounter ignores the responses and Loopback is the fastest transform for when we dont care about the value of responses.
The bench can send in redis requests as they are simplest and fastest to work with.
leaking memory sounds bad but its actually fine since we only create this value once for the lifetime of shotover. And leaking the memory gives us 2 advantages:
we never have to pay the cost of freeing its allocation when dropping it
we can pass it around as a reference without having to worry about lifetimes, this reduces cloning.
When we recreate the counter every time, its doing something similar to a hashmap lookup in the background but I believe the way it does it will be slower since it has more fields to hash.
These two crates must be upgraded together since metrics-exporter-prometheus exposes types from metrics in its public API that we use.
However, the new version of metrics has improved its diagnostics such that unused diagnostics are shown as compiler warnings. We have 2 such instances of these warnings.
Since CI will prevent merging with warnings, we need to either ignore the warnings or fix them. I can see that our usages that are getting warned do have an associated performance cost that we should try to avoid.
&'static str- this will definitely be faster.Counterwe create in aHashMap<String, Counter>this should be faster, we should measure it with a microbenchmark though.But also I think there are legitimate uses of creating a metric without using it so I've made a comment to see what upstream thinks: https://github.com/metrics-rs/metrics/pull/475#issuecomment-2144361030
TODO, all in separate PRs:
shotover/benches/benches/chain.rsredis_filterorloopbackas a base for the new bench.QueryCounterfollowed byLoopback. This is because QueryCounter ignores the responses and Loopback is the fastest transform for when we dont care about the value of responses.String::leak()the counter_name so we store it as&'static str- this will definitely be faster, run the benchmark to double check. https://github.com/shotover/shotover-proxy/pull/1669Counterwe create in aHashMap<String, Counter>this should be faster, run the benchmark to see if it is. https://github.com/shotover/shotover-proxy/pull/1671