ARROW-8974: [C++] Simplify TransferBitmap #7373
Conversation
pitrou
left a comment
There was a problem hiding this comment.
Thank you for the simplification. Just one suggestion.
|
RTools CI failure looks not related |
cpp/src/arrow/util/bitmap_ops.cc
Outdated
There was a problem hiding this comment.
I meant use the kTrailingBitmask constant array that's exposed in bit_util.h :-)
There was a problem hiding this comment.
Ah, it's a lookup table, cool. Will change.
There was a problem hiding this comment.
We need a table {255, 127, 63, 31, 15, 7, 3, 1}, looks there's no such lookup table available.
Shall I add a new table or keep the code as is (revert the renaming)?
https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/bit_util.h#L420
There was a problem hiding this comment.
Hmm, let's simply revert the renaming then :-)
|
s390 ci failure is not related, it says "no space left on device". |
|
the Appveyor failures are ARROW-9085 |
This patch removes "restore_trailing_bits" template parameter of TransferBitmap class. Trailing bits are now always not clobbered, which is no harm. It also refines trailing bits processing to keep the performance influence trivial. Besides, this patch replaces "invert_bits" boolean parameter with enum to allow explicit naming.
This reverts commit 3a385542bea5954f2443f79d34a6a83c85e45f05.
|
Rebased to (hopefully) fix AppVeyor. |
|
Thank you @cyb70289 ! |
This patch removes "restore_trailing_bits" template parameter of TransferBitmap class. Trailing bits are now always not clobbered, which is no harm. It also refines trailing bits processing to keep the performance influence trivial. Besides, this patch replaces "invert_bits" boolean parameter with enum to allow explicit naming. Closes apache#7373 from cyb70289/bitmap-transfer Authored-by: Yibo Cai <yibo.cai@arm.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
This patch removes "restore_trailing_bits" template parameter of
TransferBitmap class. Trailing bits are now always not clobbered,
which is no harm. It also refines trailing bits processing to keep
the performance influence trivial. Besides, this patch replaces
"invert_bits" boolean parameter with enum to allow explicit naming.