From a0008deb679480fd30e845d7e52421af72160c2c Mon Sep 17 00:00:00 2001 From: costan Date: Fri, 13 Apr 2018 15:17:46 -0700 Subject: [PATCH] Reimplement ConsumeDecimalNumber. The old implementation caused odd crashes on ARM, which were fixed by changing a local variable type. The main suspect is the use of a static local variable. This CL replaces the static local variable with constexpr, which still ensures the compiler sees the expressions as constants. The CL also replaces Slice operations in the functions' inner loop with iterator-style pointer operations, which can help the compiler generate less code. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=192832175 --- util/logging.cc | 48 +++++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/util/logging.cc b/util/logging.cc index d48b6dd..0c2890f 100644 --- a/util/logging.cc +++ b/util/logging.cc @@ -46,28 +46,38 @@ std::string EscapeString(const Slice& value) { } bool ConsumeDecimalNumber(Slice* in, uint64_t* val) { - uint64_t v = 0; - int digits = 0; - while (!in->empty()) { - char c = (*in)[0]; - if (c >= '0' && c <= '9') { - ++digits; - // |delta| intentionally unit64_t to avoid Android crash (see log). - const uint64_t delta = (c - '0'); - static const uint64_t kMaxUint64 = ~static_cast(0); - if (v > kMaxUint64/10 || - (v == kMaxUint64/10 && delta > kMaxUint64%10)) { - // Overflow - return false; - } - v = (v * 10) + delta; - in->remove_prefix(1); - } else { + // Constants that will be optimized away. + constexpr const uint64_t kMaxUint64 = std::numeric_limits::max(); + constexpr const char kLastDigitOfMaxUint64 = + '0' + static_cast(kMaxUint64 % 10); + + uint64_t value = 0; + + // reinterpret_cast-ing from char* to unsigned char* to avoid signedness. + const unsigned char* start = + reinterpret_cast(in->data()); + + const unsigned char* end = start + in->size(); + const unsigned char* current = start; + for (; current != end; ++current) { + const unsigned char ch = *current; + if (ch < '0' || ch > '9') break; + + // Overflow check. + // kMaxUint64 / 10 is also constant and will be optimized away. + if (value > kMaxUint64 / 10 || + (value == kMaxUint64 / 10 && ch > kLastDigitOfMaxUint64)) { + return false; } + + value = (value * 10) + (ch - '0'); } - *val = v; - return (digits > 0); + + *val = value; + const size_t digits_consumed = current - start; + in->remove_prefix(digits_consumed); + return digits_consumed != 0; } } // namespace leveldb