[libcxx-commits] [libcxx] [libc++] Fix shrink_to_fit to swap buffer only when capacity is strictly smaller (PR #127321)

Peng Liu via libcxx-commits libcxx-commits at lists.llvm.org
Sat Feb 15 05:49:55 PST 2025


https://github.com/winner245 created https://github.com/llvm/llvm-project/pull/127321

The current implementation of the `shrink_to_fit()` function of `basic_string` swaps to the newly allocated buffer when the new buffer has the same capacity as the existing one. While this is not incorrect, it is truly unnecessary to swap to an equally-sized buffer. With equal capacity, we should keep using the existing buffer and simply deallocate the new one, avoiding the extra work of copying elements. 

The desired behavior was documented in the following comment within the function:

https://github.com/llvm/llvm-project/blob/61ad08792a86e62309b982189a600f4342a38d91/libcxx/include/string#L3560-L3566

However, the existing implementation did not exactly conform to this guideline, which is a QoI matter. 

This PR modifies the `shrink_to_fit()` function to ensure that the buffer is only swapped when the new allocation is strictly smaller than the existing one. When the capacities are equal, the new buffer will be discarded without copying the elements. This is achieved by including the `==` check in the above conditional logic.

>From 1e960bb7a45c66a6eb33edab91f2f230b231f787 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Sat, 15 Feb 2025 08:44:51 -0500
Subject: [PATCH] Fix shrink_to_fit to swap buffer only when capacity is
 strictly smaller

---
 libcxx/include/string | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index b280f5f458459..d2fe6c0847bd1 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -3560,7 +3560,7 @@ inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocat
     // The Standard mandates shrink_to_fit() does not increase the capacity.
     // With equal capacity keep the existing buffer. This avoids extra work
     // due to swapping the elements.
-    if (__allocation.count - 1 > capacity()) {
+    if (__allocation.count - 1 >= capacity()) {
       __alloc_traits::deallocate(__alloc_, __allocation.ptr, __allocation.count);
       return;
     }



More information about the libcxx-commits mailing list