Kanishk-Dendukuri commented on code in PR #50647:
URL: https://github.com/apache/arrow/pull/50647#discussion_r3662699466
##########
cpp/src/arrow/compute/kernels/scalar_compare.cc:
##########
@@ -529,108 +492,246 @@ struct ScalarMinMax {
}
}
+ // Fold left/right into out_values/out_valid a word at a time (validity
bitmap
+ // null => all valid). out_values may alias left. Returns the null count
+ static int64_t CombineWordwise(const OutValue* left, const uint8_t*
left_valid,
+ int64_t left_offset, const OutValue* right,
+ const uint8_t* right_valid, int64_t
right_offset,
+ bool skip_nulls, int64_t length, OutValue*
out_values,
+ uint8_t* out_valid) {
+ auto left_reader = ::arrow::internal::BitmapUInt64Reader(
+ left_valid, left_valid ? left_offset : 0, left_valid ? length : 0);
+ auto right_reader = ::arrow::internal::BitmapUInt64Reader(
+ right_valid, right_valid ? right_offset : 0, right_valid ? length : 0);
+
+ int64_t null_count = 0;
+ int64_t i = 0;
+ for (int64_t words = length / 64; words > 0; --words) {
+ const uint64_t left_word = left_valid ? left_reader.NextWord() :
~uint64_t(0);
+ const uint64_t right_word = right_valid ? right_reader.NextWord() :
~uint64_t(0);
+ const uint64_t out_word =
+ skip_nulls ? (left_word | right_word) : (left_word & right_word);
+ // out_valid is allocated at bit offset 0, so store the word directly
Review Comment:
From my understanding computing the bitmap separately means reading the
left/right validity twice. once in BitmapAnd/Or, then again per element in the
value loop, since with skip_nulls a slot can be valid with one side null and we
need to know which side to take. The fused loop reads each validity word once
and uses it for both.
--
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]
