Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory accounting in GenericBytesView overcounts allocation size #7099

Open
alamb opened this issue Feb 8, 2025 · 0 comments
Open

Memory accounting in GenericBytesView overcounts allocation size #7099

alamb opened this issue Feb 8, 2025 · 0 comments
Labels
arrow Changes to the arrow crate bug good first issue Good for newcomers

Comments

@alamb
Copy link
Contributor

alamb commented Feb 8, 2025

          This is something I also noticed in #7082

One thing to note is that NullBufferBuilder::allocated_size() is used here:

/// Return the allocated size of this builder in bytes, useful for memory accounting.
pub fn allocated_size(&self) -> usize {
let views = self.views_builder.capacity() * std::mem::size_of::<u128>();
let null = self.null_buffer_builder.allocated_size();
let buffer_size = self.completed.iter().map(|b| b.capacity()).sum::<usize>();
let in_progress = self.in_progress.capacity();
let tracker = match &self.string_tracker {
Some((ht, _)) => ht.capacity() * std::mem::size_of::<usize>(),
None => 0,
};
buffer_size + in_progress + tracker + views + null
}

And I think it's used with assumption it provides bytes not bits, so may need adjustment.

Originally posted by @Jefffrey in #7089 (review)

I think the adjustment is to divide the allocated_size by 8.

Another solution would be to deprecate allocated_size entirely and make a new function that returns allocated size in bytes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate bug good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant