Re: [External] Re: [PATCH] libstdc++: Optimize 'to_string<int>' with numeric_limits instead of __to_chars_len

Re: [External] Re: [PATCH] libstdc++: Optimize 'to_string<int>' with numeric_limits instead of __to_chars_len

[刘可]

Thank you for your review, and I apologize for my mistake. I have updated
and tested it！
GCC5 has implemented 'SSO'. The high version of GCC uses the new ABI by
default. The length of small string local buffer is 15, which is enough to
store an integer. So we can use 'numeric_limits<int>::digits+1' to get the
max length of int instead of dynamically obtaining the length of the
integer through __to_chars_len. In this way, we will get a performance
improvement of about 15%.

Before optimization:
--------------------------------------------------------------------------------
Benchmark                Time                CPU               Iterations
--------------------------------------------------------------------------------
# to_string<int>
Int2String               191785 ns       191780 ns             3645
# to_string<unsigned>
Unsigned2String    159605 ns       159599 ns             4367

After optimization:
--------------------------------------------------------------------------------
Benchmark                Time                CPU               Iterations
--------------------------------------------------------------------------------
# to_string<int>
Int2String               159382 ns       159381 ns             4354
# to_string<unsigned>
Unsigned2String    136744 ns       136742 ns             5144

2020-09-13 Liuke <liuke.gehry@bytedance.com>

libstdc++-v3/ChangeLog:

        * include/bits/basic_string.h: Use
std::numeric_limits<int>::digits10 instead of __to_chars_len.

Diff:
diff --git a/libstdc++-v3/include/bits/basic_string.h
b/libstdc++-v3/include/bits/basic_string.h
index b61fe05efcf..1040b953c39 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -51,6 +51,7 @@
 #if ! _GLIBCXX_USE_CXX11_ABI
 # include "cow_string.h"
 #else
+#include <limits>
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -3721,7 +3722,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
   {
     const bool __neg = __val < 0;
     const unsigned __uval = __neg ? (unsigned)~__val + 1u : __val;
+#if _GLIBCXX_USE_CXX11_ABI
+    const auto __len = std::numeric_limits<int>::digits10 + 1;
+#else
     const auto __len = __detail::__to_chars_len(__uval);
+#endif
     string __str(__neg + __len, '-');
     __detail::__to_chars_10_impl(&__str[__neg], __len, __uval);
     return __str;
@@ -3730,7 +3735,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
   inline string
   to_string(unsigned __val)
   {
+#if _GLIBCXX_USE_CXX11_ABI
+    string __str(std::numeric_limits<unsigned>::digits10 + 1, '\0');
+#else
     string __str(__detail::__to_chars_len(__val), '\0');
+#endif
     __detail::__to_chars_10_impl(&__str[0], __str.size(), __val);
     return __str;
   }
On Wed, Sep 15, 2021, 00:57 <jwakely@redhat.com> wrote:
Please CC libstdc++ patches to the libstdc++ list, or they won't get
reviewed (because I don't subscribe to gcc-patches). GCC 5 does implement
SSO but it's only used conditionally. Your patch uses numeric_limits
unconditionally, which will result in over-allocation for COW strings.
There also seems to be a syntax error in the unsigned overload, was this
patch tested? Minor: the "component" tag in the subject should be just
"libstdc++:" without the "-v3" part.

Re: [External] Re: [PATCH] libstdc++: Optimize 'to_string<int>' with numeric_limits instead of __to_chars_len

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

N.B. Please CC *both* the libstdc++ list and the gcc-patches list, as
per https://gcc.gnu.org/lists.html

On Wed, 15 Sept 2021 at 14:02, 刘可 wrote:
>
> Thank you for your review, and I apologize for my mistake. I have updated and tested it！

Hmm, it doesn't work though. How did you test it?

For to_string(int) the string will be padded with '-' characters, i.e.
std::to_string(1) returns "1---------" and to_string(-1) returns
"-1---------" and to_string(100) returns "1--------00" !

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2702 bytes --]

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index b61fe05efcf..e1fd42aea1a 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -3716,41 +3716,82 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 
   // DR 1261. Insufficent overloads for to_string / to_wstring
 
+  namespace __detail
+  {
+    template<typename _Tp>
+      inline unsigned
+      __to_string_len(_Tp __val) noexcept
+      {
+#if _GLIBCXX_USE_CXX11_ABI
+	// Any 32-bit integer value fits in the 15-byte SSO buffer,
+	// so don't bother counting how many chars are needed.
+	if _GLIBCXX17_CONSTEXPR (sizeof(_Tp) * __CHAR_BIT_  <= 32)
+	  return 9; // std::numeric_limits<uint32_t>::digits10
+	else
+#endif
+	return __detail::__to_chars_len(__uval);
+      }
+
+    inline void
+    __to_string_trim(string& __s) noexcept
+    {
+#if _GLIBCXX_USE_CXX11_ABI
+      ???
+#endif
+    }
+  }
+
   inline string
   to_string(int __val)
+#if _GLIBCXX_USE_CXX11_ABI
+  noexcept
+#endif
   {
     const bool __neg = __val < 0;
     const unsigned __uval = __neg ? (unsigned)~__val + 1u : __val;
-    const auto __len = __detail::__to_chars_len(__uval);
+    const auto __len = __detail::__to_string_len(__uval);
     string __str(__neg + __len, '-');
     __detail::__to_chars_10_impl(&__str[__neg], __len, __uval);
+    __detail::__to_string_trim(__str);
     return __str;
   }
 
   inline string
   to_string(unsigned __val)
+#if _GLIBCXX_USE_CXX11_ABI
+  noexcept
+#endif
   {
-    string __str(__detail::__to_chars_len(__val), '\0');
+    string __str(__detail::__to_string_len(__val), '\0');
     __detail::__to_chars_10_impl(&__str[0], __str.size(), __val);
+    __detail::__to_string_trim(__str);
     return __str;
   }
 
   inline string
   to_string(long __val)
+#if _GLIBCXX_USE_CXX11_ABI && __SIZEOF_LONG__ == __SIZEOF_INT__
+  noexcept
+#endif
   {
     const bool __neg = __val < 0;
     const unsigned long __uval = __neg ? (unsigned long)~__val + 1ul : __val;
-    const auto __len = __detail::__to_chars_len(__uval);
+    const auto __len = __detail::__to_string_len(__uval);
     string __str(__neg + __len, '-');
     __detail::__to_chars_10_impl(&__str[__neg], __len, __uval);
+    __detail::__to_string_trim(__str);
     return __str;
   }
 
   inline string
   to_string(unsigned long __val)
+#if _GLIBCXX_USE_CXX11_ABI && __SIZEOF_LONG__ == __SIZEOF_INT__
+  noexcept
+#endif
   {
-    string __str(__detail::__to_chars_len(__val), '\0');
+    string __str(__detail::__to_string_len(__val), '\0');
     __detail::__to_chars_10_impl(&__str[0], __str.size(), __val);
+    __detail::__to_string_trim(__str);
     return __str;
   }