Skip to content

test: add more tests and docs for heap size estimation - #22358

Merged
kosiew merged 2 commits into
apache:mainfrom
mkleen:improve-heapsize
May 22, 2026
Merged

test: add more tests and docs for heap size estimation#22358
kosiew merged 2 commits into
apache:mainfrom
mkleen:improve-heapsize

Conversation

@mkleen

@mkleenmkleen commented May 19, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

None.

Rationale for this change

This is a follow-up for #20047 improving docs and test coverage for the heap size estimation.

What changes are included in this PR?

See above.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@github-actionsgithub-actionsBot added the common Related to common crate label May 19, 2026
/// This is a temporary solution until <https://github.com/apache/datafusion/pull/19599> and
/// <https://github.com/apache/arrow-rs/pull/9138> are resolved.
/// Trait for calculating the size of various containers
/// Trait for computing how many bytes a value has allocated on the heap.

@mkleenmkleenMay 19, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this now because the heapsize estimations in Datafusion already diverged from the once in arrow-rs and there is a good chance that this code continues to live here.

@mkleen
mkleenforce-pushed the improve-heapsize branch from 1d4d456 to bea454cCompareMay 19, 2026 09:57
@mkleenmkleen changed the title Add more tests and docs for heap size estimationtest:Add more tests and docs for heap size estimationMay 20, 2026
@mkleenmkleen changed the title test:Add more tests and docs for heap size estimationtest: Add more tests and docs for heap size estimationMay 20, 2026
@mkleen
mkleenforce-pushed the improve-heapsize branch from bea454c to d261843CompareMay 20, 2026 07:18
@mkleen
mkleenforce-pushed the improve-heapsize branch from d261843 to 20bf843CompareMay 20, 2026 07:38
@mkleenmkleen changed the title test: Add more tests and docs for heap size estimationtest: add more tests and docs for heap size estimationMay 20, 2026
@mkleen

mkleen commented May 20, 2026

Copy link
Copy Markdown
ContributorAuthor

@kosiew Could you maybe have a quick look since you are familiar with the original pr?

@kosiewkosiew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkleen

Thanks for the ping.
I found one issue around the new HeapSize invariant and the added test coverage.

Comment threaddatafusion/common/src/heap_size.rs Outdated
@mkleen
mkleen requested a review from kosiewMay 20, 2026 12:00
@mkleen
mkleenforce-pushed the improve-heapsize branch from 1d657f9 to 3fe1bafCompareMay 20, 2026 12:26

@kosiewkosiew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkleen

Thanks for the iteration.
Looks 👍 to me

@mkleen

Copy link
Copy Markdown
ContributorAuthor

@kosiew Thanks for the review!

@kosiew
kosiew added this pull request to the merge queueMay 22, 2026
Merged via the queue into apache:main with commit 541119eMay 22, 2026
35 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commonRelated to common crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@mkleen@kosiew