[GitHub] [arrow-rs] Dandandan commented on a change in pull request #1374: Speed up the function `min_max_string`

[GitBox <gi...@apache.org>]

Dandandan commented on a change in pull request #1374:
URL: https://github.com/apache/arrow-rs/pull/1374#discussion_r816528363



##########
File path: arrow/src/compute/kernels/aggregate.rs
##########
@@ -32,39 +32,26 @@ 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 {
+        (0..array.len())
+            .map(|i| array.value(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

Review comment:
       According to your benchmark result, this new way a bit slower than before for nulls?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org