AntoinePrv commented on code in PR #47294:
URL: https://github.com/apache/arrow/pull/47294#discussion_r2355088657
##########
cpp/src/arrow/util/bit_stream_utils_internal.h:
##########
@@ -439,91 +424,84 @@ inline bool BitReader::Advance(int64_t num_bits) {
return true;
}
-inline bool BitWriter::PutVlqInt(uint32_t v) {
- bool result = true;
- while ((v & 0xFFFFFF80UL) != 0UL) {
- result &= PutAligned<uint8_t>(static_cast<uint8_t>((v & 0x7F) | 0x80), 1);
- v >>= 7;
- }
- result &= PutAligned<uint8_t>(static_cast<uint8_t>(v & 0x7F), 1);
- return result;
-}
+template <typename Int>
+inline bool BitWriter::PutVlqInt(Int v) {
+ static_assert(std::is_integral_v<Int>);
-inline bool BitReader::GetVlqInt(uint32_t* v) {
- uint32_t tmp = 0;
+ constexpr auto kBufferSize = kMaxLEB128ByteLenFor<Int>;
- for (int i = 0; i < kMaxVlqByteLength; i++) {
- uint8_t byte = 0;
- if (ARROW_PREDICT_FALSE(!GetAligned<uint8_t>(1, &byte))) {
- return false;
- }
- tmp |= static_cast<uint32_t>(byte & 0x7F) << (7 * i);
+ uint8_t buffer[kBufferSize] = {};
+ const auto bytes_written = WriteLEB128(v, buffer, kBufferSize);
+ ARROW_DCHECK_LE(bytes_written, kBufferSize);
+ ARROW_DCHECK_GT(bytes_written, 0); // Cannot fail since we gave max space
- if ((byte & 0x80) == 0) {
- *v = tmp;
- return true;
+ for (int i = 0; i < bytes_written; ++i) {
+ const bool success = PutAligned(buffer[i], 1);
+ if (ARROW_PREDICT_FALSE(!success)) {
+ return false;
}
}
- return false;
-}
-
-inline bool BitWriter::PutZigZagVlqInt(int32_t v) {
- uint32_t u_v = ::arrow::util::SafeCopy<uint32_t>(v);
- u_v = (u_v << 1) ^ static_cast<uint32_t>(v >> 31);
- return PutVlqInt(u_v);
-}
-
-inline bool BitReader::GetZigZagVlqInt(int32_t* v) {
- uint32_t u;
- if (!GetVlqInt(&u)) return false;
- u = (u >> 1) ^ (~(u & 1) + 1);
- *v = ::arrow::util::SafeCopy<int32_t>(u);
return true;
}
-inline bool BitWriter::PutVlqInt(uint64_t v) {
- bool result = true;
- while ((v & 0xFFFFFFFFFFFFFF80ULL) != 0ULL) {
- result &= PutAligned<uint8_t>(static_cast<uint8_t>((v & 0x7F) | 0x80), 1);
- v >>= 7;
+template <typename Int>
+inline bool BitReader::GetVlqInt(Int* v) {
+ static_assert(std::is_integral_v<Int>);
Review Comment:
We can, but we don't have to. The code safely works for extracting a
positive number into a signed integer type (if it fits, but that's true for all
types).
--
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]
