Skip to content

Commit

Permalink
remove snapshot from histogram (#108)
Browse files Browse the repository at this point in the history
We can simplify the histogram crate by removing the snapshot
concept. This change removes the type and related functions.
Equivalent functionality is exposed via clone/load depending on if
the histogram is vanilla or atomic.

Additionally, we add a few more conversions and constructors to
improve symmetry and flexibility.
  • Loading branch information
brayniac authored Apr 1, 2024
1 parent c39111b commit 59d7aa6
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 271 deletions.
2 changes: 1 addition & 1 deletion histogram/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "histogram"
version = "0.9.1"
version = "0.10.0"
edition = "2021"
authors = ["Brian Martin <brian@pelikan.io>"]
license = "MIT OR Apache-2.0"
Expand Down
66 changes: 18 additions & 48 deletions histogram/src/atomic.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::{Config, Error, Histogram, Snapshot};
use crate::{Config, Error, Histogram};
use core::sync::atomic::{AtomicU64, Ordering};
use std::time::SystemTime;

/// A histogram that uses atomic 64bit counters for each bucket.
///
Expand All @@ -9,7 +8,6 @@ use std::time::SystemTime;
/// the histogram at a point in time.
pub struct AtomicHistogram {
config: Config,
start: SystemTime,
buckets: Box<[AtomicU64]>,
}

Expand All @@ -29,7 +27,6 @@ impl AtomicHistogram {

Self {
config: *config,
start: SystemTime::now(),
buckets: buckets.into(),
}
}
Expand All @@ -54,23 +51,18 @@ impl AtomicHistogram {
Ok(())
}

/// Produce a snapshot from this histogram.
pub fn snapshot(&self) -> Snapshot {
let end = SystemTime::now();

/// Read the bucket values into a new `Histogram`
pub fn load(&self) -> Histogram {
let buckets: Vec<u64> = self
.buckets
.iter()
.map(|bucket| bucket.load(Ordering::Relaxed))
.collect();

let histogram = Histogram {
Histogram {
config: self.config,
start: self.start,
buckets: buckets.into(),
};

Snapshot { end, histogram }
}
}
}

Expand All @@ -80,11 +72,7 @@ mod test {

#[test]
fn size() {
#[cfg(not(target_os = "windows"))]
assert_eq!(std::mem::size_of::<AtomicHistogram>(), 64);

#[cfg(target_os = "windows")]
assert_eq!(std::mem::size_of::<AtomicHistogram>(), 56);
assert_eq!(std::mem::size_of::<AtomicHistogram>(), 48);
}

#[test]
Expand All @@ -94,56 +82,38 @@ mod test {
for i in 0..=100 {
let _ = histogram.increment(i);
assert_eq!(
histogram.snapshot().percentile(0.0),
histogram.load().percentile(0.0),
Ok(Bucket {
count: 1,
range: 0..=0,
})
);
assert_eq!(
histogram.snapshot().percentile(100.0),
histogram.load().percentile(100.0),
Ok(Bucket {
count: 1,
range: i..=i,
})
);
}
assert_eq!(
histogram.snapshot().percentile(25.0).map(|b| b.end()),
Ok(25)
);
assert_eq!(
histogram.snapshot().percentile(50.0).map(|b| b.end()),
Ok(50)
);
assert_eq!(
histogram.snapshot().percentile(75.0).map(|b| b.end()),
Ok(75)
);
assert_eq!(
histogram.snapshot().percentile(90.0).map(|b| b.end()),
Ok(90)
);
assert_eq!(
histogram.snapshot().percentile(99.0).map(|b| b.end()),
Ok(99)
);
assert_eq!(
histogram.snapshot().percentile(99.9).map(|b| b.end()),
Ok(100)
);
assert_eq!(histogram.load().percentile(25.0).map(|b| b.end()), Ok(25));
assert_eq!(histogram.load().percentile(50.0).map(|b| b.end()), Ok(50));
assert_eq!(histogram.load().percentile(75.0).map(|b| b.end()), Ok(75));
assert_eq!(histogram.load().percentile(90.0).map(|b| b.end()), Ok(90));
assert_eq!(histogram.load().percentile(99.0).map(|b| b.end()), Ok(99));
assert_eq!(histogram.load().percentile(99.9).map(|b| b.end()), Ok(100));

assert_eq!(
histogram.snapshot().percentile(-1.0),
histogram.load().percentile(-1.0),
Err(Error::InvalidPercentile)
);
assert_eq!(
histogram.snapshot().percentile(101.0),
histogram.load().percentile(101.0),
Err(Error::InvalidPercentile)
);

let percentiles: Vec<(f64, u64)> = histogram
.snapshot()
.load()
.percentiles(&[50.0, 90.0, 99.0, 99.9])
.unwrap()
.iter()
Expand All @@ -157,7 +127,7 @@ mod test {

let _ = histogram.increment(1024);
assert_eq!(
histogram.snapshot().percentile(99.9),
histogram.load().percentile(99.9),
Ok(Bucket {
count: 1,
range: 1024..=1031,
Expand Down
2 changes: 1 addition & 1 deletion histogram/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ use serde::{Deserialize, Serialize};
/// # Constraints:
/// * `max_value_power` must be in the range `0..=64`
/// * `max_value_power` must be greater than `grouping_power
#[derive(Clone, Copy, Debug, PartialEq)]
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct Config {
Expand Down
2 changes: 0 additions & 2 deletions histogram/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,12 @@ mod atomic;
mod bucket;
mod config;
mod errors;
mod snapshot;
mod sparse;
mod standard;

pub use atomic::AtomicHistogram;
pub use bucket::Bucket;
pub use config::Config;
pub use errors::Error;
pub use snapshot::Snapshot;
pub use sparse::SparseHistogram;
pub use standard::Histogram;
189 changes: 0 additions & 189 deletions histogram/src/snapshot.rs

This file was deleted.

Loading

0 comments on commit 59d7aa6

Please sign in to comment.