Skip to content

Commit

Permalink
perf: elide redundant bound checks. (#13909)
Browse files Browse the repository at this point in the history
  • Loading branch information
ritchie46 committed Jan 22, 2024
1 parent bb2d235 commit dfcf702
Show file tree
Hide file tree
Showing 31 changed files with 165 additions and 512 deletions.
6 changes: 4 additions & 2 deletions crates/polars-arrow/src/array/growable/binary.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::sync::Arc;

use polars_utils::slice::GetSaferUnchecked;

use super::utils::extend_offset_values;
use super::Growable;
use crate::array::growable::utils::{extend_validity, prepare_validity};
Expand Down Expand Up @@ -55,8 +57,8 @@ impl<'a, O: Offset> GrowableBinary<'a, O> {
}

impl<'a, O: Offset> Growable<'a> for GrowableBinary<'a, O> {
fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = self.arrays[index];
unsafe fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = *self.arrays.get_unchecked_release(index);
extend_validity(&mut self.validity, array, start, len);

let offsets = array.offsets();
Expand Down
3 changes: 1 addition & 2 deletions crates/polars-arrow/src/array/growable/binview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ impl<'a, T: ViewType + ?Sized> GrowableBinaryViewArray<'a, T> {
}

impl<'a, T: ViewType + ?Sized> Growable<'a> for GrowableBinaryViewArray<'a, T> {
fn extend(&mut self, index: usize, start: usize, len: usize) {
assert!(index < self.arrays.len());
unsafe fn extend(&mut self, index: usize, start: usize, len: usize) {
unsafe { self.extend_unchecked(index, start, len) }
}

Expand Down
6 changes: 4 additions & 2 deletions crates/polars-arrow/src/array/growable/boolean.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::sync::Arc;

use polars_utils::slice::GetSaferUnchecked;

use super::Growable;
use crate::array::growable::utils::{extend_validity, prepare_validity};
use crate::array::{Array, BooleanArray};
Expand Down Expand Up @@ -48,8 +50,8 @@ impl<'a> GrowableBoolean<'a> {
}

impl<'a> Growable<'a> for GrowableBoolean<'a> {
fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = self.arrays[index];
unsafe fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = *self.arrays.get_unchecked_release(index);
extend_validity(&mut self.validity, array, start, len);

let values = array.values();
Expand Down
14 changes: 9 additions & 5 deletions crates/polars-arrow/src/array/growable/dictionary.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::sync::Arc;

use polars_utils::slice::GetSaferUnchecked;

use super::{make_growable, Growable};
use crate::array::growable::utils::{extend_validity, prepare_validity};
use crate::array::{Array, DictionaryArray, DictionaryKey, PrimitiveArray};
Expand Down Expand Up @@ -28,7 +30,7 @@ fn concatenate_values<K: DictionaryKey>(
let mut offsets = Vec::with_capacity(arrays_keys.len() + 1);
offsets.push(0);
for (i, values) in arrays_values.iter().enumerate() {
mutable.extend(i, 0, values.len());
unsafe { mutable.extend(i, 0, values.len()) };
offsets.push(offsets[i] + values.len());
}
(mutable.as_box(), offsets)
Expand Down Expand Up @@ -94,12 +96,14 @@ impl<'a, T: DictionaryKey> GrowableDictionary<'a, T> {

impl<'a, T: DictionaryKey> Growable<'a> for GrowableDictionary<'a, T> {
#[inline]
fn extend(&mut self, index: usize, start: usize, len: usize) {
let keys_array = self.keys[index];
unsafe fn extend(&mut self, index: usize, start: usize, len: usize) {
let keys_array = *self.keys.get_unchecked_release(index);
extend_validity(&mut self.validity, keys_array, start, len);

let values = &keys_array.values()[start..start + len];
let offset = self.offsets[index];
let values = &keys_array
.values()
.get_unchecked_release(start..start + len);
let offset = self.offsets.get_unchecked_release(index);
self.key_values.extend(
values
.iter()
Expand Down
11 changes: 7 additions & 4 deletions crates/polars-arrow/src/array/growable/fixed_binary.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::sync::Arc;

use polars_utils::slice::GetSaferUnchecked;

use super::Growable;
use crate::array::growable::utils::{extend_validity, prepare_validity};
use crate::array::{Array, FixedSizeBinaryArray};
Expand Down Expand Up @@ -50,14 +52,15 @@ impl<'a> GrowableFixedSizeBinary<'a> {
}

impl<'a> Growable<'a> for GrowableFixedSizeBinary<'a> {
fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = self.arrays[index];
unsafe fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = *self.arrays.get_unchecked_release(index);
extend_validity(&mut self.validity, array, start, len);

let values = array.values();

self.values
.extend_from_slice(&values[start * self.size..start * self.size + len * self.size]);
self.values.extend_from_slice(
values.get_unchecked_release(start * self.size..start * self.size + len * self.size),
);
}

fn extend_validity(&mut self, additional: usize) {
Expand Down
6 changes: 4 additions & 2 deletions crates/polars-arrow/src/array/growable/fixed_size_list.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::sync::Arc;

use polars_utils::slice::GetSaferUnchecked;

use super::{make_growable, Growable};
use crate::array::growable::utils::{extend_validity, prepare_validity};
use crate::array::{Array, FixedSizeListArray};
Expand Down Expand Up @@ -66,8 +68,8 @@ impl<'a> GrowableFixedSizeList<'a> {
}

impl<'a> Growable<'a> for GrowableFixedSizeList<'a> {
fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = self.arrays[index];
unsafe fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = *self.arrays.get_unchecked_release(index);
extend_validity(&mut self.validity, array, start, len);

self.values
Expand Down
15 changes: 10 additions & 5 deletions crates/polars-arrow/src/array/growable/list.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use std::sync::Arc;

use polars_utils::slice::GetSaferUnchecked;

use super::{make_growable, Growable};
use crate::array::growable::utils::{extend_validity, prepare_validity};
use crate::array::{Array, ListArray};
use crate::bitmap::MutableBitmap;
use crate::offset::{Offset, Offsets};

fn extend_offset_values<O: Offset>(
unsafe fn extend_offset_values<O: Offset>(
growable: &mut GrowableList<'_, O>,
index: usize,
start: usize,
Expand All @@ -20,8 +22,11 @@ fn extend_offset_values<O: Offset>(
.try_extend_from_slice(offsets, start, len)
.unwrap();

let end = offsets.buffer()[start + len].to_usize();
let start = offsets.buffer()[start].to_usize();
let end = offsets
.buffer()
.get_unchecked_release(start + len)
.to_usize();
let start = offsets.buffer().get_unchecked_release(start).to_usize();
let len = end - start;
growable.values.extend(index, start, len);
}
Expand Down Expand Up @@ -74,8 +79,8 @@ impl<'a, O: Offset> GrowableList<'a, O> {
}

impl<'a, O: Offset> Growable<'a> for GrowableList<'a, O> {
fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = self.arrays[index];
unsafe fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = *self.arrays.get_unchecked_release(index);
extend_validity(&mut self.validity, array, start, len);
extend_offset_values::<O>(self, index, start, len);
}
Expand Down
103 changes: 0 additions & 103 deletions crates/polars-arrow/src/array/growable/map.rs

This file was deleted.

21 changes: 6 additions & 15 deletions crates/polars-arrow/src/array/growable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ use crate::datatypes::*;

mod binary;
pub use binary::GrowableBinary;
mod union;
pub use union::GrowableUnion;
mod boolean;
pub use boolean::GrowableBoolean;
mod fixed_binary;
Expand All @@ -20,8 +18,6 @@ mod primitive;
pub use primitive::GrowablePrimitive;
mod list;
pub use list::GrowableList;
mod map;
pub use map::GrowableMap;
mod structure;
pub use structure::GrowableStruct;
mod fixed_size_list;
Expand All @@ -41,11 +37,13 @@ mod utils;
pub trait Growable<'a> {
/// Extends this [`Growable`] with elements from the bounded [`Array`] at index `index` from
/// a slice starting at `start` and length `len`.
/// # Panic
/// This function panics if the range is out of bounds, i.e. if `start + len >= array.len()`.
fn extend(&mut self, index: usize, start: usize, len: usize);
/// # Safety
/// Doesn't do any bound checks
unsafe fn extend(&mut self, index: usize, start: usize, len: usize);

/// Extends this [`Growable`] with null elements, disregarding the bound arrays
/// # Safety
/// Doesn't do any bound checks
fn extend_validity(&mut self, additional: usize);

/// The current length of the [`Growable`].
Expand Down Expand Up @@ -155,13 +153,6 @@ pub fn make_growable<'a>(
))
})
},
Map => dyn_growable!(map::GrowableMap, arrays, use_validity, capacity),
Union => {
let arrays = arrays
.iter()
.map(|array| array.as_any().downcast_ref().unwrap())
.collect::<Vec<_>>();
Box::new(union::GrowableUnion::new(arrays, capacity))
},
Union | Map => unimplemented!(),
}
}
2 changes: 1 addition & 1 deletion crates/polars-arrow/src/array/growable/null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl GrowableNull {
}

impl<'a> Growable<'a> for GrowableNull {
fn extend(&mut self, _: usize, _: usize, len: usize) {
unsafe fn extend(&mut self, _: usize, _: usize, len: usize) {
self.length += len;
}

Expand Down
9 changes: 6 additions & 3 deletions crates/polars-arrow/src/array/growable/primitive.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::sync::Arc;

use polars_utils::slice::GetSaferUnchecked;

use super::Growable;
use crate::array::growable::utils::{extend_validity, prepare_validity};
use crate::array::{Array, PrimitiveArray};
Expand Down Expand Up @@ -55,12 +57,13 @@ impl<'a, T: NativeType> GrowablePrimitive<'a, T> {

impl<'a, T: NativeType> Growable<'a> for GrowablePrimitive<'a, T> {
#[inline]
fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = self.arrays[index];
unsafe fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = *self.arrays.get_unchecked_release(index);
extend_validity(&mut self.validity, array, start, len);

let values = array.values().as_slice();
self.values.extend_from_slice(&values[start..start + len]);
self.values
.extend_from_slice(values.get_unchecked_release(start..start + len));
}

#[inline]
Expand Down
6 changes: 4 additions & 2 deletions crates/polars-arrow/src/array/growable/structure.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::sync::Arc;

use polars_utils::slice::GetSaferUnchecked;

use super::{make_growable, Growable};
use crate::array::growable::utils::{extend_validity, prepare_validity};
use crate::array::{Array, StructArray};
Expand Down Expand Up @@ -65,8 +67,8 @@ impl<'a> GrowableStruct<'a> {
}

impl<'a> Growable<'a> for GrowableStruct<'a> {
fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = self.arrays[index];
unsafe fn extend(&mut self, index: usize, start: usize, len: usize) {
let array = *self.arrays.get_unchecked_release(index);
extend_validity(&mut self.validity, array, start, len);

if array.null_count() == 0 {
Expand Down
Loading

0 comments on commit dfcf702

Please sign in to comment.