More realistic sort benchmarks #5881
Conversation
| // parameters: | ||
| // | ||
| // Input schemas | ||
| lazy_static! { |
There was a problem hiding this comment.
Lazy static seemed overkill for what this was doing, I opted to keep things simple
There was a problem hiding this comment.
Thank you @tustvold -- I reviewed this and it makes sense to me. I also spot checked the inputs (by printing them out to standard out and it looked good)
cc @jaylmiller
| plan: Arc<dyn ExecutionPlan>, | ||
| partition_count: usize, | ||
| } | ||
| fn sort(partitions: &[Vec<RecordBatch>]) -> Self { |
There was a problem hiding this comment.
| fn sort(partitions: &[Vec<RecordBatch>]) -> Self { | |
| /// Test SortExec in "non partitioned" mode which sorts the input streams | |
| /// into a single sorted output stream | |
| fn sort(partitions: &[Vec<RecordBatch>]) -> Self { |
| } | ||
| } | ||
|
|
||
| fn sort_partitioned(partitions: &[Vec<RecordBatch>]) -> Self { |
There was a problem hiding this comment.
| fn sort_partitioned(partitions: &[Vec<RecordBatch>]) -> Self { | |
| /// Test SortExec in "partitioned" mode which sorts the input streams | |
| /// individually into some number of output streams | |
| fn sort_partitioned(partitions: &[Vec<RecordBatch>]) -> Self { |
| fn run(&self) { | ||
| let plan = Arc::clone(&self.plan); | ||
| let task_ctx = Arc::clone(&self.task_ctx); | ||
| fn sort_merge(partitions: &[Vec<RecordBatch>]) -> Self { |
There was a problem hiding this comment.
| fn sort_merge(partitions: &[Vec<RecordBatch>]) -> Self { | |
| /// Test SortExec in "partitioned" mode followed by a SortPreservingMerge | |
| fn sort_merge(partitions: &[Vec<RecordBatch>]) -> Self { |
| let exec = MemoryExec::try_new(partitions, schema, None).unwrap(); | ||
| let exec = | ||
| SortExec::new_with_partitioning(sort.clone(), Arc::new(exec), true, None); | ||
| let plan = Arc::new(SortPreservingMergeExec::new(sort, Arc::new(exec))); |
There was a problem hiding this comment.
I would expect this to behave the same, performance wise, as sort -- is that your expectation too?
There was a problem hiding this comment.
Eventually yes, currently for single column cases it performs worse
datafusion/core/benches/sort.rs
Outdated
| runtime: Runtime, | ||
| task_ctx: Arc<TaskContext>, | ||
|
|
||
| // The plan to run | ||
| plan: Arc<dyn ExecutionPlan>, | ||
| } | ||
|
|
||
| impl SortBenchCase { | ||
| impl BenchCase { | ||
| /// Prepare to run a benchmark that merges the specified | ||
| /// partitions (streams) together using all keyes |
There was a problem hiding this comment.
| /// partitions (streams) together using all keyes | |
| /// pre-sorted partitions (streams) together using all keys |
Looks good to me as well! |
Which issue does this PR close?
Relates to #5879
Rationale for this change
This combines the merge and sort benchmarks together to avoid code duplication, it also makes a number of changes to make the benchmarks more realistic:
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?