Reduce memory footprint of StepBucketHistogram #4955
Conversation
| @@ -73,28 +93,49 @@ else if (value > key) | |||
| return low < buckets.length ? low : -1; | |||
| } | |||
|
|
|||
| Iterator<CountAtBucket> countsAtValues(Iterator<Double> values) { | |||
| Iterator<CountAtBucket> countsAtValues(Iterator<Double> buckets) { | |||
There was a problem hiding this comment.
Can we just use getCountsAtBucket() instead of this? This makes a call to countAtBucket() for every bucket which in turn does a binary search to find the bucket. It makes sense if we need to get the counts for only a subset of buckets, but we don't do that anywhere (why would we even do that?). But at the end, all we need is a continuous list of buckets and their count which we can get by iterating on the values directly.
There were a few issues solved around this area hence not changing anything at this point. But,
my claim would be the buckets would not change after the histogram is created so we should always rely on the buckets from the underlying implementation (in this case FixedBucketHistogram).
Partially resolves #4954
I think we can also apply this safely to TimeWindowFixedBoundaryHistogram.