Jackie-Jiang commented on a change in pull request #7777:
URL: https://github.com/apache/pinot/pull/7777#discussion_r751725837
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
##########
@@ -612,7 +656,8 @@ public void processMultiValue(int numDocs, int[][]
outGroupIds) {
private int getGroupId(long rawKey) {
int numGroups = _groupIdMap.size();
if (numGroups < _globalGroupIdUpperBound) {
- return _groupIdMap.computeIfAbsent(rawKey, k -> numGroups);
+ int id = _groupIdMap.putIfAbsent(rawKey, numGroups);
+ return id == _groupIdMap.defaultReturnValue() ? numGroups : id;
Review comment:
Might be slightly faster to do
```suggestion
return id == INVALID_ID ? numGroups : id;
```
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
##########
@@ -587,6 +617,20 @@ public LongMapBasedHolder(Long2IntOpenHashMap groupIdMap) {
@Override
public void processSingleValue(int numDocs, int[] outGroupIds) {
+ if (_numGroupByExpressions == 1) {
Review comment:
We should never get into `LongMapBasedHolder` for single group-by
expression
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
##########
@@ -253,12 +252,29 @@ public int getNumKeys() {
}
private class ArrayBasedHolder implements RawKeyHolder {
- // TODO: using bitmap might better
Review comment:
Should we consider using `BitSet` to replace this boolean array?
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
