alamb commented on code in PR #12024:
URL: https://github.com/apache/arrow-rs/pull/12024#discussion_r4486323014
##########
arrow-buffer/src/builder/boolean.rs:
##########
@@ -276,10 +276,63 @@ impl BooleanBufferBuilder {
self.advance(additional);
let offset = self.len() - additional;
- for (i, v) in slice.iter().enumerate() {
- if *v {
- unsafe { bit_util::set_bit_raw(self.buffer.as_mut_ptr(),
offset + i) }
+
+ // Up to 8 values touch at most two bytes, so pack them directly. This
+ // includes 8 values at an unaligned offset, which leave no whole byte
+ // for `pack_slice` to pack.
+ if additional <= 8 {
+ if additional == 0 {
+ return;
+ }
+ // At most 8 bits starting at bit 7 end at bit 14, so they span at
most two bytes
+ let bits = (Self::pack_bits(slice) as u16) << (offset % 8);
+ let buffer = self.buffer.as_slice_mut();
Review Comment:
You could probably avoid a bounds check here with some unsafe
##########
arrow-buffer/src/builder/boolean.rs:
##########
@@ -276,10 +276,63 @@ impl BooleanBufferBuilder {
self.advance(additional);
let offset = self.len() - additional;
- for (i, v) in slice.iter().enumerate() {
- if *v {
- unsafe { bit_util::set_bit_raw(self.buffer.as_mut_ptr(),
offset + i) }
+
+ // Up to 8 values touch at most two bytes, so pack them directly. This
+ // includes 8 values at an unaligned offset, which leave no whole byte
+ // for `pack_slice` to pack.
+ if additional <= 8 {
Review Comment:
seems like a reasonable plan to me
I am curious if you tried larger sizes (like back 64 bits at a time, for
example)
--
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]
