nevi-me commented on a change in pull request #8116:
URL: https://github.com/apache/arrow/pull/8116#discussion_r484493616
##########
File path: rust/datafusion/src/physical_plan/math_expressions.rs
##########
@@ -19,23 +19,25 @@
use crate::error::{ExecutionError, Result};
-use arrow::array::{Array, ArrayRef, Float32Array, Float64Array,
Float64Builder};
+use arrow::array::{Array, ArrayRef, Float32Array, Float64Array};
use arrow::datatypes::DataType;
use std::sync::Arc;
macro_rules! compute_op {
($ARRAY:expr, $FUNC:ident, $TYPE:ident) => {{
- let mut builder = Float64Builder::new($ARRAY.len());
- for i in 0..$ARRAY.len() {
- if $ARRAY.is_null(i) {
- builder.append_null()?;
- } else {
- builder.append_value($ARRAY.value(i).$FUNC() as f64)?;
- }
- }
- Ok(Arc::new(builder.finish()))
+ let result: Float64Array = (0..$ARRAY.len())
+ .map(|i| {
Review comment:
Here's what I'm getting if I remove the loop. A further 85-90%
improvement on top of your change (on Ryzen 3700x).
The change requires making 1 function and 1 field public though, so maybe we
can find some way of working around that.
```rust
sqrt_20_12 time: [2.5142 ms 2.6041 ms 2.6987 ms]
change: [-90.546% -90.184% -89.809%] (p = 0.00 <
0.05)
Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mild
sqrt_22_12 time: [15.254 ms 15.532 ms 15.817 ms]
change: [-85.989% -85.707% -85.436%] (p = 0.00 <
0.05)
Performance has improved.
sqrt_22_14 time: [12.698 ms 12.919 ms 13.158 ms]
change: [-87.515% -87.306% -87.069%] (p = 0.00 <
0.05)
Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
4 (4.00%) high mild
```
Code
```rust
macro_rules! compute_op {
($ARRAY:expr, $FUNC:ident, $TYPE:ident) => {{
let len = $ARRAY.len();
let result = (0..len)
.map(|i| $ARRAY.value(i).$FUNC() as f64)
.collect::<Vec<f64>>();
let data = ArrayData::new(
DataType::Float64,
len,
None,
// this is nasty, not sure of how to do it better, maybe we
could introduce a fn that does this?
$ARRAY
.data()
.null_bitmap()
.clone()
.map(|bitmap| bitmap.bits),
0,
vec![Buffer::from(result.to_byte_slice())],
vec![],
);
Ok(make_array(Arc::new(data)))
}};
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
