[libc++] Take advantage of trivial relocation in std::vector::erase

ldionne · ldionne · commit ac58253f9b14 · 2024-11-27T15:02:48.000-05:00
In vector::erase(iter) and vector::erase(iter, iter), we can take advantage of a type being trivially relocatable to open up a gap in the vector and then relocate the tail of the vector into that gap. The benefit is that relocating an object is often more efficient than move-assigning and then destroying the original object. For types that can be relocated trivially but that are complicated enough for the compiler not to optimize by itself (like std::string), this provides around a 2x performance speedup in vector::erase (see below). This optimization requires stopping the usage of Clang's __is_trivially_relocatable builtin, which doesn't currently honour assignment operators like is_trivially_copyable does and can lead us to perform incorrect optimizations. It is also worth noting that __uninitialized_allocator_relocate has to be modified so that we can relocate into an overlapping range. This has an unfortunate impact on its exception safety guarantees, which needs to be investigated further. Previous implementation -------------------------------------------------------------------------------------- Benchmark Time CPU Iterations -------------------------------------------------------------------------------------- BM_erase_iter_in_middle/vector_int/1024 24.9 ns 24.9 ns 28042962 BM_erase_iter_in_middle/vector_int/4096 107 ns 107 ns 6590592 BM_erase_iter_in_middle/vector_int/10240 271 ns 265 ns 2733478 BM_erase_iter_in_middle/vector_string/1024 349 ns 349 ns 2005886 BM_erase_iter_in_middle/vector_string/4096 1410 ns 1406 ns 498355 BM_erase_iter_in_middle/vector_string/10240 3449 ns 3449 ns 201989 BM_erase_iter_at_start/vector_int/1024 47.1 ns 47.1 ns 14836261 BM_erase_iter_at_start/vector_int/4096 204 ns 204 ns 3430414 BM_erase_iter_at_start/vector_int/10240 504 ns 504 ns 1391373 BM_erase_iter_at_start/vector_string/1024 684 ns 684 ns 1025160 BM_erase_iter_at_start/vector_string/4096 2855 ns 2806 ns 254080 BM_erase_iter_at_start/vector_string/10240 7060 ns 7060 ns 94134 New implementation -------------------------------------------------------------------------------------- Benchmark Time CPU Iterations -------------------------------------------------------------------------------------- BM_erase_iter_in_middle/vector_int/1024 26.0 ns 25.9 ns 27127367 BM_erase_iter_in_middle/vector_int/4096 105 ns 105 ns 6515204 BM_erase_iter_in_middle/vector_int/10240 259 ns 258 ns 2800795 BM_erase_iter_in_middle/vector_string/1024 148 ns 147 ns 4725706 BM_erase_iter_in_middle/vector_string/4096 608 ns 606 ns 1168205 BM_erase_iter_in_middle/vector_string/10240 1523 ns 1520 ns 459909 BM_erase_iter_at_start/vector_int/1024 47.1 ns 47.1 ns 14762513 BM_erase_iter_at_start/vector_int/4096 205 ns 205 ns 3403130 BM_erase_iter_at_start/vector_int/10240 507 ns 507 ns 1382716 BM_erase_iter_at_start/vector_string/1024 300 ns 300 ns 2327546 BM_erase_iter_at_start/vector_string/4096 1205 ns 1205 ns 580855 BM_erase_iter_at_start/vector_string/10240 4296 ns 4296 ns 162956
diff --git a/libcxx/docs/ReleaseNotes/20.rst b/libcxx/docs/ReleaseNotes/20.rst
@@ -52,6 +52,9 @@ Improvements and New Features
 - The ``lexicographical_compare`` and ``ranges::lexicographical_compare`` algorithms have been optimized for trivially
   equality comparable types, resulting in a performance improvement of up to 40x.
 
+- The ``std::vector::erase`` function has been optimized for types that can be relocated trivially (such as ``std::string``),
+  yielding speed ups witnessed to be around 2x for these types (but subject to the use case).
+
 - The ``_LIBCPP_ENABLE_CXX20_REMOVED_TEMPORARY_BUFFER`` macro has been added to make ``std::get_temporary_buffer`` and
   ``std::return_temporary_buffer`` available.
 
diff --git a/libcxx/include/__vector/vector.h b/libcxx/include/__vector/vector.h
@@ -34,11 +34,13 @@
 #include <__memory/allocator.h>
 #include <__memory/allocator_traits.h>
 #include <__memory/compressed_pair.h>
+#include <__memory/is_trivially_allocator_relocatable.h>
 #include <__memory/noexcept_move_assign_container.h>
 #include <__memory/pointer_traits.h>
 #include <__memory/swap_allocator.h>
 #include <__memory/temp_value.h>
 #include <__memory/uninitialized_algorithms.h>
+#include <__memory/uninitialized_relocate.h>
 #include <__ranges/access.h>
 #include <__ranges/concepts.h>
 #include <__ranges/container_compatible_range.h>
@@ -515,8 +517,36 @@ class _LIBCPP_TEMPLATE_VIS vector {
   }
 #endif
 
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator erase(const_iterator __position);
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator erase(const_iterator __first, const_iterator __last);
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator erase(const_iterator __position) {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
+        __position != end(), "vector::erase(iterator) called with a non-dereferenceable iterator");
+    return erase(__position, __position + 1);
+  }
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI iterator erase(const_iterator __cfirst, const_iterator __clast) {
+    _LIBCPP_ASSERT_VALID_INPUT_RANGE(__cfirst <= __clast, "vector::erase(first, last) called with invalid range");
+
+    iterator __first = begin() + std::distance(cbegin(), __cfirst);
+    iterator __last  = begin() + std::distance(cbegin(), __clast);
+    if (__first == __last)
+      return __last;
+
+    auto __n = std::distance(__first, __last);
+
+    // If the value_type is nothrow move constructible, we destroy the range being erased and we
+    // relocate the tail of the vector into the created gap. This is especially efficient if the
+    // elements are trivially relocatable. Otherwise, we use the standard technique with move-assignments.
+    if constexpr (is_nothrow_move_constructible<value_type>::value) {
+      std::__allocator_destroy(this->__alloc_, __first, __last);
+      std::__uninitialized_allocator_relocate(this->__alloc_, __last, end(), __first);
+    } else {
+      auto __new_end = std::move(__last, end(), __first);
+      std::__allocator_destroy(this->__alloc_, __new_end, end());
+    }
+
+    this->__end_ -= __n;
+    __annotate_shrink(size() + __n);
+    return __first;
+  }
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void clear() _NOEXCEPT {
     size_type __old_size = size();
@@ -1108,28 +1138,6 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 inline
 #endif
 }
 
-template <class _Tp, class _Allocator>
-_LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI typename vector<_Tp, _Allocator>::iterator
-vector<_Tp, _Allocator>::erase(const_iterator __position) {
-  _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(
-      __position != end(), "vector::erase(iterator) called with a non-dereferenceable iterator");
-  difference_type __ps = __position - cbegin();
-  pointer __p          = this->__begin_ + __ps;
-  this->__destruct_at_end(std::move(__p + 1, this->__end_, __p));
-  return __make_iter(__p);
-}
-
-template <class _Tp, class _Allocator>
-_LIBCPP_CONSTEXPR_SINCE_CXX20 typename vector<_Tp, _Allocator>::iterator
-vector<_Tp, _Allocator>::erase(const_iterator __first, const_iterator __last) {
-  _LIBCPP_ASSERT_VALID_INPUT_RANGE(__first <= __last, "vector::erase(first, last) called with invalid range");
-  pointer __p = this->__begin_ + (__first - begin());
-  if (__first != __last) {
-    this->__destruct_at_end(std::move(__p + (__last - __first), this->__end_, __p));
-  }
-  return __make_iter(__p);
-}
-
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 void
 vector<_Tp, _Allocator>::__move_range(pointer __from_s, pointer __from_e, pointer __to) {
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/common.h b/libcxx/test/std/containers/sequences/vector/vector.modifiers/common.h
@@ -40,28 +40,6 @@ struct Throws {
 bool Throws::sThrows = false;
 #endif
 
-struct Tracker {
-  int copy_assignments = 0;
-  int move_assignments = 0;
-};
-
-struct TrackedAssignment {
-  Tracker* tracker_;
-  TEST_CONSTEXPR_CXX14 explicit TrackedAssignment(Tracker* tracker) : tracker_(tracker) {}
-
-  TrackedAssignment(TrackedAssignment const&) = default;
-  TrackedAssignment(TrackedAssignment&&)      = default;
-
-  TEST_CONSTEXPR_CXX14 TrackedAssignment& operator=(TrackedAssignment const&) {
-    tracker_->copy_assignments++;
-    return *this;
-  }
-  TEST_CONSTEXPR_CXX14 TrackedAssignment& operator=(TrackedAssignment&&) {
-    tracker_->move_assignments++;
-    return *this;
-  }
-};
-
 struct NonTriviallyRelocatable {
   int value_;
   TEST_CONSTEXPR NonTriviallyRelocatable() : value_(0) {}
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter.pass.cpp
@@ -107,31 +107,5 @@ int main(int, char**) {
   }
 #endif
 
-  // Make sure we satisfy the complexity requirement in terms of the number of times the assignment
-  // operator is called.
-  //
-  // There is currently ambiguity as to whether this is truly mandated by the Standard, so we only
-  // test it for libc++.
-#ifdef _LIBCPP_VERSION
-  {
-    Tracker tracker;
-    std::vector<TrackedAssignment> v;
-
-    // Set up the vector with 5 elements.
-    for (int i = 0; i != 5; ++i) {
-      v.emplace_back(&tracker);
-    }
-    assert(tracker.copy_assignments == 0);
-    assert(tracker.move_assignments == 0);
-
-    // Erase element [1] from it. Elements [2] [3] [4] should be shifted, so we should
-    // see 3 move assignments (and nothing else).
-    v.erase(v.begin() + 1);
-    assert(v.size() == 4);
-    assert(tracker.copy_assignments == 0);
-    assert(tracker.move_assignments == 3);
-  }
-#endif
-
   return 0;
 }
diff --git a/libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter_iter.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.modifiers/erase_iter_iter.pass.cpp
@@ -196,31 +196,5 @@ int main(int, char**) {
     assert(it == v.begin() + 2);
   }
 
-  // Make sure we satisfy the complexity requirement in terms of the number of times the assignment
-  // operator is called.
-  //
-  // There is currently ambiguity as to whether this is truly mandated by the Standard, so we only
-  // test it for libc++.
-#ifdef _LIBCPP_VERSION
-  {
-    Tracker tracker;
-    std::vector<TrackedAssignment> v;
-
-    // Set up the vector with 5 elements.
-    for (int i = 0; i != 5; ++i) {
-      v.emplace_back(&tracker);
-    }
-    assert(tracker.copy_assignments == 0);
-    assert(tracker.move_assignments == 0);
-
-    // Erase elements [1] and [2] from it. Elements [3] [4] should be shifted, so we should
-    // see 2 move assignments (and nothing else).
-    v.erase(v.begin() + 1, v.begin() + 3);
-    assert(v.size() == 3);
-    assert(tracker.copy_assignments == 0);
-    assert(tracker.move_assignments == 2);
-  }
-#endif
-
   return 0;
 }
