From 5d1abfeb3271ebd5429766152242c0719ac93e8e Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Fri, 29 Nov 2024 17:24:27 +0000 Subject: [PATCH 1/3] Support shrink to empty --- arrow-buffer/src/buffer/immutable.rs | 2 +- arrow-buffer/src/buffer/mutable.rs | 2 +- arrow-buffer/src/bytes.rs | 26 ++++++++++++++++++-------- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/arrow-buffer/src/buffer/immutable.rs b/arrow-buffer/src/buffer/immutable.rs index be87d726e86..dc0de7736bd 100644 --- a/arrow-buffer/src/buffer/immutable.rs +++ b/arrow-buffer/src/buffer/immutable.rs @@ -614,7 +614,7 @@ mod tests { let mut shrunk_empty = empty_slice; shrunk_empty.shrink_to_fit(); assert_eq!(shrunk_empty.as_slice(), &[]); - assert_eq!(shrunk_empty.capacity(), 1); // NOTE: `Buffer` and `Bytes` doesn't support 0-capacity + assert_eq!(shrunk_empty.capacity(), 0); } #[test] diff --git a/arrow-buffer/src/buffer/mutable.rs b/arrow-buffer/src/buffer/mutable.rs index 5471bc0e5f6..8a7be9c25fe 100644 --- a/arrow-buffer/src/buffer/mutable.rs +++ b/arrow-buffer/src/buffer/mutable.rs @@ -468,7 +468,7 @@ impl MutableBuffer { } #[inline] -fn dangling_ptr() -> NonNull { +pub(crate) fn dangling_ptr() -> NonNull { // SAFETY: ALIGNMENT is a non-zero usize which is then casted // to a *mut T. Therefore, `ptr` is not null and the conditions for // calling new_unchecked() are respected. diff --git a/arrow-buffer/src/bytes.rs b/arrow-buffer/src/bytes.rs index 924b7a72027..77724137aef 100644 --- a/arrow-buffer/src/bytes.rs +++ b/arrow-buffer/src/bytes.rs @@ -24,6 +24,7 @@ use std::ptr::NonNull; use std::{fmt::Debug, fmt::Formatter}; use crate::alloc::Deallocation; +use crate::buffer::dangling_ptr; /// A continuous, fixed-size, immutable memory region that knows how to de-allocate itself. /// @@ -104,20 +105,29 @@ impl Bytes { /// In case of `Err`, the [`Bytes`] will remain as it was (i.e. have the old size). pub fn try_realloc(&mut self, new_len: usize) -> Result<(), ()> { if let Deallocation::Standard(old_layout) = self.deallocation { - let new_len = new_len.max(1); // realloc requires a non-zero size if old_layout.size() == new_len { return Ok(()); // Nothing to do } + if let Ok(new_layout) = std::alloc::Layout::from_size_align(new_len, old_layout.align()) { let old_ptr = self.ptr.as_ptr(); - // SAFETY: the call to `realloc` is safe if all of the following holds (from https://doc.rust-lang.org/stable/std/alloc/trait.GlobalAlloc.html#method.realloc): - // * `old_ptr` must be currently allocated via this allocator (guaranteed by the invariant/contract of `Bytes`) - // * `old_layout` must be the same layout that was used to allocate that block of memory (same) - // * `new_len` must be greater than zero (ensured by the `max` call earlier) - // * `new_len`, when rounded up to the nearest multiple of `layout.align()`, must not overflow `isize` (guaranteed by the success of `Layout::from_size_align`) - let new_ptr = unsafe { std::alloc::realloc(old_ptr, old_layout, new_len) }; - if let Some(ptr) = NonNull::new(new_ptr) { + + let new_ptr = match new_layout.size() { + 0 => { + // SAFETY: Verified that old_layout.size != new_len (0) + unsafe { std::alloc::dealloc(self.ptr.as_ptr(), old_layout) }; + Some(dangling_ptr()) + } + // SAFETY: the call to `realloc` is safe if all the following hold (from https://doc.rust-lang.org/stable/std/alloc/trait.GlobalAlloc.html#method.realloc): + // * `old_ptr` must be currently allocated via this allocator (guaranteed by the invariant/contract of `Bytes`) + // * `old_layout` must be the same layout that was used to allocate that block of memory (same) + // * `new_len` must be greater than zero + // * `new_len`, when rounded up to the nearest multiple of `layout.align()`, must not overflow `isize` (guaranteed by the success of `Layout::from_size_align`) + _ => NonNull::new(unsafe { std::alloc::realloc(old_ptr, old_layout, new_len) }), + }; + + if let Some(ptr) = new_ptr { self.ptr = ptr; self.len = new_len; self.deallocation = Deallocation::Standard(new_layout); From a310c3183ce447b7d833d03267ca8caa72b10625 Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Sun, 1 Dec 2024 13:56:10 +0000 Subject: [PATCH 2/3] Docs --- arrow-buffer/src/buffer/mutable.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arrow-buffer/src/buffer/mutable.rs b/arrow-buffer/src/buffer/mutable.rs index 8a7be9c25fe..d3436cda0e3 100644 --- a/arrow-buffer/src/buffer/mutable.rs +++ b/arrow-buffer/src/buffer/mutable.rs @@ -467,10 +467,14 @@ impl MutableBuffer { } } + +/// Creates a non-null pointer with alignment of [`ALIGNMENT`] +/// +/// This is similar to [`NonNull::dangling`] #[inline] pub(crate) fn dangling_ptr() -> NonNull { - // SAFETY: ALIGNMENT is a non-zero usize which is then casted - // to a *mut T. Therefore, `ptr` is not null and the conditions for + // SAFETY: ALIGNMENT is a non-zero usize which is then cast + // to a *mut u8. Therefore, `ptr` is not null and the conditions for // calling new_unchecked() are respected. #[cfg(miri)] { From 5fe0f71f75ae592771b8ad7c5e844f75185a7d00 Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Sun, 1 Dec 2024 14:00:39 +0000 Subject: [PATCH 3/3] Format --- arrow-buffer/src/buffer/mutable.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/arrow-buffer/src/buffer/mutable.rs b/arrow-buffer/src/buffer/mutable.rs index d3436cda0e3..c4315a1d64c 100644 --- a/arrow-buffer/src/buffer/mutable.rs +++ b/arrow-buffer/src/buffer/mutable.rs @@ -467,7 +467,6 @@ impl MutableBuffer { } } - /// Creates a non-null pointer with alignment of [`ALIGNMENT`] /// /// This is similar to [`NonNull::dangling`]