From 947534eb54a635179bb5aea8dfd6654819be9726 Mon Sep 17 00:00:00 2001 From: Liam Gray Date: Thu, 12 Dec 2024 23:22:23 +0000 Subject: [PATCH] perf: optimise CounterVec hashing, enable other hashers Signed-off-by: Liam Gray --- Cargo.toml | 1 + benches/counter.rs | 26 +++++++++++++++++++++++--- src/vec.rs | 23 ++++++++++++----------- 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a62b6296..d34b87e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ push = ["reqwest", "libc", "protobuf"] [dependencies] cfg-if = "^1.0" fnv = "^1.0" +nohash-hasher = "0.2.0" lazy_static = "^1.4" libc = { version = "^0.2", optional = true } parking_lot = "^0.12" diff --git a/benches/counter.rs b/benches/counter.rs index 2ad42c3a..9ebc6d8f 100644 --- a/benches/counter.rs +++ b/benches/counter.rs @@ -11,8 +11,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use criterion::{criterion_group, criterion_main, Criterion}; +use criterion::{black_box, criterion_group, criterion_main, Criterion}; use prometheus::{Counter, CounterVec, IntCounter, Opts}; +use fnv::FnvBuildHasher; use std::collections::HashMap; use std::sync::{atomic, Arc}; use std::thread; @@ -24,7 +25,7 @@ fn bench_counter_with_label_values(c: &mut Criterion) { ) .unwrap(); c.bench_function("counter_with_label_values", |b| { - b.iter(|| counter.with_label_values(&["eins", "zwei", "drei"]).inc()) + b.iter(|| counter.with_label_values(&black_box(["eins", "zwei", "drei"])).inc()) }); } @@ -41,7 +42,25 @@ fn bench_counter_with_mapped_labels(c: &mut Criterion) { labels.insert("two", "zwei"); labels.insert("one", "eins"); labels.insert("three", "drei"); - counter.with(&labels).inc(); + counter.with(&black_box(labels)).inc(); + }) + }); +} + +fn bench_counter_with_mapped_labels_fnv(c: &mut Criterion) { + let counter = CounterVec::new( + Opts::new("benchmark_counter", "A counter to benchmark it."), + &["one", "two", "three"], + ) + .unwrap(); + + c.bench_function("counter_with_mapped_labels_fnv", |b| { + b.iter(|| { + let mut labels = HashMap::with_capacity_and_hasher(3, FnvBuildHasher::default()); + labels.insert("two", "zwei"); + labels.insert("one", "eins"); + labels.insert("three", "drei"); + counter.with(&black_box(labels)).inc(); }) }); } @@ -192,6 +211,7 @@ criterion_group!( bench_counter_with_label_values, bench_counter_with_label_values_concurrent_write, bench_counter_with_mapped_labels, + bench_counter_with_mapped_labels_fnv, bench_counter_with_prepared_mapped_labels, bench_int_counter_no_labels, bench_int_counter_no_labels_concurrent_write, diff --git a/src/vec.rs b/src/vec.rs index 90508e93..c267ff6b 100644 --- a/src/vec.rs +++ b/src/vec.rs @@ -2,10 +2,10 @@ // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. use std::collections::HashMap; -use std::hash::Hasher; +use std::hash::{BuildHasher, Hasher}; use std::sync::Arc; - use fnv::FnvHasher; +use nohash_hasher::BuildNoHashHasher; use parking_lot::RwLock; use crate::desc::{Desc, Describer}; @@ -26,7 +26,8 @@ pub trait MetricVecBuilder: Send + Sync + Clone { #[derive(Debug)] pub(crate) struct MetricVecCore { - pub children: RwLock>, + // the key is pre-hashed, and so we use a no-hash hasher to avoid hashing again. + pub children: RwLock>>, pub desc: Desc, pub metric_type: MetricType, pub new_metric: T, @@ -59,7 +60,7 @@ impl MetricVecCore { self.get_or_create_metric(h, vals) } - pub fn get_metric_with(&self, labels: &HashMap<&str, &str>) -> Result { + pub fn get_metric_with(&self, labels: &HashMap<&str, &str, S>) -> Result { let h = self.hash_labels(labels)?; if let Some(metric) = self.children.read().get(&h).cloned() { @@ -81,7 +82,7 @@ impl MetricVecCore { Ok(()) } - pub fn delete(&self, labels: &HashMap<&str, &str>) -> Result<()> { + pub fn delete(&self, labels: &HashMap<&str, &str, S>) -> Result<()> { let h = self.hash_labels(labels)?; let mut children = self.children.write(); @@ -113,7 +114,7 @@ impl MetricVecCore { Ok(h.finish()) } - fn hash_labels(&self, labels: &HashMap<&str, &str>) -> Result { + fn hash_labels(&self, labels: &HashMap<&str, &str, S>) -> Result { if labels.len() != self.desc.variable_labels.len() { return Err(Error::InconsistentCardinality { expect: self.desc.variable_labels.len(), @@ -137,7 +138,7 @@ impl MetricVecCore { Ok(h.finish()) } - fn get_label_values<'a>(&self, labels: &'a HashMap<&str, &str>) -> Result> { + fn get_label_values<'a, S: BuildHasher>(&self, labels: &'a HashMap<&str, &str, S>) -> Result> { let mut values = Vec::new(); for name in &self.desc.variable_labels { match labels.get(&name.as_ref()) { @@ -188,7 +189,7 @@ impl MetricVec { pub fn create(metric_type: MetricType, new_metric: T, opts: T::P) -> Result> { let desc = opts.describe()?; let v = MetricVecCore { - children: RwLock::new(HashMap::new()), + children: RwLock::new(HashMap::default()), desc, metric_type, new_metric, @@ -237,7 +238,7 @@ impl MetricVec { /// This method is used for the same purpose as /// `get_metric_with_label_values`. See there for pros and cons of the two /// methods. - pub fn get_metric_with(&self, labels: &HashMap<&str, &str>) -> Result { + pub fn get_metric_with(&self, labels: &HashMap<&str, &str, S>) -> Result { self.v.get_metric_with(labels) } @@ -261,7 +262,7 @@ impl MetricVec { /// `with` works as `get_metric_with`, but panics if an error occurs. The method allows /// neat syntax like: /// httpReqs.with(Labels{"status":"404", "method":"POST"}).inc() - pub fn with(&self, labels: &HashMap<&str, &str>) -> T::M { + pub fn with(&self, labels: &HashMap<&str, &str, S>) -> T::M { self.get_metric_with(labels).unwrap() } @@ -289,7 +290,7 @@ impl MetricVec { /// /// This method is used for the same purpose as `delete_label_values`. See /// there for pros and cons of the two methods. - pub fn remove(&self, labels: &HashMap<&str, &str>) -> Result<()> { + pub fn remove(&self, labels: &HashMap<&str, &str, S>) -> Result<()> { self.v.delete(labels) }