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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org