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

[Aiden Grossman via libcxx-commits]

https://github.com/boomanaiden154 updated https://github.com/llvm/llvm-project/pull/164718

>From 2a97f2d5b48328e40fa1ada827848d2ba633cb02 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <aidengrossman at google.com>
Date: Wed, 22 Oct 2025 22:03:10 +0000
Subject: [PATCH 1/3] [libcxx] Remove Redundant Reset in ~basic_string

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.
---
 libcxx/include/string | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index 33382c7af4b2c..3ac9fd55a7588 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1210,7 +1210,13 @@ public:
   }
 #  endif // _LIBCPP_CXX03_LANG
 
-  inline _LIBCPP_CONSTEXPR_SINCE_CXX20 ~basic_string() { __reset_internal_buffer(); }
+  inline _LIBCPP_CONSTEXPR_SINCE_CXX20 ~basic_string() {
+    // We do not use __reset_internal_buffer() here to avoid the overhead of
+    // resetting the object.
+    __annotate_delete();
+    if (__is_long())
+      __alloc_traits::deallocate(__alloc_, __get_long_pointer(), __get_long_cap());
+  }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 operator __self_view() const _NOEXCEPT {
     return __self_view(typename __self_view::__assume_valid(), data(), size());

>From 9de412d70b04e6d0936ddcab447c194ceafcdf0f Mon Sep 17 00:00:00 2001
From: Aiden Grossman <aidengrossman at google.com>
Date: Thu, 23 Oct 2025 23:01:22 +0000
Subject: [PATCH 2/3] refactor

---
 libcxx/include/string | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index 3ac9fd55a7588..b65badc661f8f 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1210,13 +1210,7 @@ public:
   }
 #  endif // _LIBCPP_CXX03_LANG
 
-  inline _LIBCPP_CONSTEXPR_SINCE_CXX20 ~basic_string() {
-    // We do not use __reset_internal_buffer() here to avoid the overhead of
-    // resetting the object.
-    __annotate_delete();
-    if (__is_long())
-      __alloc_traits::deallocate(__alloc_, __get_long_pointer(), __get_long_cap());
-  }
+  inline _LIBCPP_CONSTEXPR_SINCE_CXX20 ~basic_string() { __deallocate_buffer_if_long(); }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 operator __self_view() const _NOEXCEPT {
     return __self_view(typename __self_view::__assume_valid(), data(), size());

>From a98ac47d26437de6718f54f2ee59beec1329881d Mon Sep 17 00:00:00 2001
From: Aiden Grossman <aidengrossman at google.com>
Date: Mon, 3 Nov 2025 16:31:53 +0000
Subject: [PATCH 3/3] feedback

---
 libcxx/include/string | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index b65badc661f8f..16037b0394a0c 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,7 @@ public:
   }
 #  endif // _LIBCPP_CXX03_LANG
 
-  inline _LIBCPP_CONSTEXPR_SINCE_CXX20 ~basic_string() { __deallocate_buffer_if_long(); }
+  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());