jhorstmann commented on a change in pull request #8796:
URL: https://github.com/apache/arrow/pull/8796#discussion_r546362442



##########
File path: rust/arrow/src/buffer.rs
##########
@@ -208,15 +158,8 @@ impl Buffer {
 /// allocated memory region.
 impl<T: AsRef<[u8]>> From<T> for Buffer {
     fn from(p: T) -> Self {
-        // allocate aligned memory buffer
-        let slice = p.as_ref();
-        let len = slice.len() * mem::size_of::<u8>();
-        let capacity = bit_util::round_upto_multiple_of_64(len);
-        let buffer = memory::allocate_aligned(capacity);
-        unsafe {
-            memory::memcpy(buffer, slice.as_ptr(), len);
-            Buffer::build_with_arguments(buffer, len, 
Deallocation::Native(capacity))
-        }
+        let bytes = unsafe { Bytes::new(p.as_ref().to_vec(), 
Deallocation::Native) };

Review comment:
       There could be potential for further optimization here: `to_vec` has to 
copy the slice contents, a separate implementation of `From<Vec<u8>>` or 
`From<Vec<ArrowPrimitiveType>>` could avoid that copy and speed up several 
kernels involving primitives or list offsets.
   
   As a `From` implementation that would give a "conflicting implementations" 
error, an explicit `from_vec` method could work. I'd suggest trying it in a 
separate PR as it could change a bunch of code not directly related to the 
refactoring in this PR.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to