siddharthteotia commented on a change in pull request #6559:
URL: https://github.com/apache/incubator-pinot/pull/6559#discussion_r573484761
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
##########
@@ -108,35 +116,35 @@ public DictionaryBasedGroupKeyGenerator(TransformOperator
transformOperator, Exp
_isSingleValueColumn[i] =
transformOperator.getResultMetadata(groupByExpression).isSingleValue();
}
+ // TODO: Clear the holder after processing the query instead of before
if (longOverflow) {
+ // ArrayMapBasedHolder
_globalGroupIdUpperBound = numGroupsLimit;
- Object mapInternal =
mapBasedRawKeyHolders.computeIfAbsent(ArrayMapBasedHolder.class.getName(),
- o -> new ArrayMapBasedHolder(INITIAL_MAP_SIZE).getInternal());
- _rawKeyHolder = new ArrayMapBasedHolder(mapInternal);
- if (((Object2IntOpenHashMap) mapInternal).size() > MAX_CACHING_MAP_SIZE)
{
- mapBasedRawKeyHolders
- .put(ArrayMapBasedHolder.class.getName(), new
ArrayMapBasedHolder(INITIAL_MAP_SIZE).getInternal());
+ Object2IntOpenHashMap<IntArray> groupIdMap =
THREAD_LOCAL_INT_ARRAY_MAP.get();
+ int size = groupIdMap.size();
+ groupIdMap.clear();
+ if (size > MAX_CACHING_MAP_SIZE) {
+ groupIdMap.trim();
}
+ _rawKeyHolder = new ArrayMapBasedHolder(groupIdMap);
} else {
if (cardinalityProduct > Integer.MAX_VALUE) {
+ // LongMapBasedHolder
_globalGroupIdUpperBound = numGroupsLimit;
- Object mapInternal =
mapBasedRawKeyHolders.computeIfAbsent(LongMapBasedHolder.class.getName(),
- o -> new LongMapBasedHolder(INITIAL_MAP_SIZE).getInternal());
- _rawKeyHolder = new LongMapBasedHolder(mapInternal);
- if (((Long2IntOpenHashMap) mapInternal).size() > MAX_CACHING_MAP_SIZE)
{
- mapBasedRawKeyHolders
- .put(ArrayMapBasedHolder.class.getName(), new
ArrayMapBasedHolder(INITIAL_MAP_SIZE).getInternal());
+ Long2IntOpenHashMap groupIdMap = THREAD_LOCAL_LONG_MAP.get();
+ int size = groupIdMap.size();
+ groupIdMap.clear();
+ if (size > MAX_CACHING_MAP_SIZE) {
+ groupIdMap.trim();
}
+ _rawKeyHolder = new LongMapBasedHolder(groupIdMap);
} else {
_globalGroupIdUpperBound = Math.min((int) cardinalityProduct,
numGroupsLimit);
if (cardinalityProduct > arrayBasedThreshold) {
- Object mapInternal =
mapBasedRawKeyHolders.computeIfAbsent(IntMapBasedHolder.class.getName(),
- o -> new IntMapBasedHolder(INITIAL_MAP_SIZE).getInternal());
- _rawKeyHolder = new IntMapBasedHolder(mapInternal);
- if (((Int2IntOpenHashMap) mapInternal).size() >
MAX_CACHING_MAP_SIZE) {
- mapBasedRawKeyHolders
- .put(ArrayMapBasedHolder.class.getName(), new
ArrayMapBasedHolder(INITIAL_MAP_SIZE).getInternal());
- }
+ // IntMapBasedHolder
+ IntGroupIdMap groupIdMap = THREAD_LOCAL_INT_MAP.get();
+ groupIdMap.clear();
Review comment:
Thanks. We also discussed this offline that trim() here is essentially
truncation. I confused it with sort based trimming that we do in combine
operator for ordering
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
