[GitHub] [arrow] kou commented on a change in pull request #7135: ARROW-8553: [C++] Optimize unaligned bitmap operations

[GitBox <gi...@apache.org>]

kou commented on a change in pull request #7135:
URL: https://github.com/apache/arrow/pull/7135#discussion_r422718116



##########
File path: cpp/src/arrow/util/bit_util.cc
##########
@@ -273,28 +274,115 @@ void AlignedBitmapOp(const uint8_t* left, int64_t left_offset, const uint8_t* ri
   }
 }
 
-template <typename Op>
+template <template <typename> class BitOp, typename LogicalOp>
 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) {
-  Op op;
-  auto left_reader = internal::BitmapReader(left, left_offset, length);
-  auto right_reader = internal::BitmapReader(right, right_offset, length);
-  auto writer = internal::BitmapWriter(out, out_offset, length);
-  for (int64_t i = 0; i < length; ++i) {
-    if (op(left_reader.IsSet(), right_reader.IsSet())) {
-      writer.Set();
-    } else {
-      writer.Clear();
+  using Word = uint64_t;
+
+  left += left_offset / 8;
+  right += right_offset / 8;
+  out += out_offset / 8;
+
+  left_offset %= 8;
+  right_offset %= 8;
+  out_offset %= 8;
+
+  const int64_t min_offset = std::min({left_offset, right_offset, out_offset});
+  const int64_t min_nbytes = BitUtil::BytesForBits(length + min_offset);
+  int64_t nwords = min_nbytes / sizeof(Word);
+
+  // process in words, we may touch two words in each iteration
+  if (nwords > 1) {
+    BitOp<Word> op;
+    constexpr int64_t bits_per_word = sizeof(Word) * 8;
+    const Word out_mask = (1U << out_offset) - 1;
+
+    length -= (nwords - 1) * bits_per_word;
+    Word left_word0 = util::SafeLoadAs<Word>(left);
+    Word right_word0 = util::SafeLoadAs<Word>(right);
+    Word out_word0 = util::SafeLoadAs<Word>(out);
+
+    do {

Review comment:
       FYI: You can test big-endian case on local by `ARCH=s390x UBUNTU=20.04 docker-compose run --rm ubuntu-cpp` and qemu-user-static that can be installed by `apt install -y qemu-user-static` on Debian family.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org