richardstartin commented on a change in pull request #7630:
URL: https://github.com/apache/pinot/pull/7630#discussion_r735750286



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java
##########
@@ -326,6 +375,54 @@ public Long extractFinalResult(HyperLogLog 
intermediateResult) {
     return intermediateResult.cardinality();
   }
 
+  /**
+   * Returns the dictionary id bitmap from the result holder or creates a new 
one if it does not exist.
+   */
+  protected static RoaringBitmap getDictIdBitmap(AggregationResultHolder 
aggregationResultHolder,
+      Dictionary dictionary) {
+    DistinctCountHLLAggregationFunction.DictIdsWrapper dictIdsWrapper = 
aggregationResultHolder.getResult();
+    if (dictIdsWrapper == null) {
+      dictIdsWrapper = new 
DistinctCountHLLAggregationFunction.DictIdsWrapper(dictionary);
+      aggregationResultHolder.setValue(dictIdsWrapper);
+    }
+    return dictIdsWrapper._dictIdBitmap;
+  }
+
+  /**
+   * Returns the dictionary id bitmap for the given group key or creates a new 
one if it does not exist.
+   */
+  protected static RoaringBitmap getDictIdBitmap(GroupByResultHolder 
groupByResultHolder, int groupKey,
+      Dictionary dictionary) {
+    DistinctCountHLLAggregationFunction.DictIdsWrapper dictIdsWrapper = 
groupByResultHolder.getResult(groupKey);
+    if (dictIdsWrapper == null) {
+      dictIdsWrapper = new 
DistinctCountHLLAggregationFunction.DictIdsWrapper(dictionary);
+      groupByResultHolder.setValueForKey(groupKey, dictIdsWrapper);
+    }
+    return dictIdsWrapper._dictIdBitmap;
+  }
+
+  /**
+   * Helper method to set dictionary id for the given group keys into the 
result holder.
+   */
+  private static void setDictIdForGroupKeys(GroupByResultHolder 
groupByResultHolder, int[] groupKeys,
+      Dictionary dictionary, int dictId) {
+    for (int groupKey : groupKeys) {
+      getDictIdBitmap(groupByResultHolder, groupKey, dictionary).add(dictId);
+    }
+  }
+
+  private HyperLogLog convertToHLL(DictIdsWrapper dictIdsWrapper) {
+    Dictionary dictionary = dictIdsWrapper._dictionary;
+    RoaringBitmap dictIdBitmap = dictIdsWrapper._dictIdBitmap;
+    int numValues = dictIdBitmap.getCardinality();
+    PeekableIntIterator iterator = dictIdBitmap.getIntIterator();
+    HyperLogLog hyperLogLog = new HyperLogLog(_log2m);
+    while (iterator.hasNext()) {
+      hyperLogLog.offer(iterator.next());
+    }

Review comment:
       Here's the result from a benchmark in the library, it's well worth doing:
   
   ```
   Benchmark                                    Mode  Cnt        Score        
Error  Units
   IteratorsBenchmark32.testInverted_a          avgt    5      366.534 ±      
8.021  us/op
   IteratorsBenchmark32.testInverted_b          avgt    5     6910.122 ±    
254.660  us/op
   IteratorsBenchmark32.testInverted_c          avgt    5   247977.832 ±  
97483.117  us/op
   IteratorsBenchmark32.testStandard_a          avgt    5      556.616 ±     
19.426  us/op
   IteratorsBenchmark32.testStandard_b          avgt    5    92115.322 ±   
3111.469  us/op
   IteratorsBenchmark32.testStandard_c          avgt    5  1123165.972 ± 
121797.135  us/op
   ```




-- 
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]

Reply via email to