[libc-commits] [libc] a81db64 - [libc][cpp::string] Allocate fewer temp strings in operator= and += (#210895)
via libc-commits
libc-commits at lists.llvm.org
Sun Jul 26 07:25:54 PDT 2026
Author: Jackson Stogel
Date: 2026-07-26T07:25:50-07:00
New Revision: a81db64570f94c2ca8ac0f598c0b5bba1a7ae59e
URL: https://github.com/llvm/llvm-project/commit/a81db64570f94c2ca8ac0f598c0b5bba1a7ae59e
DIFF: https://github.com/llvm/llvm-project/commit/a81db64570f94c2ca8ac0f598c0b5bba1a7ae59e.diff
LOG: [libc][cpp::string] Allocate fewer temp strings in operator= and += (#210895)
This PR generally updates `cpp::string` to avoid incidental allocations.
Specifically, it:
- Updates `opreator=(string_view)` to avoid allocating a temporary
string:
https://github.com/llvm/llvm-project/blob/67ebc4b221c3e94028b33004cd5cd08deee95048/libc/src/__support/CPP/string.h#L106-L108
- Changes `operator+=(const string&)` to accept a `string_view` so that
strings may be appended without allocation.
- Makes the `string(string_view)` constructor explicit. Before, there
were non-obvious allocations because of the implicit conversion.
As a side effect, this PR has to more carefully handle self-assignment
and self-append. This PR updates append and assignment to avoid calling
`realloc` during append / assignment, as this may invalidate input
pointers held by `string_view` if they point to data held by the string.
This also fixes self-assignment, which previously didn't work, eg
`cpp::string s = "abc"; s = s;` would zero out `s`.
Added:
Modified:
libc/src/__support/CPP/CMakeLists.txt
libc/src/__support/CPP/string.h
libc/test/UnitTest/LibcTest.cpp
libc/test/src/__support/CPP/string_test.cpp
libc/test/src/stdlib/realpath_test.cpp
Removed:
################################################################################
diff --git a/libc/src/__support/CPP/CMakeLists.txt b/libc/src/__support/CPP/CMakeLists.txt
index 49cb07f329111..6ae9b44437e52 100644
--- a/libc/src/__support/CPP/CMakeLists.txt
+++ b/libc/src/__support/CPP/CMakeLists.txt
@@ -92,6 +92,7 @@ add_header_library(
libc.src.__support.integer_to_string
libc.src.__support.macros.null_check
libc.src.string.memory_utils.inline_memcpy
+ libc.src.string.memory_utils.inline_memmove
libc.src.string.memory_utils.inline_memset
libc.src.string.string_utils
)
diff --git a/libc/src/__support/CPP/string.h b/libc/src/__support/CPP/string.h
index 0de0e11a583bf..4b66f679c2ada 100644
--- a/libc/src/__support/CPP/string.h
+++ b/libc/src/__support/CPP/string.h
@@ -17,6 +17,7 @@
#include "src/__support/macros/config.h"
#include "src/__support/macros/null_check.h"
#include "src/string/memory_utils/inline_memcpy.h"
+#include "src/string/memory_utils/inline_memmove.h"
#include "src/string/memory_utils/inline_memset.h"
#include "src/string/string_utils.h" // string_length
@@ -33,6 +34,8 @@ char *realloc_or_die(char *ptr, size_t size) {
return reinterpret_cast<char *>(new_ptr);
}
+char *malloc_or_die(size_t size) { return realloc_or_die(nullptr, size); }
+
} // namespace
// This class mimics std::string but does not intend to be a full fledged
@@ -67,6 +70,46 @@ class string {
buffer_[size_] = NULL_CHARACTER;
}
+ // Assigns the new buffer, capacity, and size to this string,
+ // freeing the current internal buffer.
+ void move_assign_from_buffer(char *new_buffer, size_t new_capacity,
+ size_t new_size) {
+ if (buffer_ != get_empty_string())
+ ::free(buffer_);
+
+ buffer_ = new_buffer;
+ size_ = new_size;
+ capacity_ = new_capacity;
+ }
+
+ // The size of the buffer that should be allocated for requested_capacity.
+ LIBC_INLINE static size_t amortized_capacity(size_t requested_capacity) {
+ size_t new_capacity = requested_capacity + 1; // +1 for the terminating '\0'
+
+ // We extend the capacity to amortize buffer_ reallocations.
+ // We choose to augment the value by 11 / 8, this is about +40% and division
+ // by 8 is cheap. We guard the extension so the operation doesn't overflow.
+ if (new_capacity < SIZE_MAX / 11)
+ new_capacity = new_capacity * 11 / 8;
+ return new_capacity;
+ }
+
+ // Replaces the current buffer with a new larger one containing the first
+ // keep_prefix_size bytes of the current buffer concatenated with new_data.
+ LIBC_INLINE void grow_and_replace(size_t keep_prefix_size,
+ cpp::string_view new_data) {
+ size_t new_size = keep_prefix_size + new_data.size();
+ size_t new_capacity = amortized_capacity(new_size);
+ char *new_buffer = malloc_or_die(new_capacity);
+
+ inline_memcpy(new_buffer, buffer_, keep_prefix_size);
+ inline_memcpy(new_buffer + keep_prefix_size, new_data.data(),
+ new_data.size());
+
+ move_assign_from_buffer(new_buffer, new_capacity, new_size);
+ set_size_and_add_null_character(new_size);
+ }
+
public:
LIBC_INLINE constexpr string() {}
LIBC_INLINE string(const string &other) { this->operator+=(other); }
@@ -78,7 +121,7 @@ class string {
resize(count);
inline_memcpy(buffer_, cstr, count);
}
- LIBC_INLINE string(const string_view &view)
+ LIBC_INLINE explicit string(const string_view &view)
: string(view.data(), view.size()) {}
LIBC_INLINE string(const char *cstr)
: string(cstr, ::LIBC_NAMESPACE::internal::string_length(cstr)) {}
@@ -88,31 +131,39 @@ class string {
inline_memset((void *)buffer_, static_cast<uint8_t>(value), size_);
}
- LIBC_INLINE string &operator=(const string &other) {
- resize(0);
- return (*this) += other;
+ LIBC_INLINE string &assign(cpp::string_view view) {
+ if (view.empty()) {
+ set_size_and_add_null_character(0);
+ return *this;
+ }
+
+ if (capacity() < view.size()) {
+ grow_and_replace(/* keep_prefix_size= */ 0, view);
+ return *this;
+ }
+
+ inline_memmove(buffer_, view.data(), view.size());
+ set_size_and_add_null_character(view.size());
+ return *this;
}
+ LIBC_INLINE string &operator=(const string &other) { return assign(other); }
+
LIBC_INLINE string &operator=(char other) {
- resize(0);
- return (*this) += other;
+ return assign(string_view(&other, 1));
}
+ LIBC_INLINE string &operator=(string_view view) { return assign(view); }
+
LIBC_INLINE string &operator=(string &&other) {
- if (buffer_ != get_empty_string())
- ::free(buffer_);
+ if (this == &other)
+ return *this;
- buffer_ = other.buffer_;
- size_ = other.size_;
- capacity_ = other.capacity_;
+ move_assign_from_buffer(other.buffer_, other.capacity_, other.size_);
other.reset_no_deallocate();
return *this;
}
- LIBC_INLINE string &operator=(const string_view &view) {
- return *this = string(view);
- }
-
LIBC_INLINE ~string() {
if (buffer_ != get_empty_string())
::free(buffer_);
@@ -155,18 +206,12 @@ class string {
}
LIBC_INLINE void reserve(size_t new_cap) {
- size_t allocation_size = new_cap + 1; // +1 for terminating '\0'
- if (allocation_size <= capacity_)
+ if (new_cap <= capacity())
return;
-
- // We extend the capacity to amortize buffer_ reallocations.
- // We choose to augment the value by 11 / 8, this is about +40% and division
- // by 8 is cheap. We guard the extension so the operation doesn't overflow.
- if (allocation_size < SIZE_MAX / 11)
- allocation_size = allocation_size * 11 / 8;
+ size_t allocation_size = amortized_capacity(new_cap);
if (buffer_ == get_empty_string()) {
- buffer_ = realloc_or_die(nullptr, allocation_size);
+ buffer_ = malloc_or_die(allocation_size);
buffer_[0] = NULL_CHARACTER;
} else {
buffer_ = realloc_or_die(buffer_, allocation_size);
@@ -197,7 +242,7 @@ class string {
if (buffer_ == get_empty_string()) {
// Ensure the buffer is heap allocated,
// so that it may later be passed to `free`.
- char *res = realloc_or_die(nullptr, 1);
+ char *res = malloc_or_die(1);
res[0] = '\0';
return res;
}
@@ -207,20 +252,25 @@ class string {
return res;
}
- LIBC_INLINE string &operator+=(const string &rhs) {
- const size_t new_size = size_ + rhs.size();
- reserve(new_size);
- inline_memcpy(buffer_ + size_, rhs.data(), rhs.size());
+ LIBC_INLINE string &append(cpp::string_view view) {
+ if (view.empty())
+ return *this;
+
+ if (capacity() - size_ < view.size()) {
+ grow_and_replace(/* keep_prefix_size= */ size_, view);
+ return *this;
+ }
+
+ size_t new_size = size_ + view.size();
+ inline_memcpy(buffer_ + size_, view.data(), view.size());
set_size_and_add_null_character(new_size);
return *this;
}
+ LIBC_INLINE string &operator+=(string_view rhs) { return append(rhs); }
+
LIBC_INLINE string &operator+=(const char c) {
- const size_t new_size = size_ + 1;
- reserve(new_size);
- buffer_[size_] = c;
- set_size_and_add_null_character(new_size);
- return *this;
+ return append(string_view(&c, 1));
}
};
@@ -257,7 +307,7 @@ LIBC_INLINE string operator+(const char *lhs, const string &rhs) {
namespace internal {
template <typename T> string to_dec_string(T value) {
const IntegerToString<T> buffer(value);
- return buffer.view();
+ return string(buffer.view());
}
} // namespace internal
diff --git a/libc/test/UnitTest/LibcTest.cpp b/libc/test/UnitTest/LibcTest.cpp
index b03ceb6c7c77d..d8e5314ece213 100644
--- a/libc/test/UnitTest/LibcTest.cpp
+++ b/libc/test/UnitTest/LibcTest.cpp
@@ -45,7 +45,7 @@ cpp::enable_if_t<(cpp::is_integral_v<T> && (sizeof(T) > sizeof(uint64_t))) ||
cpp::string>
describeValue(T Value) {
const IntegerToString<T, radix::Hex::WithPrefix> buffer(Value);
- return buffer.view();
+ return cpp::string(buffer.view());
}
// When the value is of a standard integral type, just display it as normal.
diff --git a/libc/test/src/__support/CPP/string_test.cpp b/libc/test/src/__support/CPP/string_test.cpp
index ba1992dd2da39..5896dbfff2f97 100644
--- a/libc/test/src/__support/CPP/string_test.cpp
+++ b/libc/test/src/__support/CPP/string_test.cpp
@@ -277,3 +277,65 @@ TEST(LlvmLibcStringTest, ToString) {
}
}
}
+
+TEST(LlvmLibcStringTest, SelfAssignTest) {
+ string_view alphabet("abcdefghijklmnopqrstuvwxyz");
+
+ // Test with a string long enough to where memcpy'ing bytes internal
+ // to the string may fail.
+ string complicated_string;
+ for (size_t i = 0; i < 100; i++)
+ complicated_string += alphabet[i % alphabet.size()];
+
+ string s(complicated_string);
+
+ s = string_view(s).substr(1);
+ ASSERT_EQ(string_view(s), string_view(complicated_string).substr(1));
+}
+
+TEST(LlvmLibcStringTest, SelfAssignAtCapacityTest) {
+ string s("aaa");
+
+ // Append until the string is at its capacity
+ // to exercise assigning past capacity.
+ while (s.size() < s.capacity())
+ s += 'a';
+ ASSERT_EQ(s.capacity(), s.size());
+ size_t cap_before_append = s.capacity();
+
+ // Force a resize by assigning to a longer string.
+ string longer_string(s.size() + 1, 'b');
+ s = string_view(longer_string);
+
+ size_t cap_after_append = s.capacity();
+ ASSERT_EQ(s, longer_string);
+ ASSERT_GT(cap_after_append, cap_before_append);
+}
+
+TEST(LlvmLibcStringTest, SelfMoveAssign) {
+ string s("aaa");
+ s = move(s);
+
+ ASSERT_STREQ(s.c_str(), "aaa");
+}
+
+TEST(LlvmLibcStringTest, SelfAppendAtCapacityTest) {
+ string s("aaa");
+
+ // Append until the string is at its capacity
+ // to exercise self-appending past capacity.
+ while (s.size() < s.capacity())
+ s += 'a';
+ ASSERT_EQ(s.capacity(), s.size());
+
+ string_view view = string_view(s).substr(0, 3);
+ size_t expected_size = s.size() + view.size();
+ size_t cap_before_append = s.capacity();
+
+ s += view;
+
+ size_t cap_after_append = s.capacity();
+ string expected(expected_size, 'a');
+ ASSERT_EQ(s, expected);
+ ASSERT_GT(cap_after_append, cap_before_append);
+}
diff --git a/libc/test/src/stdlib/realpath_test.cpp b/libc/test/src/stdlib/realpath_test.cpp
index c987179ff89cf..f7dc440a26278 100644
--- a/libc/test/src/stdlib/realpath_test.cpp
+++ b/libc/test/src/stdlib/realpath_test.cpp
@@ -118,7 +118,10 @@ class TestDir {
// Returns the absolute path of `relative_path` in this test directory.
cpp::string absolute_path(cpp::string_view relative_path) const {
- return path + "/" + relative_path;
+ cpp::string res = path;
+ res += "/";
+ res += relative_path;
+ return res;
}
// Returns this test directory path as a C string.
More information about the libc-commits
mailing list