[GitHub] [arrow-rs] alamb commented on a change in pull request #1494: RFC: Decouple buffer deallocation from ffi and allow creating buffers from rust vec

[GitBox <gi...@apache.org>]

alamb commented on a change in pull request #1494:
URL: https://github.com/apache/arrow-rs/pull/1494#discussion_r837811862



##########
File path: arrow/src/alloc/mod.rs
##########
@@ -121,3 +124,30 @@ pub unsafe fn reallocate<T: NativeType>(
         handle_alloc_error(Layout::from_size_align_unchecked(new_size, ALIGNMENT))
     })
 }
+
+/// The owner of an allocation, that is not natively allocated.
+/// The trait implementation is responsible for dropping the allocations once no more references exist.
+pub trait Allocation: RefUnwindSafe {}
+
+impl<T: RefUnwindSafe> Allocation for T {}
+
+/// Mode of deallocating memory regions
+pub enum Deallocation {
+    /// Native deallocation, using Rust deallocator with Arrow-specific memory alignment
+    Native(usize),
+    /// Foreign interface, via a callback
+    Foreign(Arc<dyn Allocation>),
+}
+
+impl Debug for Deallocation {
+    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
+        match self {
+            Deallocation::Native(capacity) => {

Review comment:
       This seems inconsistent with the comment above. This code says `capacity` but the comment above says `Native(usize)` contains alignment

##########
File path: arrow/src/array/data.rs
##########
@@ -2594,4 +2595,52 @@ mod tests {
 
         assert_eq!(&struct_array_slice, &cloned);
     }
+
+    #[test]
+    fn test_string_data_from_foreign() {
+        let mut strings = "foobarfoobar".to_owned();
+        let mut offsets = vec![0_i32, 0, 3, 6, 12];
+        let mut bitmap = vec![0b1110_u8];
+
+        let strings_buffer = unsafe {
+            Buffer::from_foreign(
+                NonNull::new_unchecked(strings.as_mut_ptr()),
+                strings.len(),
+                Arc::new(strings),

Review comment:
       It seems pretty neat to me that the ownership of the `Vec` is transferred to `Arc` which is then stored (as `dyn Allocation`)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org