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

Reply via email to