dwsmith1983 commented on code in PR #25933:
URL: https://github.com/apache/datafusion/pull/25933#discussion_r4176175075
##########
datafusion/physical-plan/src/joins/sort_merge_join/bitwise_stream.rs:
##########
@@ -669,7 +669,15 @@ impl BitwiseSortMergeJoinStream {
let inner_batch = self.inner_batch.as_ref().unwrap();
let slice = inner_batch.slice(from, group_end - from);
- self.inner_buffer_size += slice.get_array_memory_size();
+ // A group ending inside the batch shares the current inner batch,
+ // so charge only its rows. A group reaching the batch end keeps
+ // the whole parent alive once the cursor advances, so charge all
+ // of it. View arrays still count their parent's data buffers.
+ self.inner_buffer_size += if group_end < num_inner {
Review Comment:
Thanks, applied in f99f277ae rather than in a follow-up. One change from
your diff: I wrote it as `parent_size.div_ceil(num_inner) * slice.num_rows()`,
since `parent_size * rows` can overflow `usize` on wasm32 (a 4 MiB batch with a
1024-row group is already 2^32). It overcharges by at most one byte per row.
The sliced-size test is now `bitwise_small_key_groups_charged_by_row_share`,
and its headroom check uses the same share.
I reran your shape (filtered LeftSemi/LeftAnti, 400K keys, 1 outer and 1 to
4 inner Int64 rows per key, batch 8192, no memory limit), building main, the
previous head and this version in one container and running them interleaved
for three rounds:
| | main | previous head (`get_sliced_size`) | this version |
|---|---|---|---|
| LeftSemi | 220 to 233 ms | 266 ms (1.14 to 1.21x) | 221 to 233 ms (1.00 to
1.01x) |
| LeftAnti | 220 to 234 ms | 266 ms (1.14 to 1.21x) | 222 to 234 ms (1.00 to
1.01x) |
Running each tree in its own container drifted by up to 25%, so I'd treat
cross-run numbers on this shape with care.
--
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]
