-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SuperSorter: direct merging, increased parallelism. #16775
Conversation
Two performance enhancements: 1) Direct merging of input frames to output channels, without any temporary files, if all input frames fit in memory. 2) When doing multi-level merging (now called "external mode"), improve parallelism by boosting up the number of mergers in the penultimate level. To support direct merging, FrameChannelMerger is enhanced such that the output partition min/max values are used to filter input frames. This is necessary because all direct mergers read all input frames, but only rows corresponding to a single output partition.
processing/src/main/java/org/apache/druid/frame/processor/SuperSorter.java
Dismissed
Show dismissed
Hide dismissed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
} else { | ||
return UNKNOWN_TOTAL; | ||
} | ||
} else if (level > 0 && level == totalMergingLevels - 2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: i wonder if it might be a bit easier to follow if we moved some of these penultimate and ultimate level checks that do math stuff into named functions to be like level == penultimateLevel(totalMergingLevels)
or isPenultimateLevel(level)
or something since i see them repeated in places, or i guess could also maybe precompute?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isPenultimateLevel
might make sense; I think it'd have to return a tri-state since the answer might be unknown. I/we can think about it in a future patch— simplifying the logic of SuperSorter would be great.
Without the call to readOnly, each output channel retains a 1 MB allocator, leading to excessive memory use. Fixes regression from apache#16775.
Without the call to readOnly, each output channel retains a 1 MB allocator, leading to excessive memory use. Fixes regression from #16775.
Without the call to readOnly, each output channel retains a 1 MB allocator, leading to excessive memory use. Fixes regression from apache#16775.
Two performance enhancements:
Direct merging of input frames to output channels, without any
temporary files, if all input frames fit in memory.
When doing multi-level merging (now called "external mode"),
improve parallelism by boosting up the number of mergers in the
penultimate level.
To support direct merging, FrameChannelMerger is enhanced such that the output partition min/max values are used to filter input frames. This is necessary because all direct mergers read all input frames, but only rows corresponding to a single output partition.