Skip to content

Commit b790763

Browse files
authored
refactor: centralizing shared-allocation accounting for Arc DFHeapSize impls (#23349)
## Which issue does this PR close? - Closes #22867 ## Rationale for this change DFHeapSize depends on a critical invariant: shared heap allocations must be counted once per traversal context. If Arc implementations drift, memory accounting becomes inconsistent and hard to reason about. Consolidating the one-time-accounting logic improves: Correctness durability (single source of truth for Arc dedup behavior) Maintainability (less repeated logic) Reviewability (future changes touch one helper) ## What changes are included in this PR? Helper functions for `Arc` allocation identity and deduplication, namely for providing pointer extraction & `DFHeapSizeCtx` set membership checks, with the `DFHeapSize` implementations on `Arc` backed types using the new helpers ## Are these changes tested? Yes, the below are passing: ``` cargo test -p datafusion-common heap_size --lib cargo test -p datafusion-common --doc heap_size ``` ## Are there any user-facing changes? No
1 parent 4ca85d2 commit b790763

1 file changed

Lines changed: 22 additions & 6 deletions

File tree

datafusion/common/src/heap_size.rs

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ pub struct DFHeapSizeCtx {
7676
seen: HashSet<usize>,
7777
}
7878

79+
impl DFHeapSizeCtx {
80+
fn count_allocation_once(&mut self, ptr: usize) -> bool {
81+
self.seen.insert(ptr)
82+
}
83+
}
84+
7985
impl DFHeapSize for Statistics {
8086
fn heap_size(&self, ctx: &mut DFHeapSizeCtx) -> usize {
8187
self.num_rows.heap_size(ctx)
@@ -281,11 +287,21 @@ impl<K: DFHeapSize, V: DFHeapSize> DFHeapSize for HashMap<K, V> {
281287
}
282288
}
283289

290+
fn arc_ptr<T>(arc: &Arc<T>) -> usize {
291+
Arc::as_ptr(arc) as usize
292+
}
293+
294+
/// For unsized types, `Arc::as_ptr` returns the data address + metadata - we only need the thin address
295+
/// Casting through `*const i32` gets us the thin pointer
296+
fn arc_unsized_ptr<T: ?Sized>(arc: &Arc<T>) -> usize {
297+
Arc::as_ptr(arc) as *const i32 as usize
298+
}
299+
284300
impl<T: DFHeapSize> DFHeapSize for Arc<T> {
285301
fn heap_size(&self, ctx: &mut DFHeapSizeCtx) -> usize {
286-
let ptr = Arc::as_ptr(self) as usize;
302+
let ptr = arc_ptr(self);
287303

288-
if !ctx.seen.insert(ptr) {
304+
if !ctx.count_allocation_once(ptr) {
289305
return 0;
290306
}
291307

@@ -296,9 +312,9 @@ impl<T: DFHeapSize> DFHeapSize for Arc<T> {
296312

297313
impl DFHeapSize for Arc<str> {
298314
fn heap_size(&self, ctx: &mut DFHeapSizeCtx) -> usize {
299-
let ptr = Arc::as_ptr(self) as *const i32 as usize;
315+
let ptr = arc_unsized_ptr(self);
300316

301-
if !ctx.seen.insert(ptr) {
317+
if !ctx.count_allocation_once(ptr) {
302318
return 0;
303319
}
304320

@@ -309,9 +325,9 @@ impl DFHeapSize for Arc<str> {
309325

310326
impl DFHeapSize for Arc<dyn DFHeapSize> {
311327
fn heap_size(&self, ctx: &mut DFHeapSizeCtx) -> usize {
312-
let ptr = Arc::as_ptr(self) as *const i32 as usize;
328+
let ptr = arc_unsized_ptr(self);
313329

314-
if !ctx.seen.insert(ptr) {
330+
if !ctx.count_allocation_once(ptr) {
315331
return 0;
316332
}
317333

0 commit comments

Comments
 (0)