[libcxx-commits] [libcxx] d200df0 - [libcxx] Remove Redundant Reset in ~basic_string (#164718)

[via libcxx-commits]

Author: Aiden Grossman
Date: 2025-11-03T11:04:22-08:00
New Revision: d200df0557b71fae0d77bc7fc1650d22a05af371

URL: https://github.com/llvm/llvm-project/commit/d200df0557b71fae0d77bc7fc1650d22a05af371
DIFF: https://github.com/llvm/llvm-project/commit/d200df0557b71fae0d77bc7fc1650d22a05af371.diff

LOG: [libcxx] Remove Redundant Reset in ~basic_string (#164718)

8dae17be2991cd7f0d7fd9aa5aecd064520a14f6 refactors basic_string for more
code reuse. This makes sense in most cases, but has performance overhead
in the case of ~basic_string. The refactoring of ~basic_string to call
__reset_internal_buffer() added a redundant (inside the destructor)
reset of the object, which the optimizer is unable to optimize away in
many cases. This patch prevents a ~1% regression we observed on an
internal workload when applying the original refactoring. This does
slightly pessimize the code readability, but I think this change is
worth it given the performance impact.

I'm hoping to add a benchmark(s) to the upstream libc++ benchmark suite
around string construction/destruction to ensure that this case does not
regress as it seems common in real world applications. I will put up a
separate PR for that when I figure out a reasonable way to write it.

Added: 
    

Modified: 
    libcxx/include/string

Removed: 
    


################################################################################
diff  --git a/libcxx/include/string b/libcxx/include/string
index 33382c7af4b2c..ede42467b99fe 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -644,6 +644,7 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
 #  include <__utility/forward.h>
 #  include <__utility/is_pointer_in_range.h>
 #  include <__utility/move.h>
+#  include <__utility/no_destroy.h>
 #  include <__utility/scope_guard.h>
 #  include <__utility/swap.h>
 #  include <climits>
@@ -918,6 +919,7 @@ private:
     __rep() = default;
     _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__short __r) : __s(__r) {}
     _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__long __r) : __l(__r) {}
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__uninitialized_tag) {}
   };
 
   _LIBCPP_COMPRESSED_PAIR(__rep, __rep_, allocator_type, __alloc_);
@@ -1210,7 +1212,10 @@ public:
   }
 #  endif // _LIBCPP_CXX03_LANG
 
-  inline _LIBCPP_CONSTEXPR_SINCE_CXX20 ~basic_string() { __reset_internal_buffer(); }
+  // TODO(boomanaiden154): Once we mark this in destructors as dead on return,
+  // we can use a normal call to __reset_internal_buffer and remove the extra
+  // __rep constructor.
+  inline _LIBCPP_CONSTEXPR_SINCE_CXX20 ~basic_string() { __reset_internal_buffer(__rep(__uninitialized_tag())); }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 operator __self_view() const _NOEXCEPT {
     return __self_view(typename __self_view::__assume_valid(), data(), size());