[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 updated https://github.com/llvm/llvm-project/pull/69967

>From 3260f7102c7569ddbcbd1bd75745da18a3b14b61 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  | 22 ++++++++++++++++++-
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index 91935162f02383a..14c681db3e98c6a 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_short = !__is_long();
+    pointer __old_p        = __was_short ? __get_short_pointer() : __get_long_pointer();
+    size_type __cap =
+        __was_short
+            ? __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_short = !__is_long();
+    pointer __old_p        = __was_short ? __get_short_pointer() : __get_long_pointer();
+    size_type __cap =
+        __was_short
+            ? __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..9767ef91f82ed00 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
@@ -14,6 +14,7 @@
 #include <cassert>
 #include <cstddef>
 #include <string>
+#include <iostream>
 
 #include "test_macros.h"
 
@@ -26,7 +27,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 +41,25 @@ 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::size_t sso_capacity = std::string().capacity();
+    const std::string input(sso_capacity, 'a');
+    std::string s;
+    s = input.c_str();
+    std::cout << "Have capacity = " << s.capacity() << " and size = " << s.size() << std::endl;
+    assert(s.capacity() == input.size());
+  }
+}
+
+int main(int, char**) {
+  test_size_wrt_alignment();
+  test_resize_from_small_size();
 
   return 0;
 }