vertexclique commented on a change in pull request #6980:
URL: https://github.com/apache/arrow/pull/6980#discussion_r413951534
##########
File path: rust/arrow/src/array/builder.rs
##########
@@ -236,6 +251,14 @@ impl<T: ArrowPrimitiveType> BufferBuilderTrait<T> for
BufferBuilder<T> {
self.write_bytes(v.to_byte_slice(), 1)
}
+ default fn append_n(&mut self, n: usize, v: T::Native) -> Result<()> {
+ self.reserve(n)?;
+ for _ in 0..n {
+ self.write_bytes(v.to_byte_slice(), 1)?;
+ }
Review comment:
So, `for _ in 0..n` kind of code, even you don't use, checks the
boundaries. This is one of the infamous code pieces that Rust has. Personally I
don't use this whenever its possible :). In [here]() you can see one of the
examples. Range check exists here:
```
mov rax, qword ptr [rip + core::iter::range::<impl
core::iter::traits::iterator::Iterator for
core::ops::range::Range<A>>::next@GOTPCREL]
```
This call makes checked_add and which makes range checks. So that is slowing
down for big iterations as you saw in the link. Since this is an open issue in
the compiler please consider using iterator methods instead of for loops
whenever you see.
As you can also see, in this very code 5 cycles spent every range check (in
minimal), so that said it is always better to use iterator methods. Bringing
the example here :)
```
(0..n)
.for_each(|_| {
self.write_bytes(v.to_byte_slice(), 1)?;
})
```
This will be completely pruned in `-C opt-level=3` most probably :)
----------------------------------------------------------------
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]
