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

Add Global deallocation variant (#3516) #3517

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions arrow-buffer/src/alloc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,20 +135,26 @@ pub(crate) enum Deallocation {
/// An allocation of the given capacity that needs to be deallocated using arrows's cache aligned allocator.
/// See [allocate_aligned] and [free_aligned].
Arrow(usize),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually I could see us removing this in favor of Global, but I kept it for now

/// An allocation from an external source like the FFI interface or a Rust Vec.
/// Deallocation will happen
/// An allocation using the system's default global allocator and the provided layout
#[allow(unused)]
Global(Layout),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does increase the size of Dellocation by 8 bytes. In practice this is extremely unlikely to matter

/// An allocation from an external source like the FFI interface
/// Deallocation should happen when `Allocation` is dropped
Custom(Arc<dyn Allocation>),
}

impl Debug for Deallocation {
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
match self {
Deallocation::Arrow(capacity) => {
write!(f, "Deallocation::Arrow {{ capacity: {} }}", capacity)
write!(f, "Deallocation::Arrow {{ capacity: {capacity} }}")
}
Deallocation::Custom(_) => {
write!(f, "Deallocation::Custom {{ capacity: unknown }}")
}
Deallocation::Global(layout) => {
write!(f, "Deallocation::Global {{ alignment: {layout:?} }}")
}
}
}
}
4 changes: 4 additions & 0 deletions arrow-buffer/src/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ impl Bytes {
pub fn capacity(&self) -> usize {
match self.deallocation {
Deallocation::Arrow(capacity) => capacity,
Deallocation::Global(l) => l.size(),
// we cannot determine this in general,
// and thus we state that this is externally-owned memory
Deallocation::Custom(_) => 0,
Expand All @@ -118,6 +119,9 @@ impl Drop for Bytes {
Deallocation::Arrow(capacity) => {
unsafe { alloc::free_aligned(self.ptr, *capacity) };
}
Deallocation::Global(layout) => {
unsafe { std::alloc::dealloc(self.ptr.as_ptr(), *layout) };
}
// The automatic drop implementation will free the memory once the reference count reaches zero
Deallocation::Custom(_allocation) => (),
}
Expand Down