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

[刘可 <liuke.gehry@bytedance.com>]

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.