dwsmith1983 commented on code in PR #26793:
URL: https://github.com/apache/datafusion/pull/26793#discussion_r4432313766
##########
datafusion/physical-plan/src/joins/hash_join/exec.rs:
##########
@@ -3234,18 +3232,19 @@ fn build_in_memory(
}
// The maximum is known: avoid geometric growth and its excess
capacity.
let mut hashes_buffer = Vec::with_capacity(max_batch_rows);
- let mut offset = num_rows;
-
- // Update the hashmap starting from the last batch, so rows with
smaller
- // indices end up on top of the chains. Each batch is indexed at its
- // position in the forward concatenation below.
- for batch in batches.iter().rev() {
- offset -= batch.num_rows();
+ // `max_batch_rows` is 0 when the build side is empty
+ let chunk_rows = max_batch_rows.max(1);
+
+ // Update the hashmap from slices of at most `max_batch_rows` rows,
+ // starting from the last one, so rows with smaller indices end up on
+ // top of the chains. Each slice is indexed at its offset in `batch`.
+ for offset in (0..num_rows).step_by(chunk_rows).rev() {
+ let chunk = batch.slice(offset, chunk_rows.min(num_rows - offset));
Review Comment:
Not quite: concat and slice are still linear for dictionary arrays. The cost
is in hashing. A slice shares the parent's whole values array, and
`hash_dictionary` hashes every value on each call, so many short slices of one
big dictionary rehash the same values over and over. A slice wrapper could only
avoid that by compacting the values, which is a copy of its own.
I think the general fix sits in `hash_utils`: an entry point that hashes a
row range of the key arrays and hashes dictionary values once for all ranges
(`hash_dictionary_scatter` already takes precomputed value hashes). Then
operators could slice freely without special cases. I kept this PR to the
smaller change jayzhan211 measured. Happy to open an issue for the `hash_utils`
version if that sounds useful.
--
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]
