-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Speed up binary kernels (30% faster and and or), add BooleanBuffer::from_bitwise_binary_op
#9090
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| /// ); | ||
| /// assert_eq!(result.inner().as_slice(), &[0b10101110u8, 0b00001111u8]); | ||
| /// ``` | ||
| pub fn from_bitwise_binary_op<F>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the new API
9d87a11 to
577dfa8
Compare
| /// Apply a bitwise not to one input and return the result as a Buffer. | ||
| /// The input is treated as a bitmap, meaning that offset and length are specified in number of bits. | ||
| pub fn buffer_unary_not(left: &Buffer, offset_in_bits: usize, len_in_bits: usize) -> Buffer { | ||
| // TODO: should we deprecate this function in favor of the Buffer ! impl ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no Buffer impl of ! (there is an implementation in BooleanBuffer)
|
run benchmark boolean_kernels |
|
🤖 |
|
🤖: Benchmark completed Details
|
BooleanBuffer::from_bitwise_binary_opand and or), add BooleanBuffer::from_bitwise_binary_op
arrow-buffer/src/buffer/boolean.rs
Outdated
| .iter() | ||
| .zip(right_chunks.iter()) | ||
| .map(|(left, right)| op(left, right)); | ||
| // Soundness: `BitChunks` is a `BitChunks` iterator which |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trusted Len ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in a22ad8d
Which issue does this PR close?
Buffer::from_bitwise_unaryandBuffer::from_bitwise_binaryme… #8854This is the next step after
notkernel by 50%, addBooleanBuffer::from_bitwise_unary#8996Rationale for this change
Also, it is hard to find the code to create new Buffers by applying bitwise unary operations.
What changes are included in this PR?
BooleanBuffer::from_bitwise_binarybitwise_bin_op_helperto use the new BooleanBufferAre these changes tested?
Yes new tests are added
Performance results show 30% performance improvement for the
andandorkernels for aligned buffers (common case)Are there any user-facing changes?
A new API