ref(metrics): Use a priority queue for metrics aggregator

[Dav1dde]

Uses a priority queue for the metrics aggregator:

• improves worst case performance massively (huge aggregator, small flushes)
• makes the performance for fast flushes (small aggregators, all flushed) slightly worse

bench_flush_metrics/same metric on different projects
                        time:   [18.261 µs 21.705 µs 25.368 µs]
                        change: [+97.208% +144.62% +196.81%] (p = 0.00 < 0.05)
                        Performance has regressed.

bench_flush_metrics/a lot of metrics from a lot of projects with mix of backdated buckets and non
                        time:   [21.889 ms 22.119 ms 22.365 ms]
                        change: [-67.986% -67.566% -67.127%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)

#skip-changelog

[jjbayer]

Interesting crate, might actually be a good replacement for my homegrown priority map. But do we need it for the aggregator use case? IIUC the std binary heap should be good enough?

[Dav1dde]

We still need keyed access and priority separately. Keyed for the aggregation key, priority for the flush time.

[Dav1dde]

Looked into not using a dependency for this but instead having a HashMap with a secondary BinaryHeap, performance improved but was still twice as slow as the priority_queue implementation.

I'd just go with the dependency and revisit this again if performance is still not good enough or when we're looking into priority queue alternatives for the spooler.

[jjbayer]

See question about change_priority_by vs get_mut.

[jjbayer]

This is awkward, right? For our own priority map I'm considering writing an interface that lets you do let value = queue.peek().remove() instead of queue.pop().

[Dav1dde]

Yeah it is, I proposed an API here.

What could also be nice is also something like in heed where you have extra functionality on the iterator.

[jjbayer]

Why do we not use self.buckets.get_mut() here? IIUC, the closure is only called if the element exists, and we don't actually update any priorities.

[Dav1dde]

get_mut only returns the key as mutable, but we need a mutable value which also causes this unfortunate error handling.

It's kinda double unfortunate API wise, because we never change the priority, maybe worth investigating wrapping the value itself in a RefCell then we can use get() although I am not sure if that makes it better ...