Speed up the function min_max_string (#1374)

* clean up the code

Signed-off-by: remzi <[email protected]>

* bring back the optimization when null count is zero

Signed-off-by: remzi <[email protected]>

* pretty the trait bound and update comment

Signed-off-by: remzi <[email protected]>

* use value_unchecked to replace array.value
10% extra speed up

Signed-off-by: remzi <[email protected]>

* update the performance data

Signed-off-by: remzi <[email protected]>

arrow/src/compute/kernels/aggregate.rs

@@ -32,39 +32,29 @@ fn is_nan<T: ArrowNativeType + PartialOrd + Copy>(a: T) -> bool {
     !(a == a)
 }
 
-/// Helper macro to perform min/max of strings
-fn min_max_string<T: StringOffsetSizeTrait, F: Fn(&str, &str) -> bool>(
-    array: &GenericStringArray<T>,
-    cmp: F,
-) -> Option<&str> {
+/// Helper function to perform min/max of strings
+fn min_max_string<T, F>(array: &GenericStringArray<T>, cmp: F) -> Option<&str>
+where
+    T: StringOffsetSizeTrait,
+    F: Fn(&str, &str) -> bool,
+{
     let null_count = array.null_count();
 
     if null_count == array.len() {
-        return None;
-    }
-    let data = array.data();
-    let mut n;
-    if null_count == 0 {
-        n = array.value(0);
-        for i in 1..data.len() {
-            let item = array.value(i);
-            if cmp(n, item) {
-                n = item;
-            }
-        }
+        None
+    } else if null_count == 0 {
+        // JUSTIFICATION
+        //  Benefit:  ~8% speedup
+        //  Soundness: `i` is always within the array bounds
+        (0..array.len())
+            .map(|i| unsafe { array.value_unchecked(i) })
+            .reduce(|acc, item| if cmp(acc, item) { item } else { acc })
     } else {
-        n = "";
-        let mut has_value = false;
-
-        for i in 0..data.len() {
-            let item = array.value(i);
-            if data.is_valid(i) && (!has_value || cmp(n, item)) {
-                has_value = true;
-                n = item;
-            }
-        }
+        array
+            .iter()
+            .flatten()
+            .reduce(|acc, item| if cmp(acc, item) { item } else { acc })
     }
-    Some(n)
 }
 
 /// Returns the minimum value in the array, according to the natural order.