[libcxx-commits] [libcxx] Change 'grow_by' to use precise size for the first SSO --> long allocation (PR #69967)

[via libcxx-commits]

https://github.com/EricWF created https://github.com/llvm/llvm-project/pull/69967

The logic currently always at least doubles the capacity on growth. However, the SSO size is an implementation detail, and represents nothing about the usage pattern of the string (unlike long capacities).

Further this causes any short or empty initialized string exceeding 22 bytes to be allocated to at least 48 bytes. (22 * 2 rounded up to 48). This is wasteful as there are likely plenty of strings that could be allocated into 40 (glibc) or 32 (tcmalloc) sized allocs.

This patch was originally from mvels.

>From 83edeb004fe3fd8aa3c636490758c4e999ebe4a8 Mon Sep 17 00:00:00 2001
From: Eric Fiselier <eric at efcs.ca>
Date: Mon, 23 Oct 2023 16:32:09 -0400
Subject: [PATCH] Change 'grow_by' to use precise size for the first SSO -->
 long allocation

The logic currently always at least doubles the capacity on growth.
However, the SSO size is an implementation detail, and represents
nothing about the usage pattern of the string (unlike long capacities).

Further this causes any short or empty initialized string exceeding
22 bytes to be allocated to at least 48 bytes. (22 * 2 rounded up to 48).
This is wasteful as there are likely plenty of strings that could be allocated
into 40 (glibc) or 32 (tcmalloc) sized allocs.

This patch was originally from mvels.
---
 libcxx/include/string                         | 22 ++++++++++++-------
 .../string.capacity/allocation_size.pass.cpp  | 20 ++++++++++++++++-
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index 91935162f02383a..a24e3976b5afe02 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2311,10 +2311,13 @@ basic_string<_CharT, _Traits, _Allocator>::__grow_by_and_replace
     size_type __ms = max_size();
     if (__delta_cap > __ms - __old_cap - 1)
         __throw_length_error();
-    pointer __old_p = __get_pointer();
-    size_type __cap = __old_cap < __ms / 2 - __alignment ?
-                          __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) :
-                          __ms - 1;
+    const bool __was_long = __is_long();
+    pointer __old_p       = !__was_long ? __get_short_pointer() : __get_long_pointer();
+    size_type __cap =
+        !__was_long
+            ? __recommend(__old_cap + __delta_cap)
+            : (__old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap))
+                                                  : __ms - 1);
     auto __allocation = std::__allocate_at_least(__alloc(), __cap + 1);
     pointer __p = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
@@ -2352,10 +2355,13 @@ basic_string<_CharT, _Traits, _Allocator>::__grow_by(size_type __old_cap, size_t
     size_type __ms = max_size();
     if (__delta_cap > __ms - __old_cap)
         __throw_length_error();
-    pointer __old_p = __get_pointer();
-    size_type __cap = __old_cap < __ms / 2 - __alignment ?
-                          __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) :
-                          __ms - 1;
+    const bool __was_long = __is_long();
+    pointer __old_p       = !__was_long ? __get_short_pointer() : __get_long_pointer();
+    size_type __cap =
+        !__was_long
+            ? __recommend(__old_cap + __delta_cap)
+            : (__old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap))
+                                                  : __ms - 1);
     auto __allocation = std::__allocate_at_least(__alloc(), __cap + 1);
     pointer __p = __allocation.ptr;
     __begin_lifetime(__p, __allocation.count);
diff --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
index c7df56c815a8054..787e991725875b5 100644
--- a/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
+++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
@@ -26,7 +26,7 @@ const std::size_t alignment =
     16;
 #endif
 
-int main(int, char**) {
+void test_size_wrt_alignment() {
   std::string input_string;
   input_string.resize(64, 'a');
 
@@ -40,6 +40,24 @@ int main(int, char**) {
   } else {
     assert(test_string.capacity() == expected_align8_size + 8);
   }
+}
+
+
+void test_resize_from_small_size() {
+  // Test that we don't waste additional bytes when growing from the SSO
+  // to a specific size. The size of the SSO is an implementation detail
+  // and shouldn't be taken into account when deciding on the next size.
+  {
+    const std::string input(22, 'a');
+    std::string s;
+    s = input.c_str();
+    assert(s.capacity() == input.size());
+  }
+}
+
+int main(int, char**) {
+  test_size_wrt_alignment();
+  test_resize_from_small_size();
 
   return 0;
 }