[libcxx-commits] [libcxx] Optimize __insert_with_sentinel Function in std::vector (PR #113768)

[Peng Liu via libcxx-commits]

================
@@ -1360,27 +1360,27 @@ vector<_Tp, _Allocator>::__insert_with_sentinel(const_iterator __position, _Inpu
   for (; this->__end_ != this->__end_cap() && __first != __last; ++__first) {
     __construct_one_at_end(*__first);
   }
-  __split_buffer<value_type, allocator_type&> __v(__a);
-  if (__first != __last) {
-#if _LIBCPP_HAS_EXCEPTIONS
-    try {
-#endif // _LIBCPP_HAS_EXCEPTIONS
-      __v.__construct_at_end_with_sentinel(std::move(__first), std::move(__last));
-      difference_type __old_size = __old_last - this->__begin_;
-      difference_type __old_p    = __p - this->__begin_;
-      reserve(__recommend(size() + __v.size()));
-      __p        = this->__begin_ + __old_p;
-      __old_last = this->__begin_ + __old_size;
-#if _LIBCPP_HAS_EXCEPTIONS
-    } catch (...) {
-      erase(__make_iter(__old_last), end());
-      throw;
-    }
-#endif // _LIBCPP_HAS_EXCEPTIONS
+
+  if (__first == __last)
+    (void)std::rotate(__p, __old_last, this->__end_);
+  else {
+    __split_buffer<value_type, allocator_type&> __v(__a);
+    auto __guard =
+        std::__make_exception_guard(_AllocatorDestroyRangeReverse<allocator_type, pointer>(__a, __old_last, __end_));
----------------
winner245 wrote:

Good catch! I appreciate your perspective. Yes, it is generally required to pass the iterators by reference to `_AllocatorDestroyRangeReverse` if the protected memory range could grow or shrink while the guard is active. A good example is `__uninitialized_allocator_copy_impl`, where the guarded `target` memory range grows as the copy operations proceed, making it essential to pass the end iterator by reference to keep the guard in sync with growing protected range. 

However, my case is somewhat different because the guarded range is a `source` memory range `[__old_last, __end_)`, which remains fixed during the guard's protection.  This made me realize that my initial commit might have been misleading, as I shrink `__end_` to `__old_last` before calling `__guard.complete()`. This shrink operation could actually be done after the call to `__guard.complete()`, as its purpose is to ensure that subsequent relocation in `__swap_out_circular_buffer` stops at `__old_last`. To clarify this, I will add a comment and move the call `__guard.complete()` earlier (i.e., release the guard as soon as feasible). 

https://github.com/llvm/llvm-project/pull/113768