nirandaperera commented on a change in pull request #10538:
URL: https://github.com/apache/arrow/pull/10538#discussion_r663215493
##########
File path: cpp/src/arrow/compute/kernels/scalar_if_else.cc
##########
@@ -274,18 +310,129 @@ static void RunIfElseLoop(const ArrayData& cond,
HandleBulk handle_bulk,
}
template <typename HandleBulk, typename HandleEach>
-static void RunIfElseLoopInverted(const ArrayData& cond, HandleBulk
handle_bulk,
- HandleEach handle_each) {
- return RunIfElseLoop<HandleBulk, HandleEach, true>(cond, handle_bulk,
handle_each);
+static void RunIfElseLoopInverted(const ArrayData& cond, const HandleBulk&
handle_bulk,
+ const HandleEach& handle_each) {
+ RunIfElseLoop<HandleBulk, HandleEach, true>(cond, handle_bulk, handle_each);
+}
+
+/// Runs the main if_else loop.
+///
+/// `HandleBulk` has the signature:
+/// [](int64_t offset, int64_t length){...}
+/// It should copy `length` number of elements from source array to output
array with
+/// `offset` offset in both arrays
+///
+/// `HandleEach` has the signature:
+/// [](int64_t offset){...}
+/// It should copy single element from source array to output array with
`offset`
+/// offset in both arrays
+template <typename HandleBulkLeft, typename HandleBulkRight, typename
HandleEachLeft,
+ typename HandleEachRight, bool invert = false>
+static void RunIfElseLoop(const ArrayData& cond, const HandleBulkLeft&
handle_bulk_left,
+ const HandleBulkRight& handle_bulk_right,
+ const HandleEachLeft& handle_each_left,
+ const HandleEachRight& handle_each_right) {
+ int64_t offset = 0;
+ const auto* cond_data = cond.buffers[1]->data(); // this is a BoolArray
+
+ // There are multiple options for this one. Ex: BitBlockCounter,
BitmapWordReader,
+ // BitRunReader, etc. BitRunReader would be efficient for longer contiguous
values in
+ // the cond data buffer.
+ // BitmapWordReader was slightly better performant that BitBlockCounter.
+ BitmapWordReader<Word> cond_reader(cond_data, cond.offset, cond.length);
+
+ int64_t cnt = cond_reader.words();
+ while (cnt--) {
+ Word word = cond_reader.NextWord();
+ if (invert) {
+ if (word == UINT64_MAX) {
+ handle_bulk_right(offset, word_len);
+ } else if (word == 0) {
+ handle_bulk_left(offset, word_len);
+ } else {
+ for (int64_t i = 0; i < word_len; ++i) {
+ if (!BitUtil::GetBit(cond_data, cond.offset + offset + i)) {
+ handle_each_right(offset + i);
+ } else {
+ handle_each_left(offset + i);
+ }
+ }
+ }
+ } else {
+ if (word == UINT64_MAX) {
+ handle_bulk_left(offset, word_len);
+ } else if (word == 0) {
+ handle_bulk_right(offset, word_len);
+ } else {
+ for (int64_t i = 0; i < word_len; ++i) {
+ if (BitUtil::GetBit(cond_data, cond.offset + offset + i)) {
+ handle_each_left(offset + i);
+ } else {
+ handle_each_right(offset + i);
+ }
+ }
+ }
+ }
+ offset += word_len;
+ }
+
+ cnt = cond_reader.trailing_bytes();
+ while (cnt--) {
+ int valid_bits;
+ uint8_t byte = cond_reader.NextTrailingByte(valid_bits);
+ if (invert) {
+ if (byte == UINT8_MAX && valid_bits == 8) {
+ handle_bulk_right(offset, 8);
+ } else if (byte == 0 && valid_bits == 8) {
+ handle_bulk_left(offset, 8);
+ } else {
+ for (int i = 0; i < valid_bits; ++i) {
+ if (!BitUtil::GetBit(cond_data, cond.offset + offset + i)) {
+ handle_each_right(offset + i);
+ } else {
+ handle_each_left(offset + i);
+ }
+ }
+ }
+ } else {
+ if (byte == UINT8_MAX && valid_bits == 8) {
+ handle_bulk_left(offset, 8);
+ } else if (byte == 0 && valid_bits == 8) {
+ handle_bulk_right(offset, 8);
+ } else {
+ for (int i = 0; i < valid_bits; ++i) {
+ if (BitUtil::GetBit(cond_data, cond.offset + offset + i)) {
+ handle_each_left(offset + i);
+ } else {
+ handle_each_right(offset + i);
+ }
+ }
+ }
+ }
+ offset += 8; // doesn't necessarily have to be valid_bits here. Because it
+ // valid_bits < 8, then the loop will exit
Review comment:
No, this is only done for the bytes at the end. So, we had a 100
lengthed bitmap. Then, first 64 would fall into the `Word` block. Rest 36,
would be handled byte-by-byte (8 + 8+ 8+ 8 + 4). So, `cnt=5`. When `valid_bits<
8` that means the `while` loop end on the next iteration. And `offset` value is
not used beyond the loop. So, we can just keep on incrementing by 8.
--
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]
