tustvold commented on code in PR #1577:
URL: https://github.com/apache/arrow-rs/pull/1577#discussion_r852076274
##########
arrow/src/compute/kernels/substring.rs:
##########
@@ -59,13 +78,14 @@ fn generic_substring<OffsetSize: StringOffsetSizeTrait>(
let mut len_so_far = zero;
new_offsets.push(zero);
- offsets.windows(2).for_each(|pair| {
- let new_start = cal_new_start(pair[0], pair[1]);
- let new_length = cal_new_length(new_start, pair[1]);
- len_so_far += new_length;
- new_starts_ends.push((new_start, new_start + new_length));
+ offsets.windows(2).try_for_each(|pair| -> Result<()> {
Review Comment:
My point was that modern optimising compilers will automatically lift
conditionals out of loops, in a process called loop unswitching, and that
letting it do this can be faster not just from eliminating dynamic dispatch,
but also letting it further optimise the code.
I don't feel strongly about it, but thought I'd mention that you might be
able to make the code clearer and faster by letting the compiler handle this
--
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]
