You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by jo...@apache.org on 2020/11/21 22:29:13 UTC
[arrow] branch master updated: ARROW-10682: [Rust] Improve sort
kernel performance by enabling inlining of is_valid calls
This is an automated email from the ASF dual-hosted git repository.
jorgecarleitao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new cc6158d ARROW-10682: [Rust] Improve sort kernel performance by enabling inlining of is_valid calls
cc6158d is described below
commit cc6158d326df1b2b159473ce997b5e7e310386ed
Author: Jörn Horstmann <gi...@jhorstmann.net>
AuthorDate: Sat Nov 21 23:27:58 2020 +0100
ARROW-10682: [Rust] Improve sort kernel performance by enabling inlining of is_valid calls
Benchmark results on an aws t3 instance (cascadelake):
```
$ cargo bench --bench sort_kernel
sort 2^10 time: [168.90 us 168.98 us 169.08 us]
change: [-59.182% -59.041% -58.824%] (p = 0.00 < 0.05)
Performance has improved.
sort 2^12 time: [839.88 us 840.15 us 840.46 us]
change: [-57.095% -57.040% -56.947%] (p = 0.00 < 0.05)
Performance has improved.
sort nulls 2^10 time: [148.69 us 148.75 us 148.82 us]
change: [-66.823% -66.727% -66.633%] (p = 0.00 < 0.05)
Performance has improved.
sort nulls 2^12 time: [721.79 us 721.95 us 722.12 us]
change: [-65.966% -65.920% -65.845%] (p = 0.00 < 0.05)
Performance has improved.
```
Closes #8736 from jhorstmann/ARROW-10682-improve-sort-kernel-performance
Authored-by: Jörn Horstmann <gi...@jhorstmann.net>
Signed-off-by: Jorge C. Leitao <jo...@gmail.com>
---
rust/arrow/src/array/array.rs | 4 ++--
rust/arrow/src/array/ord.rs | 10 +++++-----
rust/arrow/src/compute/kernels/sort.rs | 33 +++++++++++++++++----------------
3 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/rust/arrow/src/array/array.rs b/rust/arrow/src/array/array.rs
index 4ff5a8d..9c12078 100644
--- a/rust/arrow/src/array/array.rs
+++ b/rust/arrow/src/array/array.rs
@@ -157,7 +157,7 @@ pub trait Array: fmt::Debug + Send + Sync + JsonEqual {
/// assert_eq!(array.is_null(1), true);
/// ```
fn is_null(&self, index: usize) -> bool {
- self.data().is_null(index)
+ self.data_ref().is_null(index)
}
/// Returns whether the element at `index` is not null.
@@ -174,7 +174,7 @@ pub trait Array: fmt::Debug + Send + Sync + JsonEqual {
/// assert_eq!(array.is_valid(1), false);
/// ```
fn is_valid(&self, index: usize) -> bool {
- self.data().is_valid(index)
+ self.data_ref().is_valid(index)
}
/// Returns the total number of null values in this array.
diff --git a/rust/arrow/src/array/ord.rs b/rust/arrow/src/array/ord.rs
index 8ff5a3d..22cfa52 100644
--- a/rust/arrow/src/array/ord.rs
+++ b/rust/arrow/src/array/ord.rs
@@ -31,11 +31,11 @@ pub type DynComparator<'a> = Box<dyn Fn(usize, usize) -> Ordering + 'a>;
/// compares two floats, placing NaNs at last
fn cmp_nans_last<T: Float>(a: &T, b: &T) -> Ordering {
- match (a, b) {
- (x, y) if x.is_nan() && y.is_nan() => Ordering::Equal,
- (x, _) if x.is_nan() => Ordering::Greater,
- (_, y) if y.is_nan() => Ordering::Less,
- (_, _) => a.partial_cmp(b).unwrap(),
+ match (a.is_nan(), b.is_nan()) {
+ (true, true) => Ordering::Equal,
+ (true, false) => Ordering::Greater,
+ (false, true) => Ordering::Less,
+ _ => a.partial_cmp(b).unwrap(),
}
}
diff --git a/rust/arrow/src/compute/kernels/sort.rs b/rust/arrow/src/compute/kernels/sort.rs
index f0560ca..915d280 100644
--- a/rust/arrow/src/compute/kernels/sort.rs
+++ b/rust/arrow/src/compute/kernels/sort.rs
@@ -486,26 +486,27 @@ pub fn lexsort_to_indices(columns: &[SortColumn]) -> Result<UInt32Array> {
));
};
- // convert ArrayRefs to OrdArray trait objects and perform row count check
+ // map to data and DynComparator
let flat_columns = columns
.iter()
- .map(|column| -> Result<(&Array, DynComparator, SortOptions)> {
- // flatten and convert build comparators
- Ok((
- column.values.as_ref(),
- build_compare(column.values.as_ref(), column.values.as_ref())?,
- column.options.unwrap_or_default(),
- ))
- })
- .collect::<Result<Vec<(&Array, DynComparator, SortOptions)>>>()?;
+ .map(
+ |column| -> Result<(&ArrayDataRef, DynComparator, SortOptions)> {
+ // flatten and convert build comparators
+ // use ArrayData for is_valid checks later to avoid dynamic call
+ let values = column.values.as_ref();
+ let data = values.data_ref();
+ Ok((
+ data,
+ build_compare(values, values)?,
+ column.options.unwrap_or_default(),
+ ))
+ },
+ )
+ .collect::<Result<Vec<(&ArrayDataRef, DynComparator, SortOptions)>>>()?;
let lex_comparator = |a_idx: &usize, b_idx: &usize| -> Ordering {
- for column in flat_columns.iter() {
- let values = &column.0;
- let comparator = &column.1;
- let sort_option = column.2;
-
- match (values.is_valid(*a_idx), values.is_valid(*b_idx)) {
+ for (data, comparator, sort_option) in flat_columns.iter() {
+ match (data.is_valid(*a_idx), data.is_valid(*b_idx)) {
(true, true) => {
match (comparator)(*a_idx, *b_idx) {
// equal, move on to next column