Skip to content

Commit

Permalink
perf: optimise CounterVec hashing, enable other hashers
Browse files Browse the repository at this point in the history
  • Loading branch information
hoxxep committed Dec 12, 2024
1 parent f0c9bc2 commit 5b38dd1
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 14 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
26 changes: 23 additions & 3 deletions benches/counter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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())
});
}

Expand All @@ -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();
})
});
}
Expand Down Expand Up @@ -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,
Expand Down
23 changes: 12 additions & 11 deletions src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -26,7 +26,8 @@ pub trait MetricVecBuilder: Send + Sync + Clone {

#[derive(Debug)]
pub(crate) struct MetricVecCore<T: MetricVecBuilder> {
pub children: RwLock<HashMap<u64, T::M>>,
// the key is pre-hashed, and so we use a no-hash hasher to avoid hashing again.
pub children: RwLock<HashMap<u64, T::M, BuildNoHashHasher<u64>>>,
pub desc: Desc,
pub metric_type: MetricType,
pub new_metric: T,
Expand Down Expand Up @@ -59,7 +60,7 @@ impl<T: MetricVecBuilder> MetricVecCore<T> {
self.get_or_create_metric(h, vals)
}

pub fn get_metric_with(&self, labels: &HashMap<&str, &str>) -> Result<T::M> {
pub fn get_metric_with<S: BuildHasher>(&self, labels: &HashMap<&str, &str, S>) -> Result<T::M> {
let h = self.hash_labels(labels)?;

if let Some(metric) = self.children.read().get(&h).cloned() {
Expand All @@ -81,7 +82,7 @@ impl<T: MetricVecBuilder> MetricVecCore<T> {
Ok(())
}

pub fn delete(&self, labels: &HashMap<&str, &str>) -> Result<()> {
pub fn delete<S: BuildHasher>(&self, labels: &HashMap<&str, &str, S>) -> Result<()> {
let h = self.hash_labels(labels)?;

let mut children = self.children.write();
Expand Down Expand Up @@ -113,7 +114,7 @@ impl<T: MetricVecBuilder> MetricVecCore<T> {
Ok(h.finish())
}

fn hash_labels(&self, labels: &HashMap<&str, &str>) -> Result<u64> {
fn hash_labels<S: BuildHasher>(&self, labels: &HashMap<&str, &str, S>) -> Result<u64> {
if labels.len() != self.desc.variable_labels.len() {
return Err(Error::InconsistentCardinality {
expect: self.desc.variable_labels.len(),
Expand All @@ -137,7 +138,7 @@ impl<T: MetricVecBuilder> MetricVecCore<T> {
Ok(h.finish())
}

fn get_label_values<'a>(&self, labels: &'a HashMap<&str, &str>) -> Result<Vec<&'a str>> {
fn get_label_values<'a, S: BuildHasher>(&self, labels: &'a HashMap<&str, &str, S>) -> Result<Vec<&'a str>> {
let mut values = Vec::new();
for name in &self.desc.variable_labels {
match labels.get(&name.as_ref()) {
Expand Down Expand Up @@ -188,7 +189,7 @@ impl<T: MetricVecBuilder> MetricVec<T> {
pub fn create(metric_type: MetricType, new_metric: T, opts: T::P) -> Result<MetricVec<T>> {
let desc = opts.describe()?;
let v = MetricVecCore {
children: RwLock::new(HashMap::new()),
children: RwLock::new(HashMap::default()),
desc,
metric_type,
new_metric,
Expand Down Expand Up @@ -237,7 +238,7 @@ impl<T: MetricVecBuilder> MetricVec<T> {
/// 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<T::M> {
pub fn get_metric_with<S: BuildHasher>(&self, labels: &HashMap<&str, &str, S>) -> Result<T::M> {
self.v.get_metric_with(labels)
}

Expand All @@ -261,7 +262,7 @@ impl<T: MetricVecBuilder> MetricVec<T> {
/// `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<S: BuildHasher>(&self, labels: &HashMap<&str, &str, S>) -> T::M {
self.get_metric_with(labels).unwrap()
}

Expand Down Expand Up @@ -289,7 +290,7 @@ impl<T: MetricVecBuilder> MetricVec<T> {
///
/// 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<S: BuildHasher>(&self, labels: &HashMap<&str, &str, S>) -> Result<()> {
self.v.delete(labels)
}

Expand Down

0 comments on commit 5b38dd1

Please sign in to comment.