jpountz commented on code in PR #14421:
URL: https://github.com/apache/lucene/pull/14421#discussion_r2019755410
##########
lucene/sandbox/src/java/org/apache/lucene/sandbox/facet/plain/histograms/HistogramCollector.java:
##########
@@ -279,7 +279,9 @@ public void collect(DocIdStream stream) throws IOException {
public void finish() throws IOException {
// Put counts that we computed in the int[] back into the hash map.
for (int i = 0; i < counts.length; ++i) {
- collectorCounts.addTo(leafMinBucket + i, counts[i]);
+ if (counts[i] != 0) {
+ collectorCounts.addTo(leafMinBucket + i, counts[i]);
+ }
}
checkMaxBuckets(collectorCounts.size(), maxBuckets);
Review Comment:
I think it's ok the way it is. The end goal is to prevent unbounded heap
allocation. In this case, the amount of excess heap we may allocate is bounded
by 1024 entries, so I'd err on the side of simplicity by not checking the
number of buckets in the loop?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
