Skip to content

Commit d200df0

Browse files
[libcxx] Remove Redundant Reset in ~basic_string (#164718)
8dae17b refactors basic_string for more code reuse. This makes sense in most cases, but has performance overhead in the case of ~basic_string. The refactoring of ~basic_string to call __reset_internal_buffer() added a redundant (inside the destructor) reset of the object, which the optimizer is unable to optimize away in many cases. This patch prevents a ~1% regression we observed on an internal workload when applying the original refactoring. This does slightly pessimize the code readability, but I think this change is worth it given the performance impact. I'm hoping to add a benchmark(s) to the upstream libc++ benchmark suite around string construction/destruction to ensure that this case does not regress as it seems common in real world applications. I will put up a separate PR for that when I figure out a reasonable way to write it.
1 parent fa2c5fe commit d200df0

File tree

1 file changed

+6
-1
lines changed

1 file changed

+6
-1
lines changed

libcxx/include/string

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,7 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
644644
# include <__utility/forward.h>
645645
# include <__utility/is_pointer_in_range.h>
646646
# include <__utility/move.h>
647+
# include <__utility/no_destroy.h>
647648
# include <__utility/scope_guard.h>
648649
# include <__utility/swap.h>
649650
# include <climits>
@@ -918,6 +919,7 @@ private:
918919
__rep() = default;
919920
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__short __r) : __s(__r) {}
920921
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__long __r) : __l(__r) {}
922+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__uninitialized_tag) {}
921923
};
922924

923925
_LIBCPP_COMPRESSED_PAIR(__rep, __rep_, allocator_type, __alloc_);
@@ -1210,7 +1212,10 @@ public:
12101212
}
12111213
# endif // _LIBCPP_CXX03_LANG
12121214

1213-
inline _LIBCPP_CONSTEXPR_SINCE_CXX20 ~basic_string() { __reset_internal_buffer(); }
1215+
// TODO(boomanaiden154): Once we mark this in destructors as dead on return,
1216+
// we can use a normal call to __reset_internal_buffer and remove the extra
1217+
// __rep constructor.
1218+
inline _LIBCPP_CONSTEXPR_SINCE_CXX20 ~basic_string() { __reset_internal_buffer(__rep(__uninitialized_tag())); }
12141219

12151220
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 operator __self_view() const _NOEXCEPT {
12161221
return __self_view(typename __self_view::__assume_valid(), data(), size());

0 commit comments

Comments
 (0)