jayzhan211 commented on code in PR #25933:
URL: https://github.com/apache/datafusion/pull/25933#discussion_r4176039593
##########
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:
I took a deeper look and want to revise my suggestion, what do you think,
this could be follow-up
`get_sliced_size()` runs once per buffered key group and allocates per
column (`to_data()` + `layout()`). On the #25923 shape (filtered
LeftSemi/LeftAnti, 1 outer + 1–4 inner Int64 rows per key, 400K keys, batch
8192, no memory pressure) that is 1.18–1.32x slower than main; calling
`get_sliced_size()` but still charging `get_array_memory_size()` reproduces the
whole slowdown.
An O(1) share of the parent passes all 249 SMJ tests, measured 1.04–1.16x (a
variant doing less work than main measured 1.02–1.08x), and drops the
view-array caveat:
```diff
-use crate::spill::spill_manager::{GetSlicedSize, SpillManager};
+use crate::spill::spill_manager::SpillManager;
```
```diff
- self.inner_buffer_size += if group_end < num_inner {
- slice.get_sliced_size()?
- } else {
- slice.get_array_memory_size()
- };
+ let parent_size = slice.get_array_memory_size();
+ self.inner_buffer_size += if group_end < num_inner {
+ (parent_size * slice.num_rows()).div_ceil(num_inner)
+ } else {
+ parent_size
+ };
```
--
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]
