Optimize absl::AlphaNum

[MBkkt]

Few notes:

• Digits should be different size for different types of arguments
• I didn't try to measure performance difference, I can be wrong
• Maybe needs to remove default initialization for array, how?

class AlphaNum {
  using Digits = std::array<char, numbers_internal::kFastToBufferSize>;

 public:
  // No bool ctor -- bools convert to an integral type.
  // A bool ctor would also convert incoming pointers (bletch).

  AlphaNum(int x, Digits&& digits = {})  // NOLINT(runtime/explicit)
      : piece_(&digits[0],
               static_cast<size_t>(
                   numbers_internal::FastIntToBuffer(x, &digits[0]) -
                   &digits[0])) {}
  AlphaNum(unsigned int x, Digits&& digits = {})  // NOLINT(runtime/explicit)
      : piece_(&digits[0],
               static_cast<size_t>(
                   numbers_internal::FastIntToBuffer(x, &digits[0]) -
                   &digits[0])) {}
  AlphaNum(long x, Digits&& digits = {})  // NOLINT(*)
      : piece_(&digits[0],
               static_cast<size_t>(
                   numbers_internal::FastIntToBuffer(x, &digits[0]) -
                   &digits[0])) {}
  AlphaNum(unsigned long x, Digits&& digits = {})  // NOLINT(*)
      : piece_(&digits[0],
               static_cast<size_t>(
                   numbers_internal::FastIntToBuffer(x, &digits[0]) -
                   &digits[0])) {}
  AlphaNum(long long x, Digits&& digits = {})  // NOLINT(*)
      : piece_(&digits[0],
               static_cast<size_t>(
                   numbers_internal::FastIntToBuffer(x, &digits[0]) -
                   &digits[0])) {}
  AlphaNum(unsigned long long x, Digits&& digits = {})  // NOLINT(*)
      : piece_(&digits[0],
               static_cast<size_t>(
                   numbers_internal::FastIntToBuffer(x, &digits[0]) -
                   &digits[0])) {}

  AlphaNum(float f, Digits&& digits = {})  // NOLINT(runtime/explicit)
      : piece_(&digits[0], numbers_internal::SixDigitsToBuffer(f, &digits[0])) {
  }
  AlphaNum(double f, Digits&& digits = {})  // NOLINT(runtime/explicit)
      : piece_(&digits[0], numbers_internal::SixDigitsToBuffer(f, &digits[0])) {
  }

  template <size_t size>
  AlphaNum(  // NOLINT(runtime/explicit)
      const strings_internal::AlphaNumBuffer<size>& buf
          ABSL_ATTRIBUTE_LIFETIME_BOUND)
      : piece_(&buf.data[0], buf.size) {}

  AlphaNum(const char* c_str  // NOLINT(runtime/explicit)
               ABSL_ATTRIBUTE_LIFETIME_BOUND)
      : piece_(NullSafeStringView(c_str)) {}
  AlphaNum(absl::string_view pc  // NOLINT(runtime/explicit)
               ABSL_ATTRIBUTE_LIFETIME_BOUND)
      : piece_(pc) {}

  template <typename T, typename = typename std::enable_if<
                            strings_internal::HasAbslStringify<T>::value>::type>
  AlphaNum(  // NOLINT(runtime/explicit)
      const T& v ABSL_ATTRIBUTE_LIFETIME_BOUND,
      strings_internal::StringifySink&& sink ABSL_ATTRIBUTE_LIFETIME_BOUND = {})
      : piece_(strings_internal::ExtractStringification(sink, v)) {}

  template <typename Allocator>
  AlphaNum(  // NOLINT(runtime/explicit)
      const std::basic_string<char, std::char_traits<char>, Allocator>& str
          ABSL_ATTRIBUTE_LIFETIME_BOUND)
      : piece_(str) {}

  // Use string literals ":" instead of character literals ':'.
  AlphaNum(char c) = delete;  // NOLINT(runtime/explicit)

  AlphaNum(const AlphaNum&) = delete;
  AlphaNum& operator=(const AlphaNum&) = delete;

  absl::string_view::size_type size() const { return piece_.size(); }
  const char* data() const { return piece_.data(); }
  absl::string_view Piece() const { return piece_; }

  // Normal enums are already handled by the integer formatters.
  // This overload matches only scoped enums.
  template <typename T,
            typename = typename std::enable_if<
                std::is_enum<T>{} && !std::is_convertible<T, int>{} &&
                !strings_internal::HasAbslStringify<T>::value>::type>
  AlphaNum(T e, Digits&& digits = {})  // NOLINT(runtime/explicit)
      : piece_(&digits[0],
               static_cast<size_t>(
                   numbers_internal::FastIntToBuffer(
                       static_cast<typename std::underlying_type<T>::type>(e),
                       &digits[0]) -
                   &digits[0])) {}

  // vector<bool>::reference and const_reference require special help to
  // convert to `AlphaNum` because it requires two user defined conversions.
  template <
      typename T,
      typename std::enable_if<
          std::is_class<T>::value &&
          (std::is_same<T, std::vector<bool>::reference>::value ||
           std::is_same<T, std::vector<bool>::const_reference>::value)>::type* =
          nullptr>
  AlphaNum(T e, Digits&& digits = {})  // NOLINT(runtime/explicit)
      : piece_(&digits[0],
               static_cast<size_t>(
                   numbers_internal::FastIntToBuffer(
                       static_cast<int>(static_cast<bool>(e)), &digits[0]) -
                   &digits[0])) {}

 private:
  absl::string_view piece_;
};

[derekmauro]

Please correct me if I'm wrong, but as I understand it, this optimization is about potentially saving stack space? Is there any other benefit?

[MBkkt]

Don't allocate stack space at all in case string_view/etc?
But maybe compiler already do it.
I don't think it has other benefits.

I think better to just try to measure. Unfortunately build absl benchmarks didn't work for me for some reason that I already don't remember.

[derekmauro]

I think the benefit from saving stack space is minimal, and it also has a small code complexity cost. You still have to bump the stack pointer, the only difference is by how much. AlphaNum is used as a temporary function parameter, so there is even a limit to how many you can have at once.

I'm not going to try to measure this, but if someone finds a real benefit, let us know.

[AntonGitName]

Thanks MBkkt and derekmauro

I measured this a couple of months ago and there was no noticeable difference for the reasons Derek pointed out (neither cpu nor binary size). Additionally, it is hard to make a change to AlphaNum's interface without breaking existing dependencies. For instance, the proposed change would make the following code buggy (which unfortunately works today):

auto a = [] () { return absl::AlphaNum(239); } ();
std::string s = absl::StrCat(a);  // Digits are already gone =/

https://godbolt.org/z/TPGMcK7qv