Skip to content

Commit 2352fdd

Browse files
committed
Revert "[libc++] Optimize vector growing of trivially relocatable types (llvm#76657)"
Broke sanitizer bots: https://lab.llvm.org/buildbot/#/builders/5/builds/40641 This reverts commit 67eee4a.
1 parent 095367a commit 2352fdd

File tree

13 files changed

+62
-295
lines changed

13 files changed

+62
-295
lines changed

libcxx/benchmarks/ContainerBenchmarks.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ void BM_ConstructFromRange(benchmark::State& st, Container, GenInputs gen) {
8080
}
8181

8282
template <class Container>
83-
void BM_Pushback_no_grow(benchmark::State& state, Container c) {
83+
void BM_Pushback(benchmark::State& state, Container c) {
8484
int count = state.range(0);
8585
c.reserve(count);
8686
while (state.KeepRunningBatch(count)) {

libcxx/benchmarks/vector_operations.bench.cpp

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#include <cstdint>
22
#include <cstdlib>
33
#include <cstring>
4-
#include <deque>
54
#include <functional>
65
#include <vector>
76

@@ -40,21 +39,6 @@ BENCHMARK_CAPTURE(BM_ConstructFromRange, vector_size_t, std::vector<size_t>{}, g
4039
BENCHMARK_CAPTURE(BM_ConstructFromRange, vector_string, std::vector<std::string>{}, getRandomStringInputs)
4140
->Arg(TestNumInputs);
4241

43-
BENCHMARK_CAPTURE(BM_Pushback_no_grow, vector_int, std::vector<int>{})->Arg(TestNumInputs);
44-
45-
template <class T>
46-
void bm_grow(benchmark::State& state) {
47-
for (auto _ : state) {
48-
std::vector<T> vec;
49-
benchmark::DoNotOptimize(vec);
50-
for (size_t i = 0; i != 2048; ++i)
51-
vec.emplace_back();
52-
benchmark::DoNotOptimize(vec);
53-
}
54-
}
55-
BENCHMARK(bm_grow<int>);
56-
BENCHMARK(bm_grow<std::string>);
57-
BENCHMARK(bm_grow<std::unique_ptr<int>>);
58-
BENCHMARK(bm_grow<std::deque<int>>);
42+
BENCHMARK_CAPTURE(BM_Pushback, vector_int, std::vector<int>{})->Arg(TestNumInputs);
5943

6044
BENCHMARK_MAIN();

libcxx/docs/ReleaseNotes/19.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ Implemented Papers
4444

4545
Improvements and New Features
4646
-----------------------------
47+
TODO
4748

48-
- The performance of growing ``std::vector`` has been improved for trivially relocatable types.
4949

5050
Deprecations and Removals
5151
-------------------------

libcxx/include/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,6 @@ set(files
808808
__type_traits/is_trivially_lexicographically_comparable.h
809809
__type_traits/is_trivially_move_assignable.h
810810
__type_traits/is_trivially_move_constructible.h
811-
__type_traits/is_trivially_relocatable.h
812811
__type_traits/is_unbounded_array.h
813812
__type_traits/is_union.h
814813
__type_traits/is_unsigned.h

libcxx/include/__memory/uninitialized_algorithms.h

Lines changed: 44 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
#include <__type_traits/is_trivially_copy_constructible.h>
3030
#include <__type_traits/is_trivially_move_assignable.h>
3131
#include <__type_traits/is_trivially_move_constructible.h>
32-
#include <__type_traits/is_trivially_relocatable.h>
3332
#include <__type_traits/is_unbounded_array.h>
3433
#include <__type_traits/negation.h>
3534
#include <__type_traits/remove_const.h>
@@ -595,56 +594,60 @@ __uninitialized_allocator_copy(_Alloc& __alloc, _Iter1 __first1, _Sent1 __last1,
595594
return std::__rewrap_iter(__first2, __result);
596595
}
597596

598-
template <class _Alloc, class _Type>
599-
struct __allocator_has_trivial_move_construct : _Not<__has_construct<_Alloc, _Type*, _Type&&> > {};
600-
601-
template <class _Type>
602-
struct __allocator_has_trivial_move_construct<allocator<_Type>, _Type> : true_type {};
603-
604-
template <class _Alloc, class _Tp>
605-
struct __allocator_has_trivial_destroy : _Not<__has_destroy<_Alloc, _Tp*> > {};
606-
607-
template <class _Tp, class _Up>
608-
struct __allocator_has_trivial_destroy<allocator<_Tp>, _Up> : true_type {};
609-
610-
// __uninitialized_allocator_relocate relocates the objects in [__first, __last) into __result.
611-
// Relocation means that the objects in [__first, __last) are placed into __result as-if by move-construct and destroy,
612-
// except that the move constructor and destructor may never be called if they are known to be equivalent to a memcpy.
613-
//
614-
// Preconditions: __result doesn't contain any objects and [__first, __last) contains objects
615-
// Postconditions: __result contains the objects from [__first, __last) and
616-
// [__first, __last) doesn't contain any objects
597+
// Move-construct the elements [__first1, __last1) into [__first2, __first2 + N)
598+
// if the move constructor is noexcept, where N is distance(__first1, __last1).
617599
//
618-
// The strong exception guarantee is provided if any of the following are true:
619-
// - is_nothrow_move_constructible<_Tp>
620-
// - is_copy_constructible<_Tp>
621-
// - __libcpp_is_trivially_relocatable<_Tp>
622-
template <class _Alloc, class _Tp>
623-
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
624-
__uninitialized_allocator_relocate(_Alloc& __alloc, _Tp* __first, _Tp* __last, _Tp* __result) {
600+
// Otherwise try to copy all elements. If an exception is thrown the already copied
601+
// elements are destroyed in reverse order of their construction.
602+
template <class _Alloc, class _Iter1, class _Sent1, class _Iter2>
603+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Iter2
604+
__uninitialized_allocator_move_if_noexcept(_Alloc& __alloc, _Iter1 __first1, _Sent1 __last1, _Iter2 __first2) {
625605
static_assert(__is_cpp17_move_insertable<_Alloc>::value,
626606
"The specified type does not meet the requirements of Cpp17MoveInsertable");
627-
if (__libcpp_is_constant_evaluated() || !__libcpp_is_trivially_relocatable<_Tp>::value ||
628-
!__allocator_has_trivial_move_construct<_Alloc, _Tp>::value ||
629-
!__allocator_has_trivial_destroy<_Alloc, _Tp>::value) {
630-
auto __destruct_first = __result;
631-
auto __guard =
632-
std::__make_exception_guard(_AllocatorDestroyRangeReverse<_Alloc, _Tp*>(__alloc, __destruct_first, __result));
633-
while (__first != __last) {
607+
auto __destruct_first = __first2;
608+
auto __guard =
609+
std::__make_exception_guard(_AllocatorDestroyRangeReverse<_Alloc, _Iter2>(__alloc, __destruct_first, __first2));
610+
while (__first1 != __last1) {
634611
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
635-
allocator_traits<_Alloc>::construct(__alloc, __result, std::move_if_noexcept(*__first));
612+
allocator_traits<_Alloc>::construct(__alloc, std::__to_address(__first2), std::move_if_noexcept(*__first1));
636613
#else
637-
allocator_traits<_Alloc>::construct(__alloc, __result, std::move(*__first));
614+
allocator_traits<_Alloc>::construct(__alloc, std::__to_address(__first2), std::move(*__first1));
638615
#endif
639-
++__first;
640-
++__result;
616+
++__first1;
617+
++__first2;
618+
}
619+
__guard.__complete();
620+
return __first2;
621+
}
622+
623+
template <class _Alloc, class _Type>
624+
struct __allocator_has_trivial_move_construct : _Not<__has_construct<_Alloc, _Type*, _Type&&> > {};
625+
626+
template <class _Type>
627+
struct __allocator_has_trivial_move_construct<allocator<_Type>, _Type> : true_type {};
628+
629+
#ifndef _LIBCPP_COMPILER_GCC
630+
template <
631+
class _Alloc,
632+
class _Iter1,
633+
class _Iter2,
634+
class _Type = typename iterator_traits<_Iter1>::value_type,
635+
class = __enable_if_t<is_trivially_move_constructible<_Type>::value && is_trivially_move_assignable<_Type>::value &&
636+
__allocator_has_trivial_move_construct<_Alloc, _Type>::value> >
637+
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Iter2
638+
__uninitialized_allocator_move_if_noexcept(_Alloc&, _Iter1 __first1, _Iter1 __last1, _Iter2 __first2) {
639+
if (__libcpp_is_constant_evaluated()) {
640+
while (__first1 != __last1) {
641+
std::__construct_at(std::__to_address(__first2), std::move(*__first1));
642+
++__first1;
643+
++__first2;
641644
}
642-
__guard.__complete();
643-
std::__allocator_destroy(__alloc, __first, __last);
645+
return __first2;
644646
} else {
645-
__builtin_memcpy(__result, __first, sizeof(_Tp) * (__last - __first));
647+
return std::move(__first1, __last1, __first2);
646648
}
647649
}
650+
#endif // _LIBCPP_COMPILER_GCC
648651

649652
_LIBCPP_END_NAMESPACE_STD
650653

libcxx/include/__memory/unique_ptr.h

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
#include <__memory/compressed_pair.h>
2222
#include <__type_traits/add_lvalue_reference.h>
2323
#include <__type_traits/common_type.h>
24-
#include <__type_traits/conditional.h>
2524
#include <__type_traits/dependent_type.h>
2625
#include <__type_traits/integral_constant.h>
2726
#include <__type_traits/is_array.h>
@@ -34,7 +33,6 @@
3433
#include <__type_traits/is_reference.h>
3534
#include <__type_traits/is_same.h>
3635
#include <__type_traits/is_swappable.h>
37-
#include <__type_traits/is_trivially_relocatable.h>
3836
#include <__type_traits/is_void.h>
3937
#include <__type_traits/remove_extent.h>
4038
#include <__type_traits/type_identity.h>
@@ -131,17 +129,6 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr {
131129

132130
static_assert(!is_rvalue_reference<deleter_type>::value, "the specified deleter type cannot be an rvalue reference");
133131

134-
// A unique_ptr contains the following members which may be trivially relocatable:
135-
// - pointer : this may be trivially relocatable, so it's checked
136-
// - deleter_type: this may be trivially relocatable, so it's checked
137-
//
138-
// This unique_ptr implementation only contains a pointer to the unique object and a deleter, so there are no
139-
// references to itself. This means that the entire structure is trivially relocatable if its members are.
140-
using __trivially_relocatable = __conditional_t<
141-
__libcpp_is_trivially_relocatable<pointer>::value && __libcpp_is_trivially_relocatable<deleter_type>::value,
142-
unique_ptr,
143-
void>;
144-
145132
private:
146133
__compressed_pair<pointer, deleter_type> __ptr_;
147134

@@ -289,17 +276,6 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
289276
typedef _Dp deleter_type;
290277
typedef typename __pointer<_Tp, deleter_type>::type pointer;
291278

292-
// A unique_ptr contains the following members which may be trivially relocatable:
293-
// - pointer : this may be trivially relocatable, so it's checked
294-
// - deleter_type: this may be trivially relocatable, so it's checked
295-
//
296-
// This unique_ptr implementation only contains a pointer to the unique object and a deleter, so there are no
297-
// references to itself. This means that the entire structure is trivially relocatable if its members are.
298-
using __trivially_relocatable = __conditional_t<
299-
__libcpp_is_trivially_relocatable<pointer>::value && __libcpp_is_trivially_relocatable<deleter_type>::value,
300-
unique_ptr,
301-
void>;
302-
303279
private:
304280
__compressed_pair<pointer, deleter_type> __ptr_;
305281

libcxx/include/__type_traits/is_trivially_relocatable.h

Lines changed: 0 additions & 42 deletions
This file was deleted.

libcxx/include/libcxx.imp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,6 @@
796796
{ include: [ "<__type_traits/is_trivially_lexicographically_comparable.h>", "private", "<type_traits>", "public" ] },
797797
{ include: [ "<__type_traits/is_trivially_move_assignable.h>", "private", "<type_traits>", "public" ] },
798798
{ include: [ "<__type_traits/is_trivially_move_constructible.h>", "private", "<type_traits>", "public" ] },
799-
{ include: [ "<__type_traits/is_trivially_relocatable.h>", "private", "<type_traits>", "public" ] },
800799
{ include: [ "<__type_traits/is_unbounded_array.h>", "private", "<type_traits>", "public" ] },
801800
{ include: [ "<__type_traits/is_union.h>", "private", "<type_traits>", "public" ] },
802801
{ include: [ "<__type_traits/is_unsigned.h>", "private", "<type_traits>", "public" ] },

libcxx/include/module.modulemap.in

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1970,7 +1970,6 @@ module std_private_type_traits_is_trivially_destructible [system
19701970
module std_private_type_traits_is_trivially_lexicographically_comparable [system] { header "__type_traits/is_trivially_lexicographically_comparable.h" }
19711971
module std_private_type_traits_is_trivially_move_assignable [system] { header "__type_traits/is_trivially_move_assignable.h" }
19721972
module std_private_type_traits_is_trivially_move_constructible [system] { header "__type_traits/is_trivially_move_constructible.h" }
1973-
module std_private_type_traits_is_trivially_relocatable [system] { header "__type_traits/is_trivially_relocatable.h" }
19741973
module std_private_type_traits_is_unbounded_array [system] { header "__type_traits/is_unbounded_array.h" }
19751974
module std_private_type_traits_is_union [system] { header "__type_traits/is_union.h" }
19761975
module std_private_type_traits_is_unsigned [system] { header "__type_traits/is_unsigned.h" }

libcxx/include/string

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,6 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
599599
#include <__ranges/size.h>
600600
#include <__string/char_traits.h>
601601
#include <__string/extern_template_lists.h>
602-
#include <__type_traits/conditional.h>
603602
#include <__type_traits/is_allocator.h>
604603
#include <__type_traits/is_array.h>
605604
#include <__type_traits/is_convertible.h>
@@ -608,7 +607,6 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
608607
#include <__type_traits/is_same.h>
609608
#include <__type_traits/is_standard_layout.h>
610609
#include <__type_traits/is_trivial.h>
611-
#include <__type_traits/is_trivially_relocatable.h>
612610
#include <__type_traits/noexcept_move_assign_container.h>
613611
#include <__type_traits/remove_cvref.h>
614612
#include <__type_traits/void_t.h>
@@ -726,20 +724,6 @@ public:
726724
typedef typename __alloc_traits::pointer pointer;
727725
typedef typename __alloc_traits::const_pointer const_pointer;
728726

729-
// A basic_string contains the following members which may be trivially relocatable:
730-
// - pointer: is currently assumed to be trivially relocatable, but is still checked in case that changes
731-
// - size_type: is always trivially relocatable, since it has to be an integral type
732-
// - value_type: is always trivially relocatable, since it has to be trivial
733-
// - unsigned char: is a fundamental type, so it's trivially relocatable
734-
// - allocator_type: may or may not be trivially relocatable, so it's checked
735-
//
736-
// This string implementation doesn't contain any references into itself. It only contains a bit that says whether
737-
// it is in small or large string mode, so the entire structure is trivially relocatable if its members are.
738-
using __trivially_relocatable = __conditional_t<
739-
__libcpp_is_trivially_relocatable<allocator_type>::value && __libcpp_is_trivially_relocatable<pointer>::value,
740-
basic_string,
741-
void>;
742-
743727
static_assert((!is_array<value_type>::value), "Character type of basic_string must not be an array");
744728
static_assert((is_standard_layout<value_type>::value), "Character type of basic_string must be standard-layout");
745729
static_assert((is_trivial<value_type>::value), "Character type of basic_string must be trivial");

libcxx/include/vector

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -982,54 +982,37 @@ template <ranges::input_range _Range,
982982
vector(from_range_t, _Range&&, _Alloc = _Alloc()) -> vector<ranges::range_value_t<_Range>, _Alloc>;
983983
#endif
984984

985-
// __swap_out_circular_buffer relocates the objects in [__begin_, __end_) into the front of __v and swaps the buffers of
986-
// *this and __v. It is assumed that __v provides space for exactly (__end_ - __begin_) objects in the front. This
987-
// function has a strong exception guarantee.
988985
template <class _Tp, class _Allocator>
989986
_LIBCPP_CONSTEXPR_SINCE_CXX20 void
990987
vector<_Tp, _Allocator>::__swap_out_circular_buffer(__split_buffer<value_type, allocator_type&>& __v) {
991988
__annotate_delete();
992-
auto __new_begin = __v.__begin_ - (__end_ - __begin_);
993-
std::__uninitialized_allocator_relocate(
994-
__alloc(), std::__to_address(__begin_), std::__to_address(__end_), std::__to_address(__new_begin));
995-
__v.__begin_ = __new_begin;
996-
__end_ = __begin_; // All the objects have been destroyed by relocating them.
989+
using _RevIter = std::reverse_iterator<pointer>;
990+
__v.__begin_ = std::__uninitialized_allocator_move_if_noexcept(
991+
__alloc(), _RevIter(__end_), _RevIter(__begin_), _RevIter(__v.__begin_))
992+
.base();
997993
std::swap(this->__begin_, __v.__begin_);
998994
std::swap(this->__end_, __v.__end_);
999995
std::swap(this->__end_cap(), __v.__end_cap());
1000996
__v.__first_ = __v.__begin_;
1001997
__annotate_new(size());
1002998
}
1003999

1004-
// __swap_out_circular_buffer relocates the objects in [__begin_, __p) into the front of __v, the objects in
1005-
// [__p, __end_) into the back of __v and swaps the buffers of *this and __v. It is assumed that __v provides space for
1006-
// exactly (__p - __begin_) objects in the front and space for at least (__end_ - __p) objects in the back. This
1007-
// function has a strong exception guarantee if __begin_ == __p || __end_ == __p.
10081000
template <class _Tp, class _Allocator>
10091001
_LIBCPP_CONSTEXPR_SINCE_CXX20 typename vector<_Tp, _Allocator>::pointer
10101002
vector<_Tp, _Allocator>::__swap_out_circular_buffer(__split_buffer<value_type, allocator_type&>& __v, pointer __p) {
10111003
__annotate_delete();
1012-
pointer __ret = __v.__begin_;
1013-
1014-
// Relocate [__p, __end_) first to avoid having a hole in [__begin_, __end_)
1015-
// in case something in [__begin_, __p) throws.
1016-
std::__uninitialized_allocator_relocate(
1017-
__alloc(), std::__to_address(__p), std::__to_address(__end_), std::__to_address(__v.__end_));
1018-
__v.__end_ += (__end_ - __p);
1019-
__end_ = __p; // The objects in [__p, __end_) have been destroyed by relocating them.
1020-
auto __new_begin = __v.__begin_ - (__p - __begin_);
1021-
1022-
std::__uninitialized_allocator_relocate(
1023-
__alloc(), std::__to_address(__begin_), std::__to_address(__p), std::__to_address(__new_begin));
1024-
__v.__begin_ = __new_begin;
1025-
__end_ = __begin_; // All the objects have been destroyed by relocating them.
1026-
1004+
pointer __r = __v.__begin_;
1005+
using _RevIter = std::reverse_iterator<pointer>;
1006+
__v.__begin_ = std::__uninitialized_allocator_move_if_noexcept(
1007+
__alloc(), _RevIter(__p), _RevIter(__begin_), _RevIter(__v.__begin_))
1008+
.base();
1009+
__v.__end_ = std::__uninitialized_allocator_move_if_noexcept(__alloc(), __p, __end_, __v.__end_);
10271010
std::swap(this->__begin_, __v.__begin_);
10281011
std::swap(this->__end_, __v.__end_);
10291012
std::swap(this->__end_cap(), __v.__end_cap());
10301013
__v.__first_ = __v.__begin_;
10311014
__annotate_new(size());
1032-
return __ret;
1015+
return __r;
10331016
}
10341017

10351018
template <class _Tp, class _Allocator>

0 commit comments

Comments
 (0)