Commit 92c8575d by Arthur O'Dwyer Committed by Copybara-Service

PR #1618: inlined_vector: Use trivial relocation for `SwapInlinedElements`

Imported from GitHub PR https://github.com/abseil/abseil-cpp/pull/1618

I noticed while working on #1615 that `inlined_vector` could use the trivial relocatability trait here, too.
Here the memcpy codepath already exists; we just have to opt in to using it.
Merge 567a1dd9b6b3352f649e900b24834b59e39cfa14 into a7012a5b

Merging this change closes #1618

COPYBARA_INTEGRATE_REVIEW=https://github.com/abseil/abseil-cpp/pull/1618 from Quuxplusone:trivial-swap 567a1dd9b6b3352f649e900b24834b59e39cfa14
PiperOrigin-RevId: 609019296
Change-Id: I4055ab790245752179e405b490fcd479e7389726
parent b0f85e23
...@@ -304,6 +304,35 @@ TEST(UniquePtr, MoveAssign) { ...@@ -304,6 +304,35 @@ TEST(UniquePtr, MoveAssign) {
} }
} }
// Swapping containers of unique pointers should work fine, with no
// leaks, despite the fact that unique pointers are trivially relocatable but
// not trivially destructible.
// TODO(absl-team): Using unique_ptr here is technically correct, but
// a trivially relocatable struct would be less semantically confusing.
TEST(UniquePtr, Swap) {
for (size_t size1 = 0; size1 < 5; ++size1) {
for (size_t size2 = 0; size2 < 5; ++size2) {
absl::InlinedVector<std::unique_ptr<size_t>, 2> a;
absl::InlinedVector<std::unique_ptr<size_t>, 2> b;
for (size_t i = 0; i < size1; ++i) {
a.push_back(std::make_unique<size_t>(i + 10));
}
for (size_t i = 0; i < size2; ++i) {
b.push_back(std::make_unique<size_t>(i + 20));
}
a.swap(b);
ASSERT_THAT(a, SizeIs(size2));
ASSERT_THAT(b, SizeIs(size1));
for (size_t i = 0; i < a.size(); ++i) {
ASSERT_THAT(a[i], Pointee(i + 20));
}
for (size_t i = 0; i < b.size(); ++i) {
ASSERT_THAT(b[i], Pointee(i + 10));
}
}
}
}
// At the end of this test loop, the elements between [erase_begin, erase_end) // At the end of this test loop, the elements between [erase_begin, erase_end)
// should have reference counts == 0, and all others elements should have // should have reference counts == 0, and all others elements should have
// reference counts == 1. // reference counts == 1.
...@@ -783,7 +812,9 @@ TEST(OverheadTest, Storage) { ...@@ -783,7 +812,9 @@ TEST(OverheadTest, Storage) {
// The union should be absorbing some of the allocation bookkeeping overhead // The union should be absorbing some of the allocation bookkeeping overhead
// in the larger vectors, leaving only the size_ field as overhead. // in the larger vectors, leaving only the size_ field as overhead.
struct T { void* val; }; struct T {
void* val;
};
size_t expected_overhead = sizeof(T); size_t expected_overhead = sizeof(T);
EXPECT_EQ((2 * expected_overhead), EXPECT_EQ((2 * expected_overhead),
......
...@@ -322,14 +322,13 @@ class Storage { ...@@ -322,14 +322,13 @@ class Storage {
// The policy to be used specifically when swapping inlined elements. // The policy to be used specifically when swapping inlined elements.
using SwapInlinedElementsPolicy = absl::conditional_t< using SwapInlinedElementsPolicy = absl::conditional_t<
// Fast path: if the value type can be trivially move constructed/assigned // Fast path: if the value type can be trivially relocated, and we
// and destroyed, and we know the allocator doesn't do anything fancy, // know the allocator doesn't do anything fancy, then it's safe for us
// then it's safe for us to simply swap the bytes in the inline storage. // to simply swap the bytes in the inline storage. It's as if we had
// It's as if we had move-constructed a temporary vector, move-assigned // relocated the first vector's elements into temporary storage,
// one to the other, then move-assigned the first from the temporary. // relocated the second's elements into the (now-empty) first's,
absl::conjunction<absl::is_trivially_move_constructible<ValueType<A>>, // and then relocated from temporary storage into the second.
absl::is_trivially_move_assignable<ValueType<A>>, absl::conjunction<absl::is_trivially_relocatable<ValueType<A>>,
absl::is_trivially_destructible<ValueType<A>>,
std::is_same<A, std::allocator<ValueType<A>>>>::value, std::is_same<A, std::allocator<ValueType<A>>>>::value,
MemcpyPolicy, MemcpyPolicy,
absl::conditional_t<IsSwapOk<A>::value, ElementwiseSwapPolicy, absl::conditional_t<IsSwapOk<A>::value, ElementwiseSwapPolicy,
...@@ -624,8 +623,8 @@ void Storage<T, N, A>::InitFrom(const Storage& other) { ...@@ -624,8 +623,8 @@ void Storage<T, N, A>::InitFrom(const Storage& other) {
template <typename T, size_t N, typename A> template <typename T, size_t N, typename A>
template <typename ValueAdapter> template <typename ValueAdapter>
auto Storage<T, N, A>::Initialize(ValueAdapter values, SizeType<A> new_size) auto Storage<T, N, A>::Initialize(ValueAdapter values,
-> void { SizeType<A> new_size) -> void {
// Only callable from constructors! // Only callable from constructors!
ABSL_HARDENING_ASSERT(!GetIsAllocated()); ABSL_HARDENING_ASSERT(!GetIsAllocated());
ABSL_HARDENING_ASSERT(GetSize() == 0); ABSL_HARDENING_ASSERT(GetSize() == 0);
...@@ -656,8 +655,8 @@ auto Storage<T, N, A>::Initialize(ValueAdapter values, SizeType<A> new_size) ...@@ -656,8 +655,8 @@ auto Storage<T, N, A>::Initialize(ValueAdapter values, SizeType<A> new_size)
template <typename T, size_t N, typename A> template <typename T, size_t N, typename A>
template <typename ValueAdapter> template <typename ValueAdapter>
auto Storage<T, N, A>::Assign(ValueAdapter values, SizeType<A> new_size) auto Storage<T, N, A>::Assign(ValueAdapter values,
-> void { SizeType<A> new_size) -> void {
StorageView<A> storage_view = MakeStorageView(); StorageView<A> storage_view = MakeStorageView();
AllocationTransaction<A> allocation_tx(GetAllocator()); AllocationTransaction<A> allocation_tx(GetAllocator());
...@@ -699,8 +698,8 @@ auto Storage<T, N, A>::Assign(ValueAdapter values, SizeType<A> new_size) ...@@ -699,8 +698,8 @@ auto Storage<T, N, A>::Assign(ValueAdapter values, SizeType<A> new_size)
template <typename T, size_t N, typename A> template <typename T, size_t N, typename A>
template <typename ValueAdapter> template <typename ValueAdapter>
auto Storage<T, N, A>::Resize(ValueAdapter values, SizeType<A> new_size) auto Storage<T, N, A>::Resize(ValueAdapter values,
-> void { SizeType<A> new_size) -> void {
StorageView<A> storage_view = MakeStorageView(); StorageView<A> storage_view = MakeStorageView();
Pointer<A> const base = storage_view.data; Pointer<A> const base = storage_view.data;
const SizeType<A> size = storage_view.size; const SizeType<A> size = storage_view.size;
...@@ -885,8 +884,8 @@ auto Storage<T, N, A>::EmplaceBackSlow(Args&&... args) -> Reference<A> { ...@@ -885,8 +884,8 @@ auto Storage<T, N, A>::EmplaceBackSlow(Args&&... args) -> Reference<A> {
} }
template <typename T, size_t N, typename A> template <typename T, size_t N, typename A>
auto Storage<T, N, A>::Erase(ConstIterator<A> from, ConstIterator<A> to) auto Storage<T, N, A>::Erase(ConstIterator<A> from,
-> Iterator<A> { ConstIterator<A> to) -> Iterator<A> {
StorageView<A> storage_view = MakeStorageView(); StorageView<A> storage_view = MakeStorageView();
auto erase_size = static_cast<SizeType<A>>(std::distance(from, to)); auto erase_size = static_cast<SizeType<A>>(std::distance(from, to));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment