kou commented on a change in pull request #11674:
URL: https://github.com/apache/arrow/pull/11674#discussion_r750698236
##########
File path: cpp/src/arrow/util/bitmap_ops.cc
##########
@@ -255,53 +255,68 @@ void AlignedBitmapOp(const uint8_t* left, int64_t
left_offset, const uint8_t* ri
}
template <template <typename> class BitOp>
-void UnalignedBitmapOp(const uint8_t* left, int64_t left_offset, const
uint8_t* right,
- int64_t right_offset, uint8_t* out, int64_t out_offset,
- int64_t length) {
+int64_t UnalignedBitmapOp(const uint8_t* left, int64_t left_offset, const
uint8_t* right,
+ int64_t right_offset, uint8_t* out, int64_t
out_offset,
+ int64_t length) {
BitOp<uint64_t> op_word;
BitOp<uint8_t> op_byte;
+ uint64_t out_pop_count = 0;
auto left_reader = internal::BitmapWordReader<uint64_t>(left, left_offset,
length);
auto right_reader = internal::BitmapWordReader<uint64_t>(right,
right_offset, length);
auto writer = internal::BitmapWordWriter<uint64_t>(out, out_offset, length);
auto nwords = left_reader.words();
while (nwords--) {
- writer.PutNextWord(op_word(left_reader.NextWord(),
right_reader.NextWord()));
+ auto outWord = op_word(left_reader.NextWord(), right_reader.NextWord());
+ out_pop_count += BitUtil::PopCount(outWord);
+ writer.PutNextWord(outWord);
}
auto nbytes = left_reader.trailing_bytes();
while (nbytes--) {
int left_valid_bits, right_valid_bits;
uint8_t left_byte = left_reader.NextTrailingByte(left_valid_bits);
uint8_t right_byte = right_reader.NextTrailingByte(right_valid_bits);
DCHECK_EQ(left_valid_bits, right_valid_bits);
- writer.PutNextTrailingByte(op_byte(left_byte, right_byte),
left_valid_bits);
+ auto out_byte = op_byte(left_byte, right_byte);
+ out_pop_count += BitUtil::kBytePopcount[out_byte];
+ writer.PutNextTrailingByte(out_byte, left_valid_bits);
}
+ return out_pop_count;
}
template <template <typename> class BitOp>
void BitmapOp(const uint8_t* left, int64_t left_offset, const uint8_t* right,
- int64_t right_offset, int64_t length, int64_t out_offset,
uint8_t* dest) {
+ int64_t right_offset, int64_t length, int64_t out_offset,
uint8_t* dest,
+ int64_t* out_validity_count) {
if ((out_offset % 8 == left_offset % 8) && (out_offset % 8 == right_offset %
8)) {
- // Fast case: can use bytewise AND
AlignedBitmapOp<BitOp>(left, left_offset, right, right_offset, dest,
out_offset,
length);
+ if (out_validity_count) {
+ *out_validity_count = CountSetBits(dest, out_offset, length);
+ }
} else {
// Unaligned
- UnalignedBitmapOp<BitOp>(left, left_offset, right, right_offset, dest,
out_offset,
- length);
+ if (out_validity_count) {
+ *out_validity_count = UnalignedBitmapOp<BitOp>(
+ left, left_offset, right, right_offset, dest, out_offset, length);
+ } else {
+ UnalignedBitmapOp<BitOp>(left, left_offset, right, right_offset, dest,
out_offset,
+ length);
+ }
}
}
template <template <typename> class BitOp>
Result<std::shared_ptr<Buffer>> BitmapOp(MemoryPool* pool, const uint8_t* left,
int64_t left_offset, const uint8_t*
right,
int64_t right_offset, int64_t length,
- int64_t out_offset) {
+ int64_t out_offset,
+ int64_t* out_validity_count =
NULLPTR) {
Review comment:
Do we need ` = NULLPTR` here?
##########
File path: cpp/src/arrow/util/bit_util_test.cc
##########
@@ -2247,7 +2263,7 @@ TEST(Bitmap, VisitWordsAnd) {
BitmapAnd(bitmaps[0].buffer()->data(), bitmaps[0].offset(),
bitmaps[1].buffer()->data(), bitmaps[1].offset(),
bitmaps[0].length(),
- 0, expected_buffer->mutable_data());
+ 0, expected_buffer->mutable_data(), /*out pop count*/
nullptr);
Review comment:
It seems that we can revert this change.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
