viirya commented on code in PR #2693:
URL: https://github.com/apache/arrow-rs/pull/2693#discussion_r967883686
##########
arrow-buffer/src/alloc/mod.rs:
##########
@@ -20,34 +20,31 @@
use std::alloc::{handle_alloc_error, Layout};
use std::fmt::{Debug, Formatter};
-use std::mem::size_of;
use std::panic::RefUnwindSafe;
use std::ptr::NonNull;
use std::sync::Arc;
mod alignment;
-mod types;
pub use alignment::ALIGNMENT;
-pub use types::NativeType;
#[inline]
-unsafe fn null_pointer<T: NativeType>() -> NonNull<T> {
- NonNull::new_unchecked(ALIGNMENT as *mut T)
+unsafe fn null_pointer() -> NonNull<u8> {
+ NonNull::new_unchecked(ALIGNMENT as *mut u8)
}
/// Allocates a cache-aligned memory region of `size` bytes with uninitialized
values.
/// This is more performant than using [allocate_aligned_zeroed] when all
bytes will have
/// an unknown or non-zero value and is semantically similar to `malloc`.
-pub fn allocate_aligned<T: NativeType>(size: usize) -> NonNull<T> {
+pub fn allocate_aligned(size: usize) -> NonNull<u8> {
unsafe {
if size == 0 {
null_pointer()
} else {
- let size = size * size_of::<T>();
+ let size = size;
Review Comment:
```suggestion
```
##########
arrow-buffer/src/bytes.rs:
##########
@@ -0,0 +1,144 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! This module contains an implementation of a contiguous immutable memory
region that knows
+//! how to de-allocate itself, [`Bytes`].
+//! Note that this is a low-level functionality of this crate.
+
+use core::slice;
+use std::ptr::NonNull;
+use std::{fmt::Debug, fmt::Formatter};
+
+use crate::alloc;
+use crate::alloc::Deallocation;
+
+/// A continuous, fixed-size, immutable memory region that knows how to
de-allocate itself.
Review Comment:
Isn't this moved from arrow/src/bytes.rs? But seems copying instead of
moving?
##########
arrow-buffer/src/alloc/mod.rs:
##########
@@ -20,34 +20,31 @@
use std::alloc::{handle_alloc_error, Layout};
use std::fmt::{Debug, Formatter};
-use std::mem::size_of;
use std::panic::RefUnwindSafe;
use std::ptr::NonNull;
use std::sync::Arc;
mod alignment;
-mod types;
pub use alignment::ALIGNMENT;
-pub use types::NativeType;
#[inline]
-unsafe fn null_pointer<T: NativeType>() -> NonNull<T> {
- NonNull::new_unchecked(ALIGNMENT as *mut T)
+unsafe fn null_pointer() -> NonNull<u8> {
+ NonNull::new_unchecked(ALIGNMENT as *mut u8)
}
/// Allocates a cache-aligned memory region of `size` bytes with uninitialized
values.
Review Comment:
Looks like previously the doc is not totally correct as `size` will be
multiplied by size of T.
##########
arrow-buffer/src/alloc/mod.rs:
##########
@@ -20,34 +20,31 @@
use std::alloc::{handle_alloc_error, Layout};
use std::fmt::{Debug, Formatter};
-use std::mem::size_of;
use std::panic::RefUnwindSafe;
use std::ptr::NonNull;
use std::sync::Arc;
mod alignment;
-mod types;
pub use alignment::ALIGNMENT;
-pub use types::NativeType;
#[inline]
-unsafe fn null_pointer<T: NativeType>() -> NonNull<T> {
- NonNull::new_unchecked(ALIGNMENT as *mut T)
+unsafe fn null_pointer() -> NonNull<u8> {
+ NonNull::new_unchecked(ALIGNMENT as *mut u8)
}
/// Allocates a cache-aligned memory region of `size` bytes with uninitialized
values.
/// This is more performant than using [allocate_aligned_zeroed] when all
bytes will have
/// an unknown or non-zero value and is semantically similar to `malloc`.
-pub fn allocate_aligned<T: NativeType>(size: usize) -> NonNull<T> {
+pub fn allocate_aligned(size: usize) -> NonNull<u8> {
Review Comment:
So the caller needs to compute the allocation size by the type T they want?
##########
arrow-buffer/src/alloc/mod.rs:
##########
@@ -20,34 +20,31 @@
use std::alloc::{handle_alloc_error, Layout};
use std::fmt::{Debug, Formatter};
-use std::mem::size_of;
use std::panic::RefUnwindSafe;
use std::ptr::NonNull;
use std::sync::Arc;
mod alignment;
-mod types;
pub use alignment::ALIGNMENT;
-pub use types::NativeType;
#[inline]
-unsafe fn null_pointer<T: NativeType>() -> NonNull<T> {
- NonNull::new_unchecked(ALIGNMENT as *mut T)
+unsafe fn null_pointer() -> NonNull<u8> {
+ NonNull::new_unchecked(ALIGNMENT as *mut u8)
}
/// Allocates a cache-aligned memory region of `size` bytes with uninitialized
values.
Review Comment:
But actually I don't see any allocate_aligned usage with types other than u8
so far.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
