[libcxx-commits] [libcxx] [libc++] Fix shrink_to_fit to swap buffer only when capacity is strictly smaller (PR #127321)
via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Feb 15 05:59:39 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Peng Liu (winner245)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/127321.diff
1 Files Affected:
- (modified) libcxx/include/string (+1-1)
``````````diff
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;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/127321
More information about the libcxx-commits
mailing list