Skip to content

Commit

Permalink
Fix Record iteration when recording only zeros
Browse files Browse the repository at this point in the history
Fixes #124

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 for
last_picked_index, which is not the case for zero when there are only
zeros.

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

Would it be better to check for the following:

```rust
assert_eq!(h.iter_recorded().collect::<Vec<_>>(), vec![IterationValue::<u64>::new(0, 1.0, 1.0, 1, 1)]);
```

It is easy to assume that `h.iter_recorded().collect::<Vec<_>>().len() == h.len()`
and simplify the added test, and I'm unsure how to convey that we
_really_ want to collect and check the length of all the record.
  • Loading branch information
Carreau committed Aug 19, 2023
1 parent d56b823 commit 70b09c2
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/iterators/mod.rs
Original file line number Diff line number Diff line change
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 >= self.max_value_index && self.last_picked_index != 0 {
// is the picker done?
if !self.picker.more(self.current_index) {
self.ended = true;
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().collect::<Vec<_>>().len(), 1);
}

0 comments on commit 70b09c2

Please sign in to comment.