geoffreyclaude commented on code in PR #26242:
URL: https://github.com/apache/datafusion/pull/26242#discussion_r4346222153
##########
datafusion/physical-plan/src/sorts/sort.rs:
##########
@@ -707,11 +754,20 @@ impl ExternalSorter {
batches
};
- let streams = self
- .sort_run_streams(runs)?
- .into_iter()
- .map(|stream| spawn_buffered(stream, 1))
- .collect();
+ let concurrency =
SortConcurrency::for_current_runtime(&self.sort_concurrency);
Review Comment:
I tested this on my fork, in
[[#22](https://github.com/geoffreyclaude/datafusion/pull/22)](https://github.com/geoffreyclaude/datafusion/pull/22).
Coalescing made fragmented-input sorts 19–79% faster (depending on the benched
scenario), with essentially unchanged performance on the ordinary preloaded
wide sort. This definitely looks worth pursuing separately: I can open a
follow-up PR as the gains are massive on small batches, and the code change is
pretty simple.
To be clear though, our production case involved very large inputs with
8,192-row batches. The 32-row batches simply stress run count with a smaller
dataset and they aren’t intended to represent typical workloads. Coalescing
helps fragmented inputs, while the task bound remains necessary for the
production case.
--
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]
