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

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Eric (EricWF)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/69967.diff


2 Files Affected:

- (modified) libcxx/include/string (+14-8) 
- (modified) libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp (+18-1) 


``````````diff
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..c65c76bcba3da21 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,23 @@ 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;
 }

``````````

</details>


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