Jackie-Jiang commented on code in PR #19800:
URL: https://github.com/apache/pinot/pull/19800#discussion_r4251199781
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/SetMergeStrategy.java:
##########
@@ -41,11 +42,13 @@ public List<Set> merge(List<Set> intermediateResult1,
List<Set> intermediateResu
@Override
public LongArrayList extractFinalResult(List<Set> stepsSets) {
long[] result = new long[_numSteps];
- result[0] = stepsSets.get(0).size();
+ Set<?> firstStep = stepsSets.get(0);
+ Set<?> intersection = new HashSet<>(firstStep);
Review Comment:
Non-blocking: could we avoid copying the entire first-step population before
an intersection is needed? A supported single-step set funnel previously
finalized with one `size()` call; `new HashSet<>(firstStep)` now adds O(N) work
and N hash entries, plus boxing when the source is a primitive fastutil set.
Similarly, a two-step funnel with a million entrants and an empty second step
now copies and removes all million entries, whereas the previous extraction did
no member traversal. This also affects SSE and queries without group trimming.
We can preserve the intermediate state while reading the first count
directly, skipping copying for single-step funnels, and constructing subsequent
intersections from the smaller operand. Keeping primitive containers where
practical would also avoid boxing.
--
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]
