Skip to content

Conversation

@alamb
Copy link

@alamb alamb commented Oct 24, 2025

As suggested by @rluvaton in apache#8619 (comment)

I will later change BooleanBufferBuilder#append_packed_range function to use mutable_bitwise_bin_op_helper as I saw that running the boolean_append_packed benchmark improved by 57%


boolean_append_packed   time:   [2.0079 µs 2.0139 µs 2.0202 µs]

                        change: [−57.808% −57.653% −57.494%] (p = 0.00 < 0.05)

                        Performance has improved.

You can change the code that I described

This PR updates append_packed_range to use the new function.

NOTE it fails the fuzz test for reasons I haven't debugged

$ cargo test -p arrow-buffer
...

---- builder::boolean::tests::test_bool_buffer_fuzz stdout ----

thread 'builder::boolean::tests::test_bool_buffer_fuzz' panicked at arrow-buffer/src/builder/boolean.rs:585:9:
assertion `left == right` failed
  left: BooleanBuffer { buffer: Buffer { data: Bytes { ptr: 0x80b02c000, len: 138, data: [163, 161, 209, 144, 24, 99, 70, 27, 51, 58, 26, 141, 209, 163, 33, 25, 109, 180, 209, 163, 33, 97, 180, 209, 163, 71, 67, 2, 56, 163, 33, 113, 52, 154, 141, 134, 196, 24, 13, 9, 224, 104, 72, 192, 104, 200, 56, 26, 210, 104, 72, 31, 13, 105, 180, 209, 144, 0, 30, 13, 201, 142, 54, 26, 18, 192, 70, 27, 163, 33, 97, 52, 116, 52, 36, 48, 58, 26, 18, 96, 60, 26, 26, 71, 143, 134, 4, 48, 26, 146, 49, 26, 50, 218, 104, 70, 67, 50, 154, 25, 13, 25, 71, 115, 52, 36, 163, 33, 71, 143, 134, 142, 134, 54, 122, 52, 180, 209, 240, 56, 26, 163, 49, 26, 141, 209, 144, 108, 52, 36, 128, 209, 144, 128, 71, 143, 134, 70] }, ptr: 0x80b02c000, length: 138 }, offset: 0, len: 1098 }
 right: BooleanBuffer { buffer: Buffer { data: Bytes { ptr: 0x80a41c000, len: 138, data: [52, 164, 33, 1, 220, 48, 0, 4, 68, 2, 3, 48, 26, 209, 144, 72, 96, 180, 0, 209, 144, 128, 134, 104, 209, 162, 33, 1, 172, 209, 144, 0, 18, 18, 132, 4, 240, 208, 144, 0, 182, 33, 1, 236, 104, 136, 49, 26, 18, 9, 96, 130, 4, 16, 141, 104, 72, 0, 131, 4, 112, 104, 24, 13, 9, 96, 193, 6, 13, 9, 32, 1, 172, 33, 1, 140, 52, 26, 18, 96, 135, 4, 32, 72, 68, 67, 2, 8, 9, 96, 16, 18, 0, 54, 26, 208, 144, 64, 130, 64, 67, 194, 72, 80, 67, 2, 72, 0, 13, 13, 9, 146, 0, 60, 36, 1, 12, 72, 176, 20, 18, 104, 8, 8, 9, 36, 128, 121, 52, 36, 0, 13, 9, 96, 52, 4, 176, 1] }, ptr: 0x80a41c000, length: 138 }, offset: 0, len: 1098 }


failures:
    builder::boolean::tests::test_bool_buffer_fuzz

bit_mask::set_bits(
self.buffer.as_slice_mut(),
to_set,
mutable_bitwise_bin_op_helper(
Copy link
Author

@alamb alamb Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the real change -- I am not sure why it isn't passing. I may misunderstand what is needed (yes I am absolutely hedging in case I made a mistake 😬 )

@rluvaton
Copy link
Owner

The test fails because you found a bug, thanks, fixing

Co-authored-by: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
@alamb
Copy link
Author

alamb commented Oct 30, 2025

@alamb alamb closed this Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants