Consolidate bitwise operation implementations
- Dominant language
- Rust
- Stars
- 3.6k
- Forks
- 1.3k
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 168
Description
**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
I spent quite a while reading the code for various bitwise kernels while working
with @rluvaton on
- https://github.com/apache/arrow-rs/pull/8619
I found there are many different, somewhat overlapping functions that are spread over the codebase
This makes it
1. Hard to find the appropriate API
2. Harder to optimize the correct operations as it is not clear always where
the relevant code is is located
I also think my experience in https://github.com/apache/arrow-rs/pull/8793 suggests there is significant room for optimization,
and that it is important to have two sets of functions: Modify in place and return new buffer
I think part of the current state is somewhat due to history and some of these functions predated the Buffer APIs
**Describe the solution you'd like**
I would like the implementation for the "core" APIs to be clear:
1. Create a new `Buffer` from a unary / binary bitwise operation
2. Apply a unary/binary bitwise operation to an existing Mutable buffer in place
After https://github.com/apache/arrow-rs/pull/8619 we have 2
**Describe alternatives you've considered**
Thus, I propose we do the following:
1. Add new `Buffer::bitwise_unary` and `Buffer::bitwise_binary` functions (that do the same thing as `bitwise_bin_op_helper` and `bitwise_unary_op_helper`) but are easier to find and use, and consistently named with `PrimitiveArray::unary` and `PrimitiveArray::binary` functions: https://github.com/apache/arrow-rs/pull/8854
2. Add `BooleanArray::binary` and `BooleanArray::unary` functions that use the new Buffer functions internally
2. Deprecate `bitwise_bin_op_helper`, and `bitwise_unary_op_helper` in favor of the new Buffer methods
3. Deprecate special methods such as `buffer_bin_or` methods in favor of using the new Buffer methods directly
4. Consolidate bitwise kernel benchmarks
Then we'll basically have two core APIs:
1. Apply bitwise operations in place (via apply_bitwise_unary / apply_bitwise_binary introduced in https://github.com/apache/arrow-rs/pull/8619)
2. Create a new buffer with the result of bitwise operations (via `Buffer::bitwise_unary` / `Buffer::bitwise_binary`)
Then all other APIs will then be thin wrappers around the core APIs and we can spend optimziation and testing efforts on these two core APIs.
**Additional context**
Contributor guide
Research direction
Start by reading the existing bitwise_bin_op_helper and bitwise_unary_op_helper entry points, the Buffer and MutableBuffer APIs, and the BooleanArray and PrimitiveArray unary/binary APIs referenced in the issue. Compare the related work in pull requests 8619 and 8854. Done means the core APIs are consolidated, special helpers are deprecated or wrapped, and the bitwise kernel benchmarks are consolidated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design, data-engineering
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 32/100