[GitHub] [arrow] wesm commented on a change in pull request #8671: ARROW-10598: [C++] Separate out bit-packing in internal::GenerateBitsUnrolled for better performance

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

wesm commented on a change in pull request #8671:
URL: https://github.com/apache/arrow/pull/8671#discussion_r523781685



##########
File path: cpp/src/arrow/util/bitmap_generate.h
##########
@@ -83,17 +83,19 @@ void GenerateBitsUnrolled(uint8_t* bitmap, int64_t start_offset, int64_t length,
   }
 
   int64_t remaining_bytes = remaining / 8;
+  uint8_t out_results[8];
   while (remaining_bytes-- > 0) {
-    current_byte = 0;
-    current_byte = g() ? current_byte | 0x01 : current_byte;
-    current_byte = g() ? current_byte | 0x02 : current_byte;
-    current_byte = g() ? current_byte | 0x04 : current_byte;
-    current_byte = g() ? current_byte | 0x08 : current_byte;
-    current_byte = g() ? current_byte | 0x10 : current_byte;
-    current_byte = g() ? current_byte | 0x20 : current_byte;
-    current_byte = g() ? current_byte | 0x40 : current_byte;
-    current_byte = g() ? current_byte | 0x80 : current_byte;
-    *cur++ = current_byte;
+    out_results[0] = g();
+    out_results[1] = g();
+    out_results[2] = g();
+    out_results[3] = g();
+    out_results[4] = g();
+    out_results[5] = g();
+    out_results[6] = g();
+    out_results[7] = g();

Review comment:
       This presumes that `g()` either returns a bool or 0/1 -- might want to enforce this (at compile time) but wasn't sure the best way to do that




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