Optimizations for LexicographicTopNs #1603
Conversation
| @Override | ||
| public int compare(DimValHolder o1, DimValHolder o2) | ||
| { | ||
| return comparator.compare(o1.getDimName(), o2.getDimName()); |
There was a problem hiding this comment.
Do we need to account for null here?
There was a problem hiding this comment.
nvmnd, is done in comparator
|
Squash and I'm 👍 |
nishantmonu51
force-pushed
the
optimize-lexicographic-topN
branch
from
184ee73
to
ceecbbc
Compare
| } | ||
|
|
||
| private boolean shouldAdd(String dimName) |
There was a problem hiding this comment.
Is it possible to avoid copy pasting the code and reuse the code from the other topnresultbuilders?
There was a problem hiding this comment.
they look similar but differ in actual implementation.
| final boolean belowThreshold = pQueue.size() < this.threshold; | ||
| final boolean belowMax = belowThreshold | ||
| || this.comparator.compare(pQueue.peek().getTopNMetricVal(), dimName) < 0; | ||
| return belowMax && comparator.compare(dimName, previousStop) > 0; |
There was a problem hiding this comment.
the implementation in TopNNumericResultBuilder doesn't seem to have the added && comparator.compare(dimName, previousStop) > 0. Do you mind adding a comment to explain why it's needed?
nishantmonu51
force-pushed
the
optimize-lexicographic-topN
branch
from
ceecbbc
to
a3619e6
Compare
| final boolean belowMax = belowThreshold | ||
| || this.comparator.compare(pQueue.peek().getTopNMetricVal(), dimName) < 0; | ||
| // Only add if dimName is after previousStop | ||
| return belowMax && comparator.compare(dimName, previousStop) > 0; |
There was a problem hiding this comment.
is it possible to optimize the case where previousStop is null and shortcut the comparison altogether? Seems expensive to call comparator.compare every time?
initial review for perf optimizations for lexicographic TopNs fix compilation create map with proper size review comment review comment review comments
nishantmonu51
force-pushed
the
optimize-lexicographic-topN
branch
from
a3619e6
to
b8d8a8d
Compare
Optimizations for LexicographicTopNs
-- change storage structure to PriorityQueue
-- Only add to pQueue when required
-- Add Loop unrolling for metrics addition
-- Re-orders Queue on build() call.