You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/12/13 11:08:02 UTC
[GitHub] [arrow-rs] tustvold opened a new pull request #1039: BooleanBufferBuilder::append_packed (#1038)
tustvold opened a new pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039
# Which issue does this PR close?
Closes #1038.
# Rationale for this change
See ticket
# What changes are included in this PR?
This copies the code from [IOx](https://github.com/influxdata/influxdb_iox/blob/fe0bc2180732cc2189cbfe9a4727294793d25f4d/arrow_util/src/bitset.rs#L111) into arrow proper
# Are there any user-facing changes?
`BooleanBufferBuilder::append_packed` is public
--
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
[GitHub] [arrow-rs] alamb merged pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
alamb merged pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039
--
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
[GitHub] [arrow-rs] tustvold commented on pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold commented on pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#issuecomment-1009978794
As per @jhorstmann 's great suggestion, this instead now lifts the already extant code up from `arrow::array::transform::util` into `arrow::util::bit_mask`. Aside from being less code, it also appears to be faster :tada:
--
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
[GitHub] [arrow-rs] tustvold commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r768134400
##########
File path: arrow/src/array/builder.rs
##########
@@ -398,6 +398,54 @@ impl BooleanBufferBuilder {
}
}
+ /// Append a slice of packed bits
Review comment:
Updated, FWIW I think it is actually `ceil(count / 8)` not `ceil(count / 8) + 1`
--
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
[GitHub] [arrow-rs] codecov-commenter commented on pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#issuecomment-992362419
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1039?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1039](https://codecov.io/gh/apache/arrow-rs/pull/1039?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bf04099) into [master](https://codecov.io/gh/apache/arrow-rs/commit/e0abda2c178be0c38d4257d22de2e4a3bfafde82?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e0abda2) will **increase** coverage by `0.02%`.
> The diff coverage is `98.41%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #1039 +/- ##
==========================================
+ Coverage 82.31% 82.33% +0.02%
==========================================
Files 168 168
Lines 49031 49094 +63
==========================================
+ Hits 40359 40421 +62
- Misses 8672 8673 +1
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/1039/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `87.06% <98.41%> (+0.42%)` | :arrow_up: |
| [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/1039/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `85.10% <0.00%> (-0.14%)` | :arrow_down: |
| [arrow/src/datatypes/field.rs](https://codecov.io/gh/apache/arrow-rs/pull/1039/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2RhdGF0eXBlcy9maWVsZC5ycw==) | `53.68% <0.00%> (+0.30%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1039?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1039?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e0abda2...bf04099](https://codecov.io/gh/apache/arrow-rs/pull/1039?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
[GitHub] [arrow-rs] alamb commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r768128176
##########
File path: arrow/src/array/builder.rs
##########
@@ -398,6 +398,54 @@ impl BooleanBufferBuilder {
}
}
+ /// Append a slice of packed bits
Review comment:
```suggestion
/// Append `count` bits from `to_set`. `to_set` is a slice of bits packed into `u8`
///
/// panics if `to_set` does not contain at at least `ceil(count / 8) + 1` bytes
```
--
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
[GitHub] [arrow-rs] tustvold commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r778406471
##########
File path: arrow/src/array/builder.rs
##########
@@ -398,6 +399,95 @@ impl BooleanBufferBuilder {
}
}
+ /// Append `count` bits from `to_set`
+ ///
+ /// `to_set` is a slice of bits packed LSB-first into `[u8]`
+ ///
+ /// # Panics
+ ///
+ /// Panics if `to_set` does not contain `ceil(count / 8)` bytes
+ #[inline]
+ pub fn append_packed(&mut self, count: usize, to_set: &[u8]) {
+ assert_eq!((count + 7) >> 3, to_set.len());
Review comment:
Sorry was waiting to find time to evaluate @jhorstmann 's comment - will unmark as draft once I've done that - and address review comments
--
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
[GitHub] [arrow-rs] alamb commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r768158591
##########
File path: arrow/src/array/builder.rs
##########
@@ -398,6 +398,54 @@ impl BooleanBufferBuilder {
}
}
+ /// Append a slice of packed bits
Review comment:
👍 Even better that someone looked at it
--
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
[GitHub] [arrow-rs] tustvold commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r782142570
##########
File path: arrow/src/array/builder.rs
##########
@@ -398,6 +399,95 @@ impl BooleanBufferBuilder {
}
}
+ /// Append `count` bits from `to_set`
+ ///
+ /// `to_set` is a slice of bits packed LSB-first into `[u8]`
+ ///
+ /// # Panics
+ ///
+ /// Panics if `to_set` does not contain `ceil(count / 8)` bytes
+ #[inline]
+ pub fn append_packed(&mut self, count: usize, to_set: &[u8]) {
+ assert_eq!((count + 7) >> 3, to_set.len());
Review comment:
The transform version is 2x faster :tada: will update PR to use this :smile:
--
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
[GitHub] [arrow-rs] tustvold commented on pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold commented on pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#issuecomment-996865825
Turns out this currently runs into #1051 which is fixed by #1052
--
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
[GitHub] [arrow-rs] Dandandan commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r773070870
##########
File path: arrow/src/array/builder.rs
##########
@@ -398,6 +399,95 @@ impl BooleanBufferBuilder {
}
}
+ /// Append `count` bits from `to_set`
+ ///
+ /// `to_set` is a slice of bits packed LSB-first into `[u8]`
+ ///
+ /// # Panics
+ ///
+ /// Panics if `to_set` does not contain `ceil(count / 8)` bytes
+ #[inline]
+ pub fn append_packed(&mut self, count: usize, to_set: &[u8]) {
+ assert_eq!((count + 7) >> 3, to_set.len());
Review comment:
I think `/ 8` is more clear here?
--
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
[GitHub] [arrow-rs] tustvold commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r782156460
##########
File path: arrow/src/array/builder.rs
##########
@@ -2713,7 +2734,8 @@ mod tests {
let buffer = b.finish();
assert_eq!(1, buffer.len());
- let mut b = BooleanBufferBuilder::new(4);
+ // Overallocate capacity
+ let mut b = BooleanBufferBuilder::new(8);
Review comment:
Additional test for #1051
Edit: I think this is actually just the diff being unhelpful - this code exists on master...
--
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
[GitHub] [arrow-rs] alamb commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r782240568
##########
File path: arrow/src/array/transform/boolean.rs
##########
@@ -15,12 +15,9 @@
// specific language governing permissions and limitations
// under the License.
+use super::{Extend, _MutableArrayData, utils::resize_for_bits};
use crate::array::ArrayData;
-
-use super::{
- Extend, _MutableArrayData,
- utils::{resize_for_bits, set_bits},
-};
+use crate::util::bit_mask::set_bits;
Review comment:
just to be explicit the `bit_mask` crate is not exposed publically
##########
File path: arrow/src/util/mod.rs
##########
@@ -18,6 +18,7 @@
#[cfg(feature = "test_utils")]
pub mod bench_util;
pub mod bit_chunk_iterator;
+pub(crate) mod bit_mask;
Review comment:
Let's leave it as `pub(crate)` until the next round of parquet finagling is complete and then make a separate decision (PR) about if we should make it public or not
--
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
[GitHub] [arrow-rs] tustvold commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r782156460
##########
File path: arrow/src/array/builder.rs
##########
@@ -2713,7 +2734,8 @@ mod tests {
let buffer = b.finish();
assert_eq!(1, buffer.len());
- let mut b = BooleanBufferBuilder::new(4);
+ // Overallocate capacity
+ let mut b = BooleanBufferBuilder::new(8);
Review comment:
Additional test for #1051
--
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
[GitHub] [arrow-rs] tustvold edited a comment on pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold edited a comment on pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#issuecomment-995917079
Added packed_append_range, also from IOx, as I needed it for #1037
@jhorstmann I will look into benchmarking the approaches and report back. I'll mark this as a draft until I find time for 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#issuecomment-998146272
#1052 is merged now
--
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
[GitHub] [arrow-rs] tustvold commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r782154268
##########
File path: arrow/src/util/bit_mask.rs
##########
@@ -0,0 +1,193 @@
+// Licensed to the Apache Software Foundation (ASF) under one
Review comment:
Split out from `arrow::array::transform::util`
--
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
[GitHub] [arrow-rs] tustvold commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r782153323
##########
File path: arrow/src/array/transform/utils.rs
##########
@@ -33,49 +26,6 @@ pub(super) fn resize_for_bits(buffer: &mut MutableBuffer, len: usize) {
}
}
-/// sets all bits on `write_data` on the range `[offset_write..offset_write+len]` to be equal to the
Review comment:
This is moved to `arrow::util::bit_mask` so that it can be used within `arrow::array::builder`
--
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
[GitHub] [arrow-rs] tustvold commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r782152854
##########
File path: arrow/src/util/mod.rs
##########
@@ -18,6 +18,7 @@
#[cfg(feature = "test_utils")]
pub mod bench_util;
pub mod bit_chunk_iterator;
+pub(crate) mod bit_mask;
Review comment:
This could be made `pub mod` and then we could use it in IOx but wasn't sure about 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
tustvold commented on pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#issuecomment-995917079
Added packed_append_range, also from IOx, as I needed it for #1037
@jhorstmann I will look into benchmarking the approaches and report back
--
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
[GitHub] [arrow-rs] alamb commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r778397856
##########
File path: arrow/src/array/builder.rs
##########
@@ -398,6 +399,95 @@ impl BooleanBufferBuilder {
}
}
+ /// Append `count` bits from `to_set`
+ ///
+ /// `to_set` is a slice of bits packed LSB-first into `[u8]`
+ ///
+ /// # Panics
+ ///
+ /// Panics if `to_set` does not contain `ceil(count / 8)` bytes
+ #[inline]
+ pub fn append_packed(&mut self, count: usize, to_set: &[u8]) {
+ assert_eq!((count + 7) >> 3, to_set.len());
Review comment:
@tustvold would you like to make this change? Or shall we merge it as is?
--
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
[GitHub] [arrow-rs] jhorstmann commented on a change in pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
jhorstmann commented on a change in pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#discussion_r778789580
##########
File path: arrow/src/array/builder.rs
##########
@@ -398,6 +399,95 @@ impl BooleanBufferBuilder {
}
}
+ /// Append `count` bits from `to_set`
+ ///
+ /// `to_set` is a slice of bits packed LSB-first into `[u8]`
+ ///
+ /// # Panics
+ ///
+ /// Panics if `to_set` does not contain `ceil(count / 8)` bytes
+ #[inline]
+ pub fn append_packed(&mut self, count: usize, to_set: &[u8]) {
+ assert_eq!((count + 7) >> 3, to_set.len());
Review comment:
My comment about benchmarking shouldn't be seen as blocking this PR, the api is definitely useful and we can always try to improve the performance later.
--
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
[GitHub] [arrow-rs] jhorstmann commented on pull request #1039: BooleanBufferBuilder::append_packed (#1038)
Posted by GitBox <gi...@apache.org>.
jhorstmann commented on pull request #1039:
URL: https://github.com/apache/arrow-rs/pull/1039#issuecomment-993821801
Logic looks correct to me. There is code with a similar goal already in [`transform/utils.rs`](https://github.com/apache/arrow-rs/blob/07660c61680220ac54b7bf4c42a64c840872cc43/arrow/src/array/transform/utils.rs#L39) which is used when concatenating boolean arrays and null bitmaps. That code uses chunked iteration, so in theory could append 64 bits at once, but it's also a bit more complex and indirect. Could be interesting to benchmark both approaches.
--
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