You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/08/29 03:52:40 UTC
(datafusion) branch gh-readonly-queue/main/pr-24744-8403871425c1c459604e8a4ee1fa531d5c4e3f80 created (now dd77ffc102)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-24744-8403871425c1c459604e8a4ee1fa531d5c4e3f80
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at dd77ffc102 Fix OneSideHashJoiner memory size accounting (#24744)
This branch includes the following new commits:
new dd77ffc102 Fix OneSideHashJoiner memory size accounting (#24744)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: Fix OneSideHashJoiner memory size accounting (#24744)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24744-8403871425c1c459604e8a4ee1fa531d5c4e3f80
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit dd77ffc102fdd4a6bfa67fdefc1ddac97d538cdd
Author: kosiew <ko...@gmail.com>
AuthorDate: Sat Aug 29 03:52:25 2026 +0000
Fix OneSideHashJoiner memory size accounting (#24744)
## Which issue does this PR close?
* Part of #23393
## Rationale for this change
`OneSideHashJoiner::size()` can overcount memory by adding inline fields
that are already included in the joiner descriptor, while its previous
`HashSet<usize>` estimate only accounted for element payload capacity
and omitted hashbrown control/metadata allocation.
This change defines the component-level accounting contract more
explicitly so the joiner descriptor is counted once and directly owned
container allocations are included using their actual allocation-aware
helpers where available.
## What changes are included in this PR?
* Document the `OneSideHashJoiner::size()` contract as including the
joiner descriptor, directly owned container allocations, and input
arrays, while excluding allocations referenced by shared `on`
expressions.
* Remove double counting of inline fields already covered by
`size_of_val(self)`.
* Account for the `on` vector's owned backing allocation using its
capacity.
* Adjust `PruningJoinHashMap` accounting so its descriptor is not
counted twice.
* Replace the `visited_rows` payload-only capacity calculation with
`allocation_size()`, which accounts for the hashbrown allocation layout,
including control bytes.
* Add regression coverage for empty and grown `visited_rows` states and
for the owned `on`, hash map, and hashes-buffer allocations.
* No transformer composition or shared-record-batch accounting is
changed.
## Are these changes tested?
Yes. This PR adds the focused unit test:
`one_side_hash_joiner_size_counts_descriptor_and_hash_set_allocation_once`
The test checks the explicit expected size formula for an empty joiner,
verifies `visited_rows` allocation accounting across capacity growth,
and verifies that owned container allocation changes are reflected
without double-counting descriptors.
## Are there any user-facing changes?
No public API or query-result behavior changes are introduced.
This changes internal memory-size accounting for `OneSideHashJoiner`,
making its reported size more consistent with the documented ownership
contract.
## LLM-generated code disclosure
This PR includes LLM-generated code and comments. All LLM-generated
content has been manually reviewed.
---
.../physical-plan/src/joins/symmetric_hash_join.rs | 82 +++++++++++++++++++---
1 file changed, 71 insertions(+), 11 deletions(-)
diff --git a/datafusion/physical-plan/src/joins/symmetric_hash_join.rs b/datafusion/physical-plan/src/joins/symmetric_hash_join.rs
index c9b9975d75..aec693e839 100644
--- a/datafusion/physical-plan/src/joins/symmetric_hash_join.rs
+++ b/datafusion/physical-plan/src/joins/symmetric_hash_join.rs
@@ -1470,18 +1470,20 @@ pub struct OneSideHashJoiner {
}
impl OneSideHashJoiner {
+ /// Returns the joiner descriptor, directly owned container allocations, and input arrays.
+ ///
+ /// Allocations referenced by `on` are shared plan expressions and are excluded.
pub fn size(&self) -> usize {
- let mut size = 0;
- size += size_of_val(self);
- size += size_of_val(&self.build_side);
- size += self.input_buffer.get_array_memory_size();
- size += size_of_val(&self.on);
- size += self.hashmap.size();
- size += self.hashes_buffer.capacity() * size_of::<u64>();
- size += self.visited_rows.capacity() * size_of::<usize>();
- size += size_of_val(&self.offset);
- size += size_of_val(&self.deleted_offset);
- size
+ // `allocation_size` uses the hashbrown 0.17 layout, including control bytes.
+ size_of_val(self)
+ + self.input_buffer.get_array_memory_size()
+ + self.on.capacity() * size_of::<PhysicalExprRef>()
+ // `PruningJoinHashMap::size()` includes its descriptor, which is
+ // already included by `size_of_val(self)`.
+ + self.hashmap.size()
+ - size_of_val(&self.hashmap)
+ + self.hashes_buffer.capacity() * size_of::<u64>()
+ + self.visited_rows.allocation_size()
}
fn size_without_input_buffer(&self) -> usize {
@@ -2203,6 +2205,64 @@ mod tests {
}
}
+ fn expected_one_side_hash_joiner_size(joiner: &OneSideHashJoiner) -> usize {
+ size_of_val(joiner)
+ + joiner.input_buffer.get_array_memory_size()
+ + joiner.on.capacity() * size_of::<PhysicalExprRef>()
+ // `PruningJoinHashMap::size()` includes its inline descriptor.
+ + joiner.hashmap.size()
+ - size_of_val(&joiner.hashmap)
+ + joiner.hashes_buffer.capacity() * size_of::<u64>()
+ + joiner.visited_rows.allocation_size()
+ }
+
+ #[test]
+ fn one_side_hash_joiner_size_counts_descriptor_and_hash_set_allocation_once() {
+ let schema = Arc::new(Schema::empty());
+ let mut joiner = OneSideHashJoiner::new(JoinSide::Left, vec![], schema);
+
+ let empty_size = joiner.size();
+ assert_eq!(empty_size, expected_one_side_hash_joiner_size(&joiner));
+ assert_eq!(joiner.visited_rows.allocation_size(), 0);
+
+ joiner.visited_rows.extend(0..3);
+ let initial_capacity = joiner.visited_rows.capacity();
+ let initial_allocation = joiner.visited_rows.allocation_size();
+ assert!(initial_capacity > 0);
+ assert_eq!(joiner.size(), expected_one_side_hash_joiner_size(&joiner));
+ assert_eq!(joiner.size() - empty_size, initial_allocation);
+
+ joiner.visited_rows.extend(3..initial_capacity * 2);
+ let grown_allocation = joiner.visited_rows.allocation_size();
+ assert!(joiner.visited_rows.capacity() > initial_capacity);
+ assert_eq!(joiner.size(), expected_one_side_hash_joiner_size(&joiner));
+ assert_eq!(joiner.size() - empty_size, grown_allocation);
+ assert!(grown_allocation > initial_allocation);
+
+ let size_with_visited_rows = joiner.size();
+ let initial_hashmap_allocation =
+ joiner.hashmap.size() - size_of_val(&joiner.hashmap);
+ joiner.on = vec![Arc::new(Column::new("a", 0))];
+ joiner.hashmap = PruningJoinHashMap::with_capacity(3);
+ joiner.hashes_buffer.try_reserve(3).unwrap();
+ assert!(joiner.on.capacity() > 0);
+ assert!(joiner.hashmap.map.capacity() > 0);
+ assert!(joiner.hashes_buffer.capacity() > 0);
+
+ let owned_container_allocation_delta = joiner.on.capacity()
+ * size_of::<PhysicalExprRef>()
+ + (joiner.hashmap.size()
+ - size_of_val(&joiner.hashmap)
+ - initial_hashmap_allocation)
+ + joiner.hashes_buffer.capacity() * size_of::<u64>();
+ assert!(owned_container_allocation_delta > 0);
+ assert_eq!(joiner.size(), expected_one_side_hash_joiner_size(&joiner));
+ assert_eq!(
+ joiner.size() - size_with_visited_rows,
+ owned_container_allocation_delta
+ );
+ }
+
fn assert_stream_accounts_for_transformer<T: BatchTransformer>(batch_transformer: T) {
let batch = RecordBatch::try_from_iter(vec![(
"a",
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org