[libcxx-commits] [libcxx] [libc++] Optimize basic_string::append(Iter, Iter) (PR #169794)

[Nikolas Klauser via libcxx-commits]

https://github.com/philnik777 created https://github.com/llvm/llvm-project/pull/169794

None

>From a878ac3208f3074668c1117063d79cc709a7ef04 Mon Sep 17 00:00:00 2001
From: Nikolas Klauser <nikolasklauser at berlin.de>
Date: Thu, 20 Nov 2025 13:43:58 +0100
Subject: [PATCH] [libc++] Optimize basic_string::append(Iter, Iter)

---
 libcxx/include/string                         | 47 ++++++++++++++-----
 .../benchmarks/containers/string.bench.cpp    | 23 +++++++++
 2 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index 2b3ba6d2d9b62..1becbb7d0eeeb 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1440,24 +1440,45 @@ public:
   template <class _ForwardIterator, __enable_if_t<__has_forward_iterator_category<_ForwardIterator>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string&
   append(_ForwardIterator __first, _ForwardIterator __last) {
-    size_type __sz  = size();
-    size_type __cap = capacity();
-    size_type __n   = static_cast<size_type>(std::distance(__first, __last));
+    const size_type __size = size();
+    const size_type __cap  = capacity();
+    const size_type __n    = static_cast<size_type>(std::distance(__first, __last));
     if (__n == 0)
       return *this;
 
-    if (__string_is_trivial_iterator_v<_ForwardIterator> && !__addr_in_range(*__first)) {
-      if (__cap - __sz < __n)
-        __grow_by_without_replace(__cap, __sz + __n - __cap, __sz, __sz, 0);
-      __annotate_increase(__n);
-      auto __end = __copy_non_overlapping_range(__first, __last, std::__to_address(__get_pointer() + __sz));
-      traits_type::assign(*__end, value_type());
-      __set_size(__sz + __n);
-      return *this;
+    if (__n > __cap - __size) {
+      __long __buffer  = __allocate_long_buffer(__alloc_, __get_amortized_growth_capacity(__size + __n));
+      __buffer.__size_ = __size + __n;
+      auto __guard     = std::__make_exception_guard([&] {
+        __alloc_traits::deallocate(__alloc_, __buffer.__data_, __buffer.__cap_ * __endian_factor);
+      });
+      auto __end       = __copy_non_overlapping_range(__first, __last, std::__to_address(__buffer.__data_) + __size);
+      traits_type::assign(*__end, _CharT());
+      traits_type::copy(std::__to_address(__buffer.__data_), data(), __size);
+      __guard.__complete();
+      __reset_internal_buffer(__buffer);
     } else {
-      const basic_string __temp(__first, __last, __alloc_);
-      return append(__temp.data(), __temp.size());
+      _CharT* const __ptr = std::__to_address(__get_pointer());
+      if constexpr (__libcpp_is_contiguous_iterator<_ForwardIterator>::value &&
+                    is_same<value_type, __remove_cvref_t<decltype(*__first)>>::value) {
+        traits_type::move(__ptr + __size, std::__to_address(__first), __last - __first);
+      } else if constexpr (__has_bidirectional_iterator_category<_ForwardIterator>::value) {
+        auto __out = __ptr + __size + __n;
+        while (__first != __last) {
+          --__last;
+          --__out;
+          traits_type::assign(*__out, *__last);
+        }
+      } else {
+        _CharT __first_val = *__first;
+        ++__first;
+        auto __end = __copy_non_overlapping_range(__first, __last, __ptr + __size + 1);
+        traits_type::assign(__ptr[__size], __first_val);
+      }
+      traits_type::assign(__ptr[__size + __n], _CharT());
+      __set_size(__size + __n);
     }
+    return *this;
   }
 
 #  if _LIBCPP_STD_VER >= 23
diff --git a/libcxx/test/benchmarks/containers/string.bench.cpp b/libcxx/test/benchmarks/containers/string.bench.cpp
index 2484ec8fd955f..6408b2312f022 100644
--- a/libcxx/test/benchmarks/containers/string.bench.cpp
+++ b/libcxx/test/benchmarks/containers/string.bench.cpp
@@ -11,7 +11,10 @@
 #include <algorithm>
 #include <cstdint>
 #include <cstdlib>
+#include <deque>
+#include <iterator>
 #include <new>
+#include <list>
 #include <vector>
 
 #include "../CartesianBenchmarks.h"
@@ -122,6 +125,26 @@ static void BM_StringResizeAndOverwrite(benchmark::State& state) {
 }
 BENCHMARK(BM_StringResizeAndOverwrite);
 
+template <class Container>
+static void BM_StringAppend(benchmark::State& state) {
+  std::string orig_str = "This is some data so the string isn't empty";
+  std::string str = orig_str;
+
+  const char to_append_str[] = "This is a long string to make sure append does some work";
+  Container to_append;
+  std::copy(std::begin(to_append_str), std::end(to_append_str), std::back_inserter(to_append));
+
+  for (auto _ : state) {
+    benchmark::DoNotOptimize(str);
+    str.append(to_append.begin(), to_append.end());
+    benchmark::DoNotOptimize(str);
+    str.resize(orig_str.size());
+  }
+}
+BENCHMARK(BM_StringAppend<std::vector<char>>);
+BENCHMARK(BM_StringAppend<std::deque<char>>);
+BENCHMARK(BM_StringAppend<std::list<char>>);
+
 enum class Length { Empty, Small, Large, Huge };
 struct AllLengths : EnumValuesAsTuple<AllLengths, Length, 4> {
   static constexpr const char* Names[] = {"Empty", "Small", "Large", "Huge"};