[libcxx-commits] [PATCH] D72164: Optimize / partially inline basic_string assignment operator

[Martijn Vels via Phabricator via libcxx-commits]

mvels created this revision.
mvels added a project: libc++.
Herald added a reviewer: mclow.lists.
mvels edited the summary of this revision.
mvels added a reviewer: EricWF.
mvels added a parent revision: D72160: Optimize / partially inline basic_string copy constructor.
mvels abandoned this revision.

This change adds an optimization where assigning a short string value to a short string instance is optimized into a direct memory copy.

'As is', this optimization provides a 2X to 3X+ speed up.

BM_StringAssignStr_Empty_Opaque                     5.23ns ± 0%             2.27ns ± 0%  -56.62%          (p=0.008 n=5+5)
BM_StringAssignStr_Empty_Transparent                5.14ns ± 0%             2.27ns ± 0%  -55.89%          (p=0.008 n=5+5)
BM_StringAssignStr_Small_Opaque                     7.69ns ± 0%             2.27ns ± 0%  -70.56%          (p=0.008 n=5+5)
BM_StringAssignStr_Small_Transparent                7.65ns ± 0%             2.26ns ± 0%  -70.41%          (p=0.008 n=5+5)
BM_StringAssignStr_Large_Opaque                     24.4ns ± 0%             25.6ns ± 0%   +5.11%          (p=0.008 n=5+5)
BM_StringAssignStr_Large_Transparent                22.2ns ± 0%             22.9ns ± 0%   +3.43%          (p=0.008 n=5+5)
BM_StringAssignStr_Huge_Opaque                       320ns ± 3%              320ns ± 4%     ~             (p=0.841 n=5+5)
BM_StringAssignStr_Huge_Transparent                  321ns ± 4%              321ns ± 4%     ~             (p=1.000 n=5+5)

Subsequently changing the assignment operator to be inlined (not externally instantiated) provides a speed up of 3X to 5X.

BM_StringAssignStr_Empty_Opaque                     5.25ns ± 1%             1.71ns ± 0%  -67.33%          (p=0.008 n=5+5)
BM_StringAssignStr_Empty_Transparent                5.13ns ± 0%             1.29ns ± 0%  -74.89%          (p=0.008 n=5+5)
BM_StringAssignStr_Small_Opaque                     7.69ns ± 0%             1.71ns ± 0%  -77.70%          (p=0.008 n=5+5)
BM_StringAssignStr_Small_Transparent                7.64ns ± 0%             1.29ns ± 0%  -83.10%          (p=0.008 n=5+5)
BM_StringAssignStr_Large_Opaque                     24.4ns ± 0%             25.3ns ± 0%   +3.97%          (p=0.008 n=5+5)
BM_StringAssignStr_Large_Transparent                22.2ns ± 0%             22.6ns ± 0%   +2.13%          (p=0.008 n=5+5)
BM_StringAssignStr_Huge_Opaque                       325ns ± 5%              315ns ± 6%     ~             (p=0.222 n=5+5)
BM_StringAssignStr_Huge_Transparent                  325ns ± 5%              316ns ± 5%     ~             (p=0.310 n=5+5)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72164

Files:
  libcxx/include/string


Index: libcxx/include/string
===================================================================
--- libcxx/include/string
+++ libcxx/include/string
@@ -796,6 +796,10 @@
         _NOEXCEPT;
 #endif
 
+    // Optimization opportunity: do not externally instantiate the copy
+    // constructor, which inlines short string initialization. Long string
+    // initialization is delegated to the (external) __init_long()method,
+    // which results in a 3X-4X speed up for SSO initialization.
     basic_string(const basic_string& __str);
     basic_string(const basic_string& __str, const allocator_type& __a);
 
@@ -874,6 +878,7 @@
     _LIBCPP_INLINE_VISIBILITY
     operator __self_view() const _NOEXCEPT { return __self_view(data(), size()); }
 
+    // Optimization opportunity: do not externally instantiate
     basic_string& operator=(const basic_string& __str);
 
     template <class _Tp, class = typename enable_if<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value, void>::type>
@@ -1543,6 +1548,8 @@
     inline
     void __init(size_type __n, value_type __c);
 
+    void __init_long(const basic_string& __str);
+
     template <class _InputIterator>
     inline
     typename enable_if
@@ -1794,6 +1801,20 @@
     traits_type::assign(__p[__sz], value_type());
 }
 
+template <class _CharT, class _Traits, class _Allocator>
+void basic_string<_CharT, _Traits, _Allocator>::__init_long(
+    const basic_string& __str) {
+  const value_type* __s = _VSTD::__to_address(__str.__get_long_pointer());
+  size_type __sz = __str.__get_long_size();
+  size_type __cap = __recommend(__sz);
+  pointer __p = __alloc_traits::allocate(__alloc(), __cap + 1);
+  __set_long_pointer(__p);
+  __set_long_cap(__cap + 1);
+  __set_long_size(__sz);
+  traits_type::copy(_VSTD::__to_address(__p), __s, __sz);
+  traits_type::assign(__p[__sz], value_type());
+}
+
 template <class _CharT, class _Traits, class _Allocator>
 template <class>
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const _CharT* __s, const _Allocator& __a)
@@ -1837,7 +1858,7 @@
     if (!__str.__is_long())
         __r_.first().__r = __str.__r_.first().__r;
     else
-        __init(_VSTD::__to_address(__str.__get_long_pointer()), __str.__get_long_size());
+        __init_long(__str);
 #if _LIBCPP_DEBUG_LEVEL >= 2
     __get_db()->__insert_c(this);
 #endif
@@ -1851,7 +1872,7 @@
     if (!__str.__is_long())
         __r_.first().__r = __str.__r_.first().__r;
     else
-        __init(_VSTD::__to_address(__str.__get_long_pointer()), __str.__get_long_size());
+        __init_long(__str);
 #if _LIBCPP_DEBUG_LEVEL >= 2
     __get_db()->__insert_c(this);
 #endif
@@ -2266,7 +2287,10 @@
     if (this != &__str)
     {
         __copy_assign_alloc(__str);
-        return assign(__str.data(), __str.size());
+        if (__is_long() | __str.__is_long()) {  // LINT: explicit binary or.
+          return assign(__str.data(), __str.size());
+        }
+        __r_.first().__r = __str.__r_.first().__r;
     }
     return *this;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72164.236074.patch
Type: text/x-patch
Size: 3022 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20200103/36154598/attachment-0001.bin>