Skip to content

Commit

Permalink
Fix #124 Record iteration when recording only zeros (#125)
Browse files Browse the repository at this point in the history
As pointed in the comment in the corresponding issues, we want to ensure
we trigger the picker logic at least once to keep the invariant of the
loop, that is to say that the picking logic has been executed at least
once for last_picked_index, which is not the case for zero when there
are only zeros.

To do so we use and Option, and initialize to None, to make sure the
pick logic is ran at least once.

I added a test that collect the record and check for their count.

Fixes #124
  • Loading branch information
Carreau authored Nov 11, 2023
1 parent bdfad24 commit 6de1b9b
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
8 changes: 4 additions & 4 deletions src/iterators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pub struct HistogramIterator<'a, T: 'a + Counter, P: PickyIterator<T>> {
count_since_last_iteration: u64,
count_at_index: T,
current_index: usize,
last_picked_index: usize,
last_picked_index: Option<usize>,
max_value_index: usize,
fresh: bool,
ended: bool,
Expand Down Expand Up @@ -152,7 +152,7 @@ impl<'a, T: Counter, P: PickyIterator<T>> HistogramIterator<'a, T, P> {
count_since_last_iteration: 0,
count_at_index: T::zero(),
current_index: 0,
last_picked_index: 0,
last_picked_index: None,
max_value_index: h.index_for(h.max()).expect("Either 0 or an existing index"),
picker,
fresh: true,
Expand Down Expand Up @@ -186,7 +186,7 @@ where
}

// Have we already picked the index with the last non-zero count in the histogram?
if self.last_picked_index >= self.max_value_index {
if self.last_picked_index >= Some(self.max_value_index) {
// is the picker done?
if !self.picker.more(self.current_index) {
self.ended = true;
Expand Down Expand Up @@ -243,7 +243,7 @@ where
// step multiple times without advancing the index.

self.count_since_last_iteration = 0;
self.last_picked_index = self.current_index;
self.last_picked_index = Some(self.current_index);
return Some(val);
}

Expand Down
7 changes: 7 additions & 0 deletions tests/histogram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,3 +560,10 @@ fn subtract_underflow_guarded_by_per_value_count_check() {
h.subtract(h2).unwrap_err()
);
}

#[test]
fn recorded_only_zeros() {
let mut h = Histogram::<u64>::new(1).unwrap();
h += 0;
assert_eq!(h.iter_recorded().count(), 1);
}

0 comments on commit 6de1b9b

Please sign in to comment.