You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2025/04/03 15:01:06 UTC
[I] Consider using `Vec` directly in builders [arrow-rs]
alamb opened a new issue, #7383:
URL: https://github.com/apache/arrow-rs/issues/7383
I think many of these instances could be changed to use `Vec` directly (and use optimized extend, etc. from them)
_Originally posted by @Dandandan in https://github.com/apache/arrow-rs/pull/7309#discussion_r2023700374_
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Consider using `Vec` directly in builders [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7383:
URL: https://github.com/apache/arrow-rs/issues/7383#issuecomment-3107168981
So specifically, for the PR referenced in https://github.com/apache/arrow-rs/pull/7309#discussion_r2023700374
I think it means changing stuff like
https://github.com/apache/arrow-rs/blob/721150286b00bece40ffcc6f5ac14ebb5d64785b/arrow-array/src/builder/generic_bytes_builder.rs#L33-L32
```rust
pub struct GenericByteBuilder<T: ByteArrayType> {
value_builder: UInt8BufferBuilder,
offsets_builder: BufferBuilder<T::Offset>,
null_buffer_builder: NullBufferBuilder,
}
```
To
```rust
pub struct GenericByteBuilder<T: ByteArrayType> {
value_builder: Vec<u8>,
offsets_builder: Vec<T::Offset::Native>,
null_buffer_builder: NullBufferBuilder,
}
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Consider using `Vec` directly in builders [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #7383: Consider using `Vec` directly in builders
URL: https://github.com/apache/arrow-rs/issues/7383
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Consider using `Vec` directly in builders [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7383:
URL: https://github.com/apache/arrow-rs/issues/7383#issuecomment-3107146545
> Hi [@alamb](https://github.com/alamb) and [@Dandandan](https://github.com/Dandandan), I'm looking into this issue and wanted to clarify what you meant by "using `Vec` directly." From what I see, the code is already using a `Vec`, so I assume you're suggesting a refactor like this:
>
> let mut intermediate = Vec::with_capacity(offsets.len() - 1);
> for &offset in &offsets[1..] {
> intermediate.push(offset + shift);
> }
> to use extend
>
> let mut intermediate = Vec::with_capacity(offsets.len() - 1);
> intermediate.extend(offsets[1..].iter().map(|&offset| offset + shift));
> so that the code is more concise and the performance may improve.
>
> Am I right? Thanks!
i think @Dandandan was referring to using the ArrayBuilders
So for example, things that use a builder like
```rust
let mut builder = Int32Builder::new()
for i in 0..100 {
builder.append_value(i)
}
let array = builder.build()
```
Is typically much faster to implement using Vec directly:
```rust
let data: Vec<_> = (0..100).collect();
let array = Int32Array::from(data)
```
This is certainly true for primitive arrays. It is not clear to me it is the same for StringArrays, StringViewArray, etc.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Consider using `Vec` directly in builders [arrow-rs]
Posted by "liamzwbao (via GitHub)" <gi...@apache.org>.
liamzwbao commented on issue #7383:
URL: https://github.com/apache/arrow-rs/issues/7383#issuecomment-3105523896
Hi @alamb and @Dandandan, I'm looking into this issue and wanted to clarify what you meant by "using `Vec` directly." From what I see, the code is already using a `Vec`, so I assume you're suggesting a refactor like this:
```rust
let mut intermediate = Vec::with_capacity(offsets.len() - 1);
for &offset in &offsets[1..] {
intermediate.push(offset + shift);
}
```
to use extend
```rust
let mut intermediate = Vec::with_capacity(offsets.len() - 1);
intermediate.extend(offsets[1..].iter().map(|&offset| offset + shift));
```
so that the code is more concise and the performance may improve.
Am I right? Thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org