[GitHub] [lucene] mdmarshmallow commented on a change in pull request #304: LUCENE 10080 Added FixedBitSet for one counts when counting taxonomy facet labels

[GitBox <gi...@apache.org>]

mdmarshmallow commented on a change in pull request #304:
URL: https://github.com/apache/lucene/pull/304#discussion_r718921856



##########
File path: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java
##########
@@ -253,4 +257,92 @@ public FacetResult getTopChildren(int topN, String dim, String... path) throws I
 
     return new FacetResult(dim, path, totValue, labelValues, childCount);
   }
+
+  /**
+   * Class that uses FixedBitSet to store counts for all ordinals with 1 count and IntIntHashMap for
+   * all other counts
+   */
+  protected static class IntIntHashMapWithFixedBitSet implements Iterable<IntIntCursor> {

Review comment:
       I checked and there are several classes that this change could apply to, with varying amounts of additional work:
   * `IntTaxonomyFacets`
   * `StringValueFacetCounts`
   * `FloatTaxonomyFacets`: Currently this only uses a dense array to count facets
   * `LongValueTaxonomyFacets`: Would need to create a class like `LongIntHashMapWithFixedBitSet` to work with this
   * `SortedSetDocValuesCount`: Same as `FloatTaxonomyFacets`, it only uses dense arrays to count facets
   * `ConcurrentSortedSetDocValueFacetCounts`: Currently uses a dense atomic array, will need to make a threadsafe version of `IntIntHashMapWithFixedBitSet` to use




-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org