From e3f31557b125d036e2128c4472f161d5cd265476 Mon Sep 17 00:00:00 2001 From: "Jorge C. Leitao" Date: Mon, 6 Sep 2021 07:02:36 +0000 Subject: [PATCH] Removed custom allocator. --- src/alloc/alignment.rs | 119 ------------- src/alloc/mod.rs | 133 --------------- src/bitmap/bitmap_ops.rs | 4 +- src/buffer/bytes.rs | 14 +- src/buffer/immutable.rs | 3 +- src/buffer/mod.rs | 1 - src/buffer/mutable.rs | 299 ++++++++------------------------- src/buffer/util.rs | 12 -- src/lib.rs | 2 - src/types/bit_chunk.rs | 21 +-- src/types/mod.rs | 53 +++++- tests/it/alloc.rs | 49 ------ tests/it/array/utf8/mutable.rs | 4 +- tests/it/bitmap/mutable.rs | 9 +- tests/it/buffer/mutable.rs | 7 +- tests/it/main.rs | 1 - 16 files changed, 139 insertions(+), 592 deletions(-) delete mode 100644 src/alloc/alignment.rs delete mode 100644 src/alloc/mod.rs delete mode 100644 src/buffer/util.rs delete mode 100644 tests/it/alloc.rs diff --git a/src/alloc/alignment.rs b/src/alloc/alignment.rs deleted file mode 100644 index dbf4602f83a..00000000000 --- a/src/alloc/alignment.rs +++ /dev/null @@ -1,119 +0,0 @@ -// 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. - -// NOTE: Below code is written for spatial/temporal prefetcher optimizations. Memory allocation -// should align well with usage pattern of cache access and block sizes on layers of storage levels from -// registers to non-volatile memory. These alignments are all cache aware alignments incorporated -// from [cuneiform](https://crates.io/crates/cuneiform) crate. This approach mimicks Intel TBB's -// cache_aligned_allocator which exploits cache locality and minimizes prefetch signals -// resulting in less round trip time between the layers of storage. -// For further info: https://software.intel.com/en-us/node/506094 - -// 32-bit architecture and things other than netburst microarchitecture are using 64 bytes. -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "x86")] -pub const ALIGNMENT: usize = 1 << 6; - -// Intel x86_64: -// L2D streamer from L1: -// Loads data or instructions from memory to the second-level cache. To use the streamer, -// organize the data or instructions in blocks of 128 bytes, aligned on 128 bytes. -// - https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-optimization-manual.pdf -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "x86_64")] -pub const ALIGNMENT: usize = 1 << 7; - -// 24Kc: -// Data Line Size -// - https://s3-eu-west-1.amazonaws.com/downloads-mips/documents/MD00346-2B-24K-DTS-04.00.pdf -// - https://gitlab.e.foundation/e/devices/samsung/n7100/stable_android_kernel_samsung_smdk4412/commit/2dbac10263b2f3c561de68b4c369bc679352ccee -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "mips")] -pub const ALIGNMENT: usize = 1 << 5; -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "mips64")] -pub const ALIGNMENT: usize = 1 << 5; - -// Defaults for powerpc -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "powerpc")] -pub const ALIGNMENT: usize = 1 << 5; - -// Defaults for the ppc 64 -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "powerpc64")] -pub const ALIGNMENT: usize = 1 << 6; - -// e.g.: sifive -// - https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/riscv/sifive-l2-cache.txt#L41 -// in general all of them are the same. -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "riscv")] -pub const ALIGNMENT: usize = 1 << 6; - -// This size is same across all hardware for this architecture. -// - https://docs.huihoo.com/doxygen/linux/kernel/3.7/arch_2s390_2include_2asm_2cache_8h.html -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "s390x")] -pub const ALIGNMENT: usize = 1 << 8; - -// This size is same across all hardware for this architecture. -// - https://docs.huihoo.com/doxygen/linux/kernel/3.7/arch_2sparc_2include_2asm_2cache_8h.html#a9400cc2ba37e33279bdbc510a6311fb4 -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "sparc")] -pub const ALIGNMENT: usize = 1 << 5; -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "sparc64")] -pub const ALIGNMENT: usize = 1 << 6; - -// On ARM cache line sizes are fixed. both v6 and v7. -// Need to add board specific or platform specific things later. -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "thumbv6")] -pub const ALIGNMENT: usize = 1 << 5; -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "thumbv7")] -pub const ALIGNMENT: usize = 1 << 5; - -// Operating Systems cache size determines this. -// Currently no way to determine this without runtime inference. -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "wasm32")] -pub const ALIGNMENT: usize = 1 << 6; - -// Same as v6 and v7. -// List goes like that: -// Cortex A, M, R, ARM v7, v7-M, Krait and NeoverseN uses this size. -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "arm")] -pub const ALIGNMENT: usize = 1 << 5; - -// Combined from 4 sectors. Volta says 128. -// Prevent chunk optimizations better to go to the default size. -// If you have smaller data with less padded functionality then use 32 with force option. -// - https://devtalk.nvidia.com/default/topic/803600/variable-cache-line-width-/ -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "nvptx")] -pub const ALIGNMENT: usize = 1 << 7; -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "nvptx64")] -pub const ALIGNMENT: usize = 1 << 7; - -// This size is same across all hardware for this architecture. -/// Cache and allocation multiple alignment size -#[cfg(target_arch = "aarch64")] -pub const ALIGNMENT: usize = 1 << 6; diff --git a/src/alloc/mod.rs b/src/alloc/mod.rs deleted file mode 100644 index deda8a83124..00000000000 --- a/src/alloc/mod.rs +++ /dev/null @@ -1,133 +0,0 @@ -// 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. - -//! Defines memory-related functions, such as allocate/deallocate/reallocate memory -//! regions, cache and allocation alignments. - -use std::mem::size_of; -use std::ptr::NonNull; -use std::{ - alloc::{handle_alloc_error, Layout}, - sync::atomic::AtomicIsize, -}; - -use crate::types::NativeType; - -mod alignment; - -pub use alignment::ALIGNMENT; - -// If this number is not zero after all objects have been `drop`, there is a memory leak -static mut ALLOCATIONS: AtomicIsize = AtomicIsize::new(0); - -/// Returns the total number of bytes allocated to buffers by the allocator. -pub fn total_allocated_bytes() -> isize { - unsafe { ALLOCATIONS.load(std::sync::atomic::Ordering::SeqCst) } -} - -/// # Safety -/// This pointer may only be used to check if memory is allocated. -#[inline] -pub unsafe fn dangling() -> NonNull { - NonNull::new_unchecked(ALIGNMENT as *mut T) -} - -/// 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(size: usize) -> NonNull { - unsafe { - if size == 0 { - dangling() - } else { - let size = size * size_of::(); - ALLOCATIONS.fetch_add(size as isize, std::sync::atomic::Ordering::SeqCst); - - let layout = Layout::from_size_align_unchecked(size, ALIGNMENT); - let raw_ptr = std::alloc::alloc(layout) as *mut T; - NonNull::new(raw_ptr).unwrap_or_else(|| handle_alloc_error(layout)) - } - } -} - -/// Allocates a cache-aligned memory region of `size` bytes with `0` on all of them. -/// This is more performant than using [allocate_aligned] and setting all bytes to zero -/// and is semantically similar to `calloc`. -pub fn allocate_aligned_zeroed(size: usize) -> NonNull { - unsafe { - if size == 0 { - dangling() - } else { - let size = size * size_of::(); - ALLOCATIONS.fetch_add(size as isize, std::sync::atomic::Ordering::SeqCst); - - let layout = Layout::from_size_align_unchecked(size, ALIGNMENT); - let raw_ptr = std::alloc::alloc_zeroed(layout) as *mut T; - NonNull::new(raw_ptr).unwrap_or_else(|| handle_alloc_error(layout)) - } - } -} - -/// Frees memory previously allocated by [`allocate_aligned_zeroed`] or [`allocate_aligned`]. -/// # Safety -/// This function is sound iff: -/// -/// * `ptr` was allocated by [`allocate_aligned_zeroed`] or [`allocate_aligned`] -/// * `size` must be the same size that was used to allocate that block of memory. -pub unsafe fn free_aligned(ptr: NonNull, size: usize) { - if size != 0 { - let size = size * size_of::(); - ALLOCATIONS.fetch_sub(size as isize, std::sync::atomic::Ordering::SeqCst); - std::alloc::dealloc( - ptr.as_ptr() as *mut u8, - Layout::from_size_align_unchecked(size, ALIGNMENT), - ); - } -} - -/// Reallocates memory previously allocated by [`allocate_aligned_zeroed`] or [`allocate_aligned`]. -/// # Safety -/// This function is sound iff `ptr` was previously allocated by `allocate_aligned` or `allocate_aligned_zeroed` for `old_size` items. -pub unsafe fn reallocate( - ptr: NonNull, - old_size: usize, - new_size: usize, -) -> NonNull { - if old_size == 0 { - return allocate_aligned(new_size); - } - - if new_size == 0 { - free_aligned(ptr, old_size); - return dangling(); - } - let old_size = old_size * size_of::(); - let new_size = new_size * size_of::(); - - ALLOCATIONS.fetch_add( - new_size as isize - old_size as isize, - std::sync::atomic::Ordering::SeqCst, - ); - let raw_ptr = std::alloc::realloc( - ptr.as_ptr() as *mut u8, - Layout::from_size_align_unchecked(old_size, ALIGNMENT), - new_size, - ) as *mut T; - NonNull::new(raw_ptr).unwrap_or_else(|| { - handle_alloc_error(Layout::from_size_align_unchecked(new_size, ALIGNMENT)) - }) -} diff --git a/src/bitmap/bitmap_ops.rs b/src/bitmap/bitmap_ops.rs index b402b717bf4..8e4fc88344b 100644 --- a/src/bitmap/bitmap_ops.rs +++ b/src/bitmap/bitmap_ops.rs @@ -112,9 +112,9 @@ where let iterator = iter.map(|left| op(left)).chain(std::iter::once(rem)); - let buffer = MutableBuffer::from_trusted_len_iter(iterator); + let buffer = MutableBuffer::from_chunk_iter(iterator); - Bitmap::from_u8_buffer(buffer.into(), length) + Bitmap::from_u8_buffer(buffer, length) } /// Apply a bitwise operation `op` to one input and return the result as a [`Bitmap`]. diff --git a/src/buffer/bytes.rs b/src/buffer/bytes.rs index eeb28ae1374..65a959a91ea 100644 --- a/src/buffer/bytes.rs +++ b/src/buffer/bytes.rs @@ -5,7 +5,6 @@ use std::slice; use std::{fmt::Debug, fmt::Formatter}; use std::{ptr::NonNull, sync::Arc}; -use crate::alloc; use crate::ffi; use crate::types::NativeType; @@ -79,11 +78,6 @@ impl Bytes { self.len } - #[inline] - pub fn is_empty(&self) -> bool { - self.len == 0 - } - #[inline] pub fn ptr(&self) -> NonNull { self.ptr @@ -94,9 +88,9 @@ impl Drop for Bytes { #[inline] fn drop(&mut self) { match &self.deallocation { - Deallocation::Native(capacity) => { - unsafe { alloc::free_aligned(self.ptr, *capacity) }; - } + Deallocation::Native(capacity) => unsafe { + Vec::from_raw_parts(self.ptr.as_ptr(), self.len, *capacity); + }, // foreign interface knows how to deallocate itself. Deallocation::Foreign(_) => (), } @@ -127,6 +121,6 @@ impl Debug for Bytes { } } -// This is sound because `Bytes` is an imutable container +// This is sound because `Bytes` is an immutable container unsafe impl Send for Bytes {} unsafe impl Sync for Bytes {} diff --git a/src/buffer/immutable.rs b/src/buffer/immutable.rs index 07f0d68239b..cd307434671 100644 --- a/src/buffer/immutable.rs +++ b/src/buffer/immutable.rs @@ -53,8 +53,7 @@ impl Buffer { } /// Auxiliary method to create a new Buffer - #[inline] - pub fn from_bytes(bytes: Bytes) -> Self { + pub(crate) fn from_bytes(bytes: Bytes) -> Self { let length = bytes.len(); Buffer { data: Arc::new(bytes), diff --git a/src/buffer/mod.rs b/src/buffer/mod.rs index bdb13195a3a..6a75438a4d1 100644 --- a/src/buffer/mod.rs +++ b/src/buffer/mod.rs @@ -6,7 +6,6 @@ mod immutable; mod mutable; pub(crate) mod bytes; -pub(crate) mod util; pub use immutable::Buffer; pub use mutable::MutableBuffer; diff --git a/src/buffer/mutable.rs b/src/buffer/mutable.rs index 9df74115723..6fb1bc08460 100644 --- a/src/buffer/mutable.rs +++ b/src/buffer/mutable.rs @@ -1,23 +1,14 @@ use std::iter::FromIterator; -use std::mem::size_of; use std::ptr::NonNull; use std::usize; -use crate::types::NativeType; -use crate::{alloc, trusted_len::TrustedLen}; +use crate::trusted_len::TrustedLen; +use crate::types::{BitChunk, NativeType}; -use super::{ - bytes::{Bytes, Deallocation}, - util, -}; +use super::bytes::{Bytes, Deallocation}; use super::immutable::Buffer; -#[inline] -fn capacity_multiple_of_64(capacity: usize) -> usize { - util::round_upto_multiple_of_64(capacity * size_of::()) / size_of::() -} - /// A [`MutableBuffer`] is this crates' interface to store types that are byte-like such as `i32`. /// It behaves like a [`Vec`], with the following differences: /// * memory is allocated along cache lines and in multiple of 64 bytes. @@ -34,11 +25,7 @@ fn capacity_multiple_of_64(capacity: usize) -> usize { /// assert_eq!(buffer.as_slice(), &[256, 1]) /// ``` pub struct MutableBuffer { - // dangling iff capacity = 0 - ptr: NonNull, - // invariant: len <= capacity - len: usize, - capacity: usize, + data: Vec, } impl std::fmt::Debug for MutableBuffer { @@ -57,23 +44,14 @@ impl MutableBuffer { /// Creates an empty [`MutableBuffer`]. This does not allocate in the heap. #[inline] pub fn new() -> Self { - let ptr = alloc::allocate_aligned(0); - Self { - ptr, - len: 0, - capacity: 0, - } + Self { data: Vec::new() } } /// Allocate a new [`MutableBuffer`] with initial capacity to be at least `capacity`. #[inline] pub fn with_capacity(capacity: usize) -> Self { - let capacity = capacity_multiple_of_64::(capacity); - let ptr = alloc::allocate_aligned(capacity); Self { - ptr, - len: 0, - capacity, + data: Vec::with_capacity(capacity), } } @@ -90,12 +68,8 @@ impl MutableBuffer { /// ``` #[inline] pub fn from_len_zeroed(len: usize) -> Self { - let new_capacity = capacity_multiple_of_64::(len); - let ptr = alloc::allocate_aligned_zeroed(new_capacity); Self { - ptr, - len, - capacity: new_capacity, + data: vec![T::default(); len], } } @@ -114,17 +88,7 @@ impl MutableBuffer { // exits. #[inline(always)] pub fn reserve(&mut self, additional: usize) { - let required_cap = self.len + additional; - if required_cap > self.capacity { - // JUSTIFICATION - // Benefit - // necessity - // Soundness - // `self.data` is valid for `self.capacity`. - let (ptr, new_capacity) = unsafe { reallocate(self.ptr, self.capacity, required_cap) }; - self.ptr = ptr; - self.capacity = new_capacity; - } + self.data.reserve(additional) } /// Resizes the buffer, either truncating its contents (with no change in capacity), or @@ -140,91 +104,66 @@ impl MutableBuffer { // exits. #[inline(always)] pub fn resize(&mut self, new_len: usize, value: T) { - if new_len > self.len { - if self.capacity == 0 && value == T::default() { - // edge case where the allocate - let required_cap = capacity_multiple_of_64::(new_len); - let ptr = alloc::allocate_aligned_zeroed(required_cap); - self.ptr = ptr; - self.capacity = required_cap; - self.len = new_len; - return; - } - - let diff = new_len - self.len; - self.reserve(diff); - unsafe { - // write the value - let mut ptr = self.ptr.as_ptr().add(self.len); - (0..diff).for_each(|_| { - std::ptr::write(ptr, value); - ptr = ptr.add(1); - }) - } - } - // this truncates the buffer when new_len < self.len - self.len = new_len; + self.data.resize(new_len, value) } /// Returns whether this buffer is empty. #[inline] pub fn is_empty(&self) -> bool { - self.len == 0 + self.data.is_empty() } /// Returns the length (the number of items) in this buffer. /// The invariant `buffer.len() <= buffer.capacity()` is always upheld. #[inline] pub fn len(&self) -> usize { - self.len + self.data.len() } /// Returns the total capacity in this buffer. /// The invariant `buffer.len() <= buffer.capacity()` is always upheld. #[inline] pub fn capacity(&self) -> usize { - self.capacity + self.data.capacity() } /// Clear all existing data from this buffer. #[inline] pub fn clear(&mut self) { - self.len = 0 + self.data.clear() } /// Shortens the buffer. /// If `len` is greater or equal to the buffers' current length, this has no effect. #[inline] pub fn truncate(&mut self, len: usize) { - if len < self.len { - self.len = len; - } + self.data.truncate(len) } /// Returns the data stored in this buffer as a slice. #[inline] pub fn as_slice(&self) -> &[T] { - self + self.data.as_slice() } /// Returns the data stored in this buffer as a mutable slice. #[inline] pub fn as_mut_slice(&mut self) -> &mut [T] { - self + self.data.as_mut_slice() } /// Returns a raw pointer to this buffer's internal memory /// This pointer is guaranteed to be aligned along cache-lines. #[inline] pub fn as_ptr(&self) -> *const T { - self.ptr.as_ptr() + self.data.as_ptr() } /// Returns a mutable raw pointer to this buffer's internal memory /// This pointer is guaranteed to be aligned along cache-lines. #[inline] pub fn as_mut_ptr(&mut self) -> *mut T { - self.ptr.as_ptr() + self.data.as_mut_ptr() } /// Extends this buffer from a slice of items, increasing its capacity if needed. @@ -237,14 +176,7 @@ impl MutableBuffer { /// ``` #[inline] pub fn extend_from_slice(&mut self, items: &[T]) { - let additional = items.len(); - self.reserve(additional); - unsafe { - let dst = self.ptr.as_ptr().add(self.len); - let src = items.as_ptr(); - std::ptr::copy_nonoverlapping(src, dst, additional) - } - self.len += additional; + self.data.extend_from_slice(items) } /// Pushes a new item to the buffer, increasing its capacity if needed. @@ -257,12 +189,7 @@ impl MutableBuffer { /// ``` #[inline] pub fn push(&mut self, item: T) { - self.reserve(1); - unsafe { - let dst = self.ptr.as_ptr().add(self.len) as *mut T; - std::ptr::write(dst, item); - } - self.len += 1; + self.data.push(item) } /// Extends the buffer with a new item without checking for sufficient capacity @@ -270,9 +197,9 @@ impl MutableBuffer { /// Caller must ensure that `self.capacity() - self.len() >= 1` #[inline] pub(crate) unsafe fn push_unchecked(&mut self, item: T) { - let dst = self.ptr.as_ptr().add(self.len); + let dst = self.as_mut_ptr().add(self.len()); std::ptr::write(dst, item); - self.len += 1; + self.data.set_len(self.data.len() + 1); } /// Sets the length of this buffer. @@ -284,7 +211,7 @@ impl MutableBuffer { #[inline] pub unsafe fn set_len(&mut self, len: usize) { assert!(len <= self.capacity()); - self.len = len; + self.data.set_len(len); } /// Extends this buffer by `additional` items of value `value`. @@ -309,61 +236,13 @@ impl MutableBuffer { /// assert!(buffer.capacity() == 8); /// ``` pub fn shrink_to_fit(&mut self) { - let new_capacity = capacity_multiple_of_64::(self.len); - if new_capacity < self.capacity { - // JUSTIFICATION - // Benefit - // necessity - // Soundness - // `self.ptr` is valid for `self.capacity`. - let ptr = unsafe { alloc::reallocate(self.ptr, self.capacity, new_capacity) }; - - self.ptr = ptr; - self.capacity = new_capacity; - } + self.data.shrink_to_fit(); } } -/// # Safety -/// `ptr` must be allocated for `old_capacity`. -#[inline] -unsafe fn reallocate( - ptr: NonNull, - old_capacity: usize, - new_capacity: usize, -) -> (NonNull, usize) { - let new_capacity = capacity_multiple_of_64::(new_capacity); - let new_capacity = std::cmp::max(new_capacity, old_capacity * 2); - let ptr = alloc::reallocate(ptr, old_capacity, new_capacity); - (ptr, new_capacity) -} - impl Extend for MutableBuffer { fn extend>(&mut self, iter: T) { - let mut iterator = iter.into_iter(); - let (lower, _) = iterator.size_hint(); - let additional = lower; - self.reserve(additional); - - // this is necessary because of https://github.com/rust-lang/rust/issues/32155 - let mut len = SetLenOnDrop::new(&mut self.len); - let mut dst = unsafe { self.ptr.as_ptr().add(len.local_len) as *mut A }; - let capacity = self.capacity; - - while len.local_len < capacity { - if let Some(item) = iterator.next() { - unsafe { - std::ptr::write(dst, item); - dst = dst.add(1); - } - len.local_len += 1; - } else { - break; - } - } - drop(len); - - iterator.for_each(|item| self.push(item)); + self.data.extend(iter) } } @@ -388,19 +267,21 @@ impl MutableBuffer { let upper = upper.expect("trusted_len_iter requires an upper limit"); let len = upper; + let self_len = self.len(); + self.reserve(len); - let mut dst = self.ptr.as_ptr().add(self.len); + let mut dst = self.as_mut_ptr().add(self_len); for item in iterator { // note how there is no reserve here (compared with `extend_from_iter`) std::ptr::write(dst, item); dst = dst.add(1); } assert_eq!( - dst.offset_from(self.ptr.as_ptr().add(self.len)) as usize, + dst.offset_from(self.as_ptr().add(self_len)) as usize, upper, "Trusted iterator length was not accurately reported" ); - self.len += len; + self.set_len(self_len + len); } /// Creates a [`MutableBuffer`] from an [`Iterator`] with a trusted (upper) length. @@ -474,41 +355,25 @@ impl MutableBuffer { let mut buffer = MutableBuffer::with_capacity(len); - let mut dst = buffer.ptr.as_ptr(); + let mut dst = buffer.as_mut_ptr(); for item in iterator { std::ptr::write(dst, item?); dst = dst.add(1); } assert_eq!( - dst.offset_from(buffer.ptr.as_ptr()) as usize, + dst.offset_from(buffer.as_ptr()) as usize, upper, "Trusted iterator length was not accurately reported" ); - buffer.len = len; + buffer.set_len(len); Ok(buffer) } } impl FromIterator for MutableBuffer { fn from_iter>(iter: I) -> Self { - let mut iterator = iter.into_iter(); - - // first iteration, which will likely reserve sufficient space for the buffer. - let mut buffer = match iterator.next() { - None => MutableBuffer::new(), - Some(element) => { - let (lower, _) = iterator.size_hint(); - let mut buffer = MutableBuffer::with_capacity(lower.saturating_add(1)); - unsafe { - std::ptr::write(buffer.as_mut_ptr(), element); - buffer.len = 1; - } - buffer - } - }; - - buffer.extend(iterator); - buffer + let data = Vec::from_iter(iter); + Self { data } } } @@ -523,42 +388,14 @@ impl std::ops::Deref for MutableBuffer { #[inline] fn deref(&self) -> &[T] { - unsafe { std::slice::from_raw_parts(self.as_ptr(), self.len) } + &self.data } } impl std::ops::DerefMut for MutableBuffer { #[inline] fn deref_mut(&mut self) -> &mut [T] { - unsafe { std::slice::from_raw_parts_mut(self.as_mut_ptr(), self.len) } - } -} - -impl Drop for MutableBuffer { - fn drop(&mut self) { - unsafe { alloc::free_aligned(self.ptr, self.capacity) }; - } -} - -struct SetLenOnDrop<'a> { - len: &'a mut usize, - local_len: usize, -} - -impl<'a> SetLenOnDrop<'a> { - #[inline] - fn new(len: &'a mut usize) -> Self { - SetLenOnDrop { - local_len: *len, - len, - } - } -} - -impl Drop for SetLenOnDrop<'_> { - #[inline] - fn drop(&mut self) { - *self.len = self.local_len; + &mut self.data } } @@ -574,53 +411,61 @@ impl> From

