[libcxx-commits] [libcxx] [libc++] Optimize basic_string::operator= (PR #224535)

[via libcxx-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

<details>
<summary>Changes</summary>

This improves a few things in the long string code path. Specifically:
- The null terminator is copied from the assigned-from string instead of setting it manually
- various branches have been removed via `__assert`s that are known
- `memmove` is tail-called by updating the pointer and size of the string first and not returning `this` anymore
- the allocating path is outlined into a separate function, which basically amounts to manual shrink-wrapping (this should be removed once LLVM is able to do this on its own)

---
Full diff: https://github.com/llvm/llvm-project/pull/224535.diff


1 Files Affected:

- (modified) libcxx/include/string (+29-13) 


``````````diff
diff --git a/libcxx/include/string b/libcxx/include/string
index 1f52bc02e0ea0..711fda966f725 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2374,7 +2374,7 @@ private:
   // have proof that the input does not alias the current instance.
   // For example, operator=(basic_string) performs a 'self' check.
   template <bool __is_short>
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE basic_string& __assign_no_alias(const value_type* __s, size_type __n);
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE void __assign_no_alias(const value_type* __s, size_type __n);
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __erase_to_end(size_type __pos) {
     _LIBCPP_ASSERT_INTERNAL(__pos <= capacity(), "Trying to erase at position outside the strings capacity!");
@@ -2727,28 +2727,40 @@ basic_string<_CharT, _Traits, _Allocator>::__grow_by_without_replace(
 
 template <class _CharT, class _Traits, class _Allocator>
 template <bool __is_short>
-_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE basic_string<_CharT, _Traits, _Allocator>&
+_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE void
 basic_string<_CharT, _Traits, _Allocator>::__assign_no_alias(const value_type* __s, size_type __n) {
   const auto __cap  = __is_short ? static_cast<size_type>(__min_cap) : __get_long_cap();
-  const auto __size = __is_short ? __get_short_size() : __get_long_size();
   if (__n >= __cap) {
-    __grow_by_and_replace(__cap - 1, __n - __cap + 1, __size, 0, __size, __n, __s);
-    return *this;
+    return [] [[__gnu__::__noinline__]] (basic_string& __this, const value_type* __ls, size_type __ln) static {
+      std::__assume(__this.__is_long() == !__is_short);
+      std::__assume(!__alloc_traits::is_always_equal::value || __ln <= __this.max_size());
+      __this.__annotate_delete();
+      if _LIBCPP_CONSTEXPR (!__is_short)
+        __alloc_traits::deallocate(__this.__alloc_, __this.__get_long_pointer(), __this.__get_long_cap());
+      auto __guard   = std::__make_exception_guard([&] {
+        if _LIBCPP_CONSTEXPR (!__is_short)
+          __this.__rep_ = __rep();
+      });
+      auto __new_cap = __this.__get_amortized_growth_capacity(__ln);
+      std::__assume(__new_cap <= __this.max_size());
+      __this.__init_internal_buffer(__new_cap);
+      __this.__set_long_size(__ln);
+      __guard.__complete();
+      traits_type::copy(std::__to_address(__this.__get_long_pointer()), __ls, __ln + 1);
+    }(*this, __s, __n);
   }
 
   __annotate_delete();
   auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
   pointer __p;
-  if (__is_short) {
+  if _LIBCPP_CONSTEXPR (__is_short) {
     __p = __get_short_pointer();
     __set_short_size(__n);
   } else {
     __p = __get_long_pointer();
     __set_long_size(__n);
   }
-  traits_type::copy(std::__to_address(__p), __s, __n);
-  traits_type::assign(__p[__n], value_type());
-  return *this;
+  traits_type::copy(std::__to_address(__p), __s, __n + 1);
 }
 
 template <class _CharT, class _Traits, class _Allocator>
@@ -2826,11 +2838,15 @@ basic_string<_CharT, _Traits, _Allocator>::operator=(const basic_string& __str)
 
   __copy_assign_alloc(__str);
 
-  if (__is_long())
-    return __assign_no_alias<false>(__str.data(), __str.size());
+  if (__is_long()) {
+     __assign_no_alias<false>(__str.data(), __str.size());
+     return *this;
+  }
 
-  if (__str.__is_long())
-    return __assign_no_alias<true>(__str.data(), __str.size());
+  if (__str.__is_long()) {
+    __assign_no_alias<true>(__str.data(), __str.size());
+    return *this;
+  }
 
   __annotate_delete();
   auto __guard = std::__make_scope_guard(__annotate_new_size(*this));

``````````

</details>


https://github.com/llvm/llvm-project/pull/224535