[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6559: Optimize group-key generator

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

Jackie-Jiang commented on a change in pull request #6559:
URL: https://github.com/apache/incubator-pinot/pull/6559#discussion_r573380233



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
##########
@@ -61,8 +61,16 @@
  * bounded by the number of groups limit (globalGroupIdUpperBound is always smaller or equal to numGroupsLimit).
  */
 public class DictionaryBasedGroupKeyGenerator implements GroupKeyGenerator {
-  private final static int INITIAL_MAP_SIZE = 256;
-  private final static int MAX_CACHING_MAP_SIZE = 1048576;
+  // NOTE: map size = map capacity (power of 2) * load factor
+  private static final int INITIAL_MAP_SIZE = (int) ((1 << 10) * 0.75f);
+  private static final int MAX_CACHING_MAP_SIZE = (int) ((1 << 20) * 0.75f);
+
+  private static final ThreadLocal<IntGroupIdMap> THREAD_LOCAL_INT_MAP = ThreadLocal.withInitial(IntGroupIdMap::new);
+  private static final ThreadLocal<Long2IntOpenHashMap> THREAD_LOCAL_LONG_MAP =

Review comment:
       Yes, but that is not the case for common queries. Also, for long2int map, using a single array will force us to store the value as long, which might consume more memory. This PR is focusing on the most common case of cardinality product between 10k to 2b.




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org