AdamGS commented on code in PR #6365:
URL: https://github.com/apache/arrow-rs/pull/6365#discussion_r1750392301
##########
arrow-arith/src/arity.rs:
##########
@@ -216,15 +220,39 @@ where
}
let nulls = NullBuffer::union(a.logical_nulls().as_ref(),
b.logical_nulls().as_ref());
+ let mut output: Vec<MaybeUninit<O::Native>> = vec![MaybeUninit::uninit();
a.len()];
+ let a_chunks = a.values().as_ref().chunks_exact(CHUNK_SIZE);
+ let b_chunks = b.values().as_ref().chunks_exact(CHUNK_SIZE);
+ let output_chunks = output.as_mut_slice().chunks_exact(CHUNK_SIZE);
+
+ for (output, (a_chunk, b_chunk)) in
output_chunks.zip(a_chunks.zip(b_chunks)) {
+ let a_values: [A::Native; CHUNK_SIZE] = a_chunk.try_into().unwrap();
+ let b_values: [B::Native; CHUNK_SIZE] = b_chunk.try_into().unwrap();
+ let mut output: [MaybeUninit<O::Native>; CHUNK_SIZE] =
output.try_into().unwrap();
Review Comment:
Ok that seems to be a key point here, changing that (and `let output_chunks
= output.as_mut_slice().chunks_exact_mut(CHUNK_SIZE);`) seems to make
performance worse than the current implementation on `master`. The thing I
don't understand here is that if we write into a temporary value, how do the
tests still pass?
--
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]
