Jefffrey commented on code in PR #10518:
URL: https://github.com/apache/arrow-rs/pull/10518#discussion_r3701709047
##########
arrow-arith/src/arity.rs:
##########
@@ -379,11 +386,17 @@ where
O: ArrowPrimitiveType,
F: Fn(A::Item, B::Item) -> Result<O::Native, ArrowError>,
{
- let mut buffer = MutableBuffer::new(len * O::Native::get_byte_width());
+ let byte_width = O::Native::get_byte_width();
Review Comment:
does this get better performance than simply using a `vec` here?
##########
arrow-arith/src/arity.rs:
##########
@@ -124,13 +124,24 @@ where
let nulls = NullBuffer::union(a.logical_nulls().as_ref(),
b.logical_nulls().as_ref());
- let values = a
- .values()
- .into_iter()
- .zip(b.values())
- .map(|(l, r)| op(*l, *r));
-
- let buffer: Vec<_> = values.collect();
+ let len = a.len();
+ let byte_width = O::Native::get_byte_width();
Review Comment:
> binary: replaced iterator collect with a direct loop which LLVM can better
optimize and avoids option checks. The loop overhead here is a large part of
the runtime when the underlying op is simple.
this seems surprising to me since as we were iterating+zipping over
`values()` there wouldnt be any option checks going on
##########
arrow-arith/src/arity.rs:
##########
@@ -276,18 +287,16 @@ where
let nulls =
NullBuffer::union(a.logical_nulls().as_ref(),
b.logical_nulls().as_ref()).unwrap();
- let mut buffer = BufferBuilder::<O::Native>::new(len);
- buffer.append_n_zeroed(len);
- let slice = buffer.as_slice_mut();
+ let mut buffer = vec![O::Native::default(); len];
nulls.try_for_each_valid_idx(|idx| {
unsafe {
- *slice.get_unchecked_mut(idx) = op(a.value_unchecked(idx),
b.value_unchecked(idx))?
+ *buffer.get_unchecked_mut(idx) = op(a.value_unchecked(idx),
b.value_unchecked(idx))?
};
Ok::<_, ArrowError>(())
})?;
- let values = buffer.finish().into();
+ let values = Buffer::from(buffer).into();
Ok(PrimitiveArray::new(values, Some(nulls)))
Review Comment:
```suggestion
Ok(PrimitiveArray::new(buffer.into(), Some(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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
