ankitsultana commented on code in PR #11112:
URL: https://github.com/apache/pinot/pull/11112#discussion_r1264014292
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/HashJoinOperator.java:
##########
@@ -169,9 +169,14 @@ private void buildBroadcastHashTable() {
}
List<Object[]> container = rightBlock.getContainer();
// put all the rows into corresponding hash collections keyed by the key
selector function.
+ int initialHeuristicSize = 16;
for (Object[] row : container) {
- List<Object[]> hashCollection =
- _broadcastRightTable.computeIfAbsent(new
Key(_rightKeySelector.getKey(row)), k -> new ArrayList<>());
+ ArrayList<Object[]> hashCollection =
+ _broadcastRightTable.computeIfAbsent(new
Key(_rightKeySelector.getKey(row)), k -> new ArrayList<>(16));
Review Comment:
Running `new Key()` would create a new object in each iteration. If this
loop is run millions of times per second, and assuming an object reference is
at least 8 bytes, then this could add MBs of data on the TLAB which would
likely lead to slow allocations, and also these would be unnecessary
short-lived objects.
Do you think we can change the FieldSelector to return a `Key` directly?
(edit: this won't likely work either based on how the code is laid out.. looks
inevitable)
~~Or we could re-use the `Key` object and add a setter to it, so that we
reuse the reference and only update the inner reference with a new reference.~~
(my bad this won't work obviously)
--
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]