for MutableBuffer { impl From> for Buffer { #[inline] fn from(buffer: MutableBuffer) -> Self { - Buffer::from_bytes(buffer.into()) + Self::from_bytes(buffer.into()) } } impl From> for Bytes { #[inline] fn from(buffer: MutableBuffer) -> Self { - let result = unsafe { - Bytes::new( - buffer.ptr, - buffer.len, - Deallocation::Native(buffer.capacity), - ) - }; + let mut data = buffer.data; + let ptr = NonNull::new(data.as_mut_ptr()).unwrap(); + let len = data.len(); + let capacity = data.capacity(); + + let result = unsafe { Bytes::new(ptr, len, Deallocation::Native(capacity)) }; // so that the memory region is not deallocated. - std::mem::forget(buffer); + std::mem::forget(data); result } } -impl From> for MutableBuffer { - #[inline] - fn from(buffer: MutableBuffer) -> Self { - let ratio = std::mem::size_of::() / std::mem::size_of::(); - - let capacity = buffer.capacity * ratio; - let len = buffer.len * ratio; - let ptr = unsafe { NonNull::new_unchecked(buffer.ptr.as_ptr() as *mut u8) }; - // so that the memory region is not deallocated; ownership was transfered - std::mem::forget(buffer); - Self { ptr, len, capacity } - } -} - impl MutableBuffer { /// Creates a [`MutableBuffer`] from an iterator of `u64`. #[inline] - pub fn from_chunk_iter>(iter: I) -> Self { - MutableBuffer::from_trusted_len_iter(iter).into() + pub fn from_chunk_iter>(iter: I) -> Self { + // TrustedLen + unsafe { Self::from_chunk_iter_unchecked(iter) } } /// # Safety /// This method assumes that the iterator's size is correct and is undefined behavior /// to use it on an iterator that reports an incorrect length. #[inline] - pub unsafe fn from_chunk_iter_unchecked>(iter: I) -> Self { - MutableBuffer::from_trusted_len_iter_unchecked(iter).into() + pub unsafe fn from_chunk_iter_unchecked>( + iterator: I, + ) -> Self { + let (_, upper) = iterator.size_hint(); + let upper = upper.expect("try_from_trusted_len_iter requires an upper limit"); + let len = upper * std::mem::size_of::(); + + let mut buffer = MutableBuffer::with_capacity(len); + + let mut dst = buffer.as_mut_ptr(); + for item in iterator { + let bytes = item.to_ne_bytes(); + for i in 0..std::mem::size_of::() { + std::ptr::write(dst, bytes[i]); + dst = dst.add(1); + } + } + assert_eq!( + dst.offset_from(buffer.as_ptr()) as usize, + len, + "Trusted iterator length was not accurately reported" + ); + buffer.set_len(len); + buffer } } diff --git a/src/buffer/util.rs b/src/buffer/util.rs deleted file mode 100644 index 7cd1324f196..00000000000 --- a/src/buffer/util.rs +++ /dev/null @@ -1,12 +0,0 @@ -/// Returns the nearest number that is `>=` than `num` and is a multiple of 64 -#[inline] -pub fn round_upto_multiple_of_64(num: usize) -> usize { - round_upto_power_of_2(num, 64) -} - -/// Returns the nearest multiple of `factor` that is `>=` than `num`. Here `factor` must -/// be a power of 2. -pub fn round_upto_power_of_2(num: usize, factor: usize) -> usize { - debug_assert!(factor > 0 && (factor & (factor - 1)) == 0); - (num + (factor - 1)) & !(factor - 1) -} diff --git a/src/lib.rs b/src/lib.rs index d2833904d4d..70ef59f2ef3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,5 @@ //! Doc provided by README -pub mod alloc; #[macro_use] pub mod array; pub mod bitmap; @@ -16,7 +15,6 @@ pub mod compute; pub mod io; pub mod record_batch; pub mod temporal_conversions; -pub use alloc::total_allocated_bytes; pub mod datatypes; diff --git a/src/types/bit_chunk.rs b/src/types/bit_chunk.rs index c3b1768454e..76cb1e71b7a 100644 --- a/src/types/bit_chunk.rs +++ b/src/types/bit_chunk.rs @@ -3,12 +3,15 @@ use std::{ ops::{BitAnd, BitAndAssign, BitOr, Not, Shl, ShlAssign, ShrAssign}, }; -/// Something that can be use as a chunk of bits. This is used to create masks of a given number -/// of lengths, whose width is `1`. In `simd_packed` notation, this corresponds to `m1xY`. +use super::NativeType; + +/// Something that can be use as a chunk of bits. This is used to create masks ofa given number +/// of length, whose width is `1`. In `simd_packed` notation, this corresponds to `m1xY`. /// # Safety /// Do not implement. pub unsafe trait BitChunk: Sized + + NativeType + Copy + std::fmt::Debug + Binary @@ -22,12 +25,6 @@ pub unsafe trait BitChunk: + BitAndAssign + BitOr { - /// The representation of this type in the stack. - type Bytes: std::ops::Index - + std::ops::IndexMut - + for<'a> std::convert::TryFrom<&'a [u8]> - + std::fmt::Debug; - /// A value with a single bit set at the most right position. fn one() -> Self; /// A value with no bits set. @@ -39,8 +36,6 @@ pub unsafe trait BitChunk: } unsafe impl BitChunk for u8 { - type Bytes = [u8; 1]; - #[inline(always)] fn zero() -> Self { 0 @@ -63,8 +58,6 @@ unsafe impl BitChunk for u8 { } unsafe impl BitChunk for u16 { - type Bytes = [u8; 2]; - #[inline(always)] fn zero() -> Self { 0 @@ -87,8 +80,6 @@ unsafe impl BitChunk for u16 { } unsafe impl BitChunk for u32 { - type Bytes = [u8; 4]; - #[inline(always)] fn zero() -> Self { 0 @@ -111,8 +102,6 @@ unsafe impl BitChunk for u32 { } unsafe impl BitChunk for u64 { - type Bytes = [u8; 8]; - #[inline(always)] fn zero() -> Self { 0 diff --git a/src/types/mod.rs b/src/types/mod.rs index 8e120ee274a..5f041e9107a 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -67,12 +67,20 @@ pub unsafe trait NativeType: + Sized + 'static { - /// Type denoting its representation as bytes - type Bytes: AsRef<[u8]> + for<'a> TryFrom<&'a [u8]>; + /// Type denoting its representation as bytes. + /// This must be `[u8; N]` where `N = size_of::`. + type Bytes: AsRef<[u8]> + + std::ops::Index + + std::ops::IndexMut + + for<'a> TryFrom<&'a [u8]> + + std::fmt::Debug; /// To bytes in little endian fn to_le_bytes(&self) -> Self::Bytes; + /// To bytes in native endian + fn to_ne_bytes(&self) -> Self::Bytes; + /// To bytes in big endian fn to_be_bytes(&self) -> Self::Bytes; @@ -94,6 +102,11 @@ macro_rules! native { Self::to_be_bytes(*self) } + #[inline] + fn to_ne_bytes(&self) -> Self::Bytes { + Self::to_ne_bytes(*self) + } + #[inline] fn from_be_bytes(bytes: Self::Bytes) -> Self { Self::from_be_bytes(bytes) @@ -172,6 +185,22 @@ unsafe impl NativeType for days_ms { result } + #[inline] + fn to_ne_bytes(&self) -> Self::Bytes { + let days = self.0[0].to_ne_bytes(); + let ms = self.0[1].to_ne_bytes(); + let mut result = [0; 8]; + result[0] = days[0]; + result[1] = days[1]; + result[2] = days[2]; + result[3] = days[3]; + result[4] = ms[0]; + result[5] = ms[1]; + result[6] = ms[2]; + result[7] = ms[3]; + result + } + #[inline] fn to_be_bytes(&self) -> Self::Bytes { let days = self.0[0].to_be_bytes(); @@ -260,6 +289,26 @@ unsafe impl NativeType for months_days_ns { result } + #[inline] + fn to_ne_bytes(&self) -> Self::Bytes { + let months = self.months().to_ne_bytes(); + let days = self.days().to_ne_bytes(); + let ns = self.ns().to_ne_bytes(); + let mut result = [0; 16]; + result[0] = months[0]; + result[1] = months[1]; + result[2] = months[2]; + result[3] = months[3]; + result[4] = days[0]; + result[5] = days[1]; + result[6] = days[2]; + result[7] = days[3]; + (0..8).for_each(|i| { + result[8 + i] = ns[i]; + }); + result + } + #[inline] fn to_be_bytes(&self) -> Self::Bytes { let months = self.months().to_be_bytes(); diff --git a/tests/it/alloc.rs b/tests/it/alloc.rs deleted file mode 100644 index bfd35bcae1c..00000000000 --- a/tests/it/alloc.rs +++ /dev/null @@ -1,49 +0,0 @@ -use arrow2::alloc::*; - -#[test] -fn allocate_dangling() { - let p = allocate_aligned::(0); - assert_eq!(0, (p.as_ptr() as usize) % ALIGNMENT); -} - -#[test] -fn allocate() { - let p = allocate_aligned::(1024); - assert_eq!(0, (p.as_ptr() as usize) % ALIGNMENT); - unsafe { free_aligned(p, 1024) }; -} - -#[test] -fn allocate_zeroed() { - let p = allocate_aligned_zeroed::(1024); - assert_eq!(0, (p.as_ptr() as usize) % ALIGNMENT); - unsafe { free_aligned(p, 1024) }; -} - -#[test] -fn reallocate_from_zero() { - let ptr = allocate_aligned::(0); - let ptr = unsafe { reallocate(ptr, 0, 512) }; - unsafe { free_aligned(ptr, 512) }; -} - -#[test] -fn reallocate_from_alloc() { - let ptr = allocate_aligned::(32); - let ptr = unsafe { reallocate(ptr, 32, 64) }; - unsafe { free_aligned(ptr, 64) }; -} - -#[test] -fn reallocate_smaller() { - let ptr = allocate_aligned::(32); - let ptr = unsafe { reallocate(ptr, 32, 16) }; - unsafe { free_aligned(ptr, 16) }; -} - -#[test] -fn reallocate_to_zero() { - let ptr = allocate_aligned::(32); - let ptr = unsafe { reallocate(ptr, 32, 0) }; - assert_eq!(ptr, unsafe { dangling() }); -} diff --git a/tests/it/array/utf8/mutable.rs b/tests/it/array/utf8/mutable.rs index c7ee553ee5e..ea0da841a2f 100644 --- a/tests/it/array/utf8/mutable.rs +++ b/tests/it/array/utf8/mutable.rs @@ -7,8 +7,8 @@ use arrow2::datatypes::DataType; fn capacities() { let b = MutableUtf8Array::::with_capacities(1, 10); - assert_eq!(b.values().capacity(), 64); - assert_eq!(b.offsets().capacity(), 16); // 64 bytes + assert!(b.values().capacity() >= 10); + assert!(b.offsets().capacity() >= 2); } #[test] diff --git a/tests/it/bitmap/mutable.rs b/tests/it/bitmap/mutable.rs index f5a973547be..3c9af618813 100644 --- a/tests/it/bitmap/mutable.rs +++ b/tests/it/bitmap/mutable.rs @@ -75,14 +75,7 @@ fn push_exact_ones() { #[test] fn capacity() { let b = MutableBitmap::with_capacity(10); - assert_eq!(b.capacity(), 512); - - let b = MutableBitmap::with_capacity(512); - assert_eq!(b.capacity(), 512); - - let mut b = MutableBitmap::with_capacity(512); - b.reserve(8); - assert_eq!(b.capacity(), 512); + assert!(b.capacity() >= 10); } #[test] diff --git a/tests/it/buffer/mutable.rs b/tests/it/buffer/mutable.rs index cb6134b0807..ba26f4a6ebc 100644 --- a/tests/it/buffer/mutable.rs +++ b/tests/it/buffer/mutable.rs @@ -71,12 +71,7 @@ fn push() { #[test] fn capacity() { let b = MutableBuffer::::with_capacity(10); - assert_eq!(b.capacity(), 64 / std::mem::size_of::()); - let b = MutableBuffer::::with_capacity(16); - assert_eq!(b.capacity(), 16); - - let b = MutableBuffer::::with_capacity(64); - assert!(b.capacity() >= 64); + assert!(b.capacity() >= 10); let mut b = MutableBuffer::::with_capacity(16); b.reserve(4); diff --git a/tests/it/main.rs b/tests/it/main.rs index ad9a9b97353..b791184608a 100644 --- a/tests/it/main.rs +++ b/tests/it/main.rs @@ -1,4 +1,3 @@ -mod alloc; mod array; mod bitmap; mod buffer;