[libcxx-commits] [libcxx] [libc++] Refactor __split_buffer to eliminate code duplication (PR #114138)

Peng Liu via libcxx-commits libcxx-commits at lists.llvm.org
Fri Nov 1 06:49:06 PDT 2024


https://github.com/winner245 updated https://github.com/llvm/llvm-project/pull/114138

>From ad72ac7675e8fb147227973b1fb533218a9abdb1 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Tue, 29 Oct 2024 16:28:33 -0400
Subject: [PATCH 1/2] Refactor __split_buffer to eliminate code duplication

---
 libcxx/include/__split_buffer | 67 +++++++----------------------------
 1 file changed, 12 insertions(+), 55 deletions(-)

diff --git a/libcxx/include/__split_buffer b/libcxx/include/__split_buffer
index 6fc3d9255946ce..de3779b9810473 100644
--- a/libcxx/include/__split_buffer
+++ b/libcxx/include/__split_buffer
@@ -152,6 +152,8 @@ public:
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_front(value_type&& __x);
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_back(value_type&& __x);
 
+  template <class... _Args>
+  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void emplace_front(_Args&&... __args);
   template <class... _Args>
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void emplace_back(_Args&&... __args);
 
@@ -456,28 +458,17 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::shrink_to_fi
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_front(const_reference __x) {
-  if (__begin_ == __first_) {
-    if (__end_ < __end_cap()) {
-      difference_type __d = __end_cap() - __end_;
-      __d                 = (__d + 1) / 2;
-      __begin_            = std::move_backward(__begin_, __end_, __end_ + __d);
-      __end_ += __d;
-    } else {
-      size_type __c = std::max<size_type>(2 * static_cast<size_t>(__end_cap() - __first_), 1);
-      __split_buffer<value_type, __alloc_rr&> __t(__c, (__c + 3) / 4, __alloc());
-      __t.__construct_at_end(move_iterator<pointer>(__begin_), move_iterator<pointer>(__end_));
-      std::swap(__first_, __t.__first_);
-      std::swap(__begin_, __t.__begin_);
-      std::swap(__end_, __t.__end_);
-      std::swap(__end_cap(), __t.__end_cap());
-    }
-  }
-  __alloc_traits::construct(__alloc(), std::__to_address(__begin_ - 1), __x);
-  --__begin_;
+  emplace_front(__x);
 }
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_front(value_type&& __x) {
+  emplace_front(std::move(__x));
+}
+
+template <class _Tp, class _Allocator>
+template <class... _Args>
+_LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::emplace_front(_Args&&... __args) {
   if (__begin_ == __first_) {
     if (__end_ < __end_cap()) {
       difference_type __d = __end_cap() - __end_;
@@ -494,53 +485,19 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_front(v
       std::swap(__end_cap(), __t.__end_cap());
     }
   }
-  __alloc_traits::construct(__alloc(), std::__to_address(__begin_ - 1), std::move(__x));
+  __alloc_traits::construct(__alloc(), std::__to_address(__begin_ - 1), std::forward<_Args>(__args)...);
   --__begin_;
 }
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI void
 __split_buffer<_Tp, _Allocator>::push_back(const_reference __x) {
-  if (__end_ == __end_cap()) {
-    if (__begin_ > __first_) {
-      difference_type __d = __begin_ - __first_;
-      __d                 = (__d + 1) / 2;
-      __end_              = std::move(__begin_, __end_, __begin_ - __d);
-      __begin_ -= __d;
-    } else {
-      size_type __c = std::max<size_type>(2 * static_cast<size_t>(__end_cap() - __first_), 1);
-      __split_buffer<value_type, __alloc_rr&> __t(__c, __c / 4, __alloc());
-      __t.__construct_at_end(move_iterator<pointer>(__begin_), move_iterator<pointer>(__end_));
-      std::swap(__first_, __t.__first_);
-      std::swap(__begin_, __t.__begin_);
-      std::swap(__end_, __t.__end_);
-      std::swap(__end_cap(), __t.__end_cap());
-    }
-  }
-  __alloc_traits::construct(__alloc(), std::__to_address(__end_), __x);
-  ++__end_;
+  emplace_back(__x);
 }
 
 template <class _Tp, class _Allocator>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_back(value_type&& __x) {
-  if (__end_ == __end_cap()) {
-    if (__begin_ > __first_) {
-      difference_type __d = __begin_ - __first_;
-      __d                 = (__d + 1) / 2;
-      __end_              = std::move(__begin_, __end_, __begin_ - __d);
-      __begin_ -= __d;
-    } else {
-      size_type __c = std::max<size_type>(2 * static_cast<size_t>(__end_cap() - __first_), 1);
-      __split_buffer<value_type, __alloc_rr&> __t(__c, __c / 4, __alloc());
-      __t.__construct_at_end(move_iterator<pointer>(__begin_), move_iterator<pointer>(__end_));
-      std::swap(__first_, __t.__first_);
-      std::swap(__begin_, __t.__begin_);
-      std::swap(__end_, __t.__end_);
-      std::swap(__end_cap(), __t.__end_cap());
-    }
-  }
-  __alloc_traits::construct(__alloc(), std::__to_address(__end_), std::move(__x));
-  ++__end_;
+  emplace_back(std::move(__x));
 }
 
 template <class _Tp, class _Allocator>

>From 13eec16e9c96198dff9ddffd71e483addf272aad Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Thu, 31 Oct 2024 09:19:33 -0400
Subject: [PATCH 2/2] Replace all usages of __split_buffer::push_{back,front}
 by emplace_{back,front}

---
 libcxx/include/__split_buffer    | 25 ---------------
 libcxx/include/__vector/vector.h |  4 +--
 libcxx/include/deque             | 52 ++++++++++++++++----------------
 3 files changed, 28 insertions(+), 53 deletions(-)

diff --git a/libcxx/include/__split_buffer b/libcxx/include/__split_buffer
index de3779b9810473..102df63e814ae4 100644
--- a/libcxx/include/__split_buffer
+++ b/libcxx/include/__split_buffer
@@ -147,10 +147,6 @@ public:
 
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void reserve(size_type __n);
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void shrink_to_fit() _NOEXCEPT;
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_front(const_reference __x);
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_back(const_reference __x);
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_front(value_type&& __x);
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void push_back(value_type&& __x);
 
   template <class... _Args>
   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void emplace_front(_Args&&... __args);
@@ -456,16 +452,6 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::shrink_to_fi
   }
 }
 
-template <class _Tp, class _Allocator>
-_LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_front(const_reference __x) {
-  emplace_front(__x);
-}
-
-template <class _Tp, class _Allocator>
-_LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_front(value_type&& __x) {
-  emplace_front(std::move(__x));
-}
-
 template <class _Tp, class _Allocator>
 template <class... _Args>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::emplace_front(_Args&&... __args) {
@@ -489,17 +475,6 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::emplace_fron
   --__begin_;
 }
 
-template <class _Tp, class _Allocator>
-_LIBCPP_CONSTEXPR_SINCE_CXX20 inline _LIBCPP_HIDE_FROM_ABI void
-__split_buffer<_Tp, _Allocator>::push_back(const_reference __x) {
-  emplace_back(__x);
-}
-
-template <class _Tp, class _Allocator>
-_LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::push_back(value_type&& __x) {
-  emplace_back(std::move(__x));
-}
-
 template <class _Tp, class _Allocator>
 template <class... _Args>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 void __split_buffer<_Tp, _Allocator>::emplace_back(_Args&&... __args) {
diff --git a/libcxx/include/__vector/vector.h b/libcxx/include/__vector/vector.h
index 844e5d6a210568..7d95e5deef5f3b 100644
--- a/libcxx/include/__vector/vector.h
+++ b/libcxx/include/__vector/vector.h
@@ -1263,7 +1263,7 @@ vector<_Tp, _Allocator>::insert(const_iterator __position, const_reference __x)
   } else {
     allocator_type& __a = this->__alloc();
     __split_buffer<value_type, allocator_type&> __v(__recommend(size() + 1), __p - this->__begin_, __a);
-    __v.push_back(__x);
+    __v.emplace_back(__x);
     __p = __swap_out_circular_buffer(__v, __p);
   }
   return __make_iter(__p);
@@ -1283,7 +1283,7 @@ vector<_Tp, _Allocator>::insert(const_iterator __position, value_type&& __x) {
   } else {
     allocator_type& __a = this->__alloc();
     __split_buffer<value_type, allocator_type&> __v(__recommend(size() + 1), __p - this->__begin_, __a);
-    __v.push_back(std::move(__x));
+    __v.emplace_back(std::move(__x));
     __p = __swap_out_circular_buffer(__v, __p);
   }
   return __make_iter(__p);
diff --git a/libcxx/include/deque b/libcxx/include/deque
index 11219d1a99244e..1a83465b39ef81 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -2041,20 +2041,20 @@ void deque<_Tp, _Allocator>::__add_front_capacity() {
     __start_ += __block_size;
     pointer __pt = __map_.back();
     __map_.pop_back();
-    __map_.push_front(__pt);
+    __map_.emplace_front(__pt);
   }
   // Else if __map_.size() < __map_.capacity() then we need to allocate 1 buffer
   else if (__map_.size() < __map_.capacity()) { // we can put the new buffer into the map, but don't shift things around
     // until all buffers are allocated.  If we throw, we don't need to fix
     // anything up (any added buffers are undetectible)
     if (__map_.__front_spare() > 0)
-      __map_.push_front(__alloc_traits::allocate(__a, __block_size));
+      __map_.emplace_front(__alloc_traits::allocate(__a, __block_size));
     else {
-      __map_.push_back(__alloc_traits::allocate(__a, __block_size));
+      __map_.emplace_back(__alloc_traits::allocate(__a, __block_size));
       // Done allocating, reorder capacity
       pointer __pt = __map_.back();
       __map_.pop_back();
-      __map_.push_front(__pt);
+      __map_.emplace_front(__pt);
     }
     __start_ = __map_.size() == 1 ? __block_size / 2 : __start_ + __block_size;
   }
@@ -2065,11 +2065,11 @@ void deque<_Tp, _Allocator>::__add_front_capacity() {
 
     typedef __allocator_destructor<_Allocator> _Dp;
     unique_ptr<pointer, _Dp> __hold(__alloc_traits::allocate(__a, __block_size), _Dp(__a, __block_size));
-    __buf.push_back(__hold.get());
+    __buf.emplace_back(__hold.get());
     __hold.release();
 
     for (__map_pointer __i = __map_.begin(); __i != __map_.end(); ++__i)
-      __buf.push_back(*__i);
+      __buf.emplace_back(*__i);
     std::swap(__map_.__first_, __buf.__first_);
     std::swap(__map_.__begin_, __buf.__begin_);
     std::swap(__map_.__end_, __buf.__end_);
@@ -2095,7 +2095,7 @@ void deque<_Tp, _Allocator>::__add_front_capacity(size_type __n) {
     for (; __back_capacity > 0; --__back_capacity) {
       pointer __pt = __map_.back();
       __map_.pop_back();
-      __map_.push_front(__pt);
+      __map_.emplace_front(__pt);
     }
   }
   // Else if __nb <= __map_.capacity() - __map_.size() then we need to allocate __nb buffers
@@ -2106,17 +2106,17 @@ void deque<_Tp, _Allocator>::__add_front_capacity(size_type __n) {
     for (; __nb > 0; --__nb, __start_ += __block_size - (__map_.size() == 1)) {
       if (__map_.__front_spare() == 0)
         break;
-      __map_.push_front(__alloc_traits::allocate(__a, __block_size));
+      __map_.emplace_front(__alloc_traits::allocate(__a, __block_size));
       __annotate_whole_block(0, __asan_poison);
     }
     for (; __nb > 0; --__nb, ++__back_capacity)
-      __map_.push_back(__alloc_traits::allocate(__a, __block_size));
+      __map_.emplace_back(__alloc_traits::allocate(__a, __block_size));
     // Done allocating, reorder capacity
     __start_ += __back_capacity * __block_size;
     for (; __back_capacity > 0; --__back_capacity) {
       pointer __pt = __map_.back();
       __map_.pop_back();
-      __map_.push_front(__pt);
+      __map_.emplace_front(__pt);
       __annotate_whole_block(0, __asan_poison);
     }
   }
@@ -2129,7 +2129,7 @@ void deque<_Tp, _Allocator>::__add_front_capacity(size_type __n) {
     try {
 #endif // _LIBCPP_HAS_EXCEPTIONS
       for (; __nb > 0; --__nb) {
-        __buf.push_back(__alloc_traits::allocate(__a, __block_size));
+        __buf.emplace_back(__alloc_traits::allocate(__a, __block_size));
         // ASan: this is empty container, we have to poison whole block
         __annotate_poison_block(std::__to_address(__buf.back()), std::__to_address(__buf.back() + __block_size));
       }
@@ -2142,11 +2142,11 @@ void deque<_Tp, _Allocator>::__add_front_capacity(size_type __n) {
     }
 #endif // _LIBCPP_HAS_EXCEPTIONS
     for (; __back_capacity > 0; --__back_capacity) {
-      __buf.push_back(__map_.back());
+      __buf.emplace_back(__map_.back());
       __map_.pop_back();
     }
     for (__map_pointer __i = __map_.begin(); __i != __map_.end(); ++__i)
-      __buf.push_back(*__i);
+      __buf.emplace_back(*__i);
     std::swap(__map_.__first_, __buf.__first_);
     std::swap(__map_.__begin_, __buf.__begin_);
     std::swap(__map_.__end_, __buf.__end_);
@@ -2164,20 +2164,20 @@ void deque<_Tp, _Allocator>::__add_back_capacity() {
     __start_ -= __block_size;
     pointer __pt = __map_.front();
     __map_.pop_front();
-    __map_.push_back(__pt);
+    __map_.emplace_back(__pt);
   }
   // Else if __nb <= __map_.capacity() - __map_.size() then we need to allocate __nb buffers
   else if (__map_.size() < __map_.capacity()) { // we can put the new buffer into the map, but don't shift things around
     // until it is allocated.  If we throw, we don't need to fix
     // anything up (any added buffers are undetectible)
     if (__map_.__back_spare() != 0)
-      __map_.push_back(__alloc_traits::allocate(__a, __block_size));
+      __map_.emplace_back(__alloc_traits::allocate(__a, __block_size));
     else {
-      __map_.push_front(__alloc_traits::allocate(__a, __block_size));
+      __map_.emplace_front(__alloc_traits::allocate(__a, __block_size));
       // Done allocating, reorder capacity
       pointer __pt = __map_.front();
       __map_.pop_front();
-      __map_.push_back(__pt);
+      __map_.emplace_back(__pt);
     }
     __annotate_whole_block(__map_.size() - 1, __asan_poison);
   }
@@ -2188,11 +2188,11 @@ void deque<_Tp, _Allocator>::__add_back_capacity() {
 
     typedef __allocator_destructor<_Allocator> _Dp;
     unique_ptr<pointer, _Dp> __hold(__alloc_traits::allocate(__a, __block_size), _Dp(__a, __block_size));
-    __buf.push_back(__hold.get());
+    __buf.emplace_back(__hold.get());
     __hold.release();
 
     for (__map_pointer __i = __map_.end(); __i != __map_.begin();)
-      __buf.push_front(*--__i);
+      __buf.emplace_front(*--__i);
     std::swap(__map_.__first_, __buf.__first_);
     std::swap(__map_.__begin_, __buf.__begin_);
     std::swap(__map_.__end_, __buf.__end_);
@@ -2217,7 +2217,7 @@ void deque<_Tp, _Allocator>::__add_back_capacity(size_type __n) {
     for (; __front_capacity > 0; --__front_capacity) {
       pointer __pt = __map_.front();
       __map_.pop_front();
-      __map_.push_back(__pt);
+      __map_.emplace_back(__pt);
     }
   }
   // Else if __nb <= __map_.capacity() - __map_.size() then we need to allocate __nb buffers
@@ -2228,11 +2228,11 @@ void deque<_Tp, _Allocator>::__add_back_capacity(size_type __n) {
     for (; __nb > 0; --__nb) {
       if (__map_.__back_spare() == 0)
         break;
-      __map_.push_back(__alloc_traits::allocate(__a, __block_size));
+      __map_.emplace_back(__alloc_traits::allocate(__a, __block_size));
       __annotate_whole_block(__map_.size() - 1, __asan_poison);
     }
     for (; __nb > 0; --__nb, ++__front_capacity, __start_ += __block_size - (__map_.size() == 1)) {
-      __map_.push_front(__alloc_traits::allocate(__a, __block_size));
+      __map_.emplace_front(__alloc_traits::allocate(__a, __block_size));
       __annotate_whole_block(0, __asan_poison);
     }
     // Done allocating, reorder capacity
@@ -2240,7 +2240,7 @@ void deque<_Tp, _Allocator>::__add_back_capacity(size_type __n) {
     for (; __front_capacity > 0; --__front_capacity) {
       pointer __pt = __map_.front();
       __map_.pop_front();
-      __map_.push_back(__pt);
+      __map_.emplace_back(__pt);
     }
   }
   // Else need to allocate __nb buffers, *and* we need to reallocate __map_.
@@ -2254,7 +2254,7 @@ void deque<_Tp, _Allocator>::__add_back_capacity(size_type __n) {
     try {
 #endif // _LIBCPP_HAS_EXCEPTIONS
       for (; __nb > 0; --__nb) {
-        __buf.push_back(__alloc_traits::allocate(__a, __block_size));
+        __buf.emplace_back(__alloc_traits::allocate(__a, __block_size));
         // ASan: this is an empty container, we have to poison the whole block
         __annotate_poison_block(std::__to_address(__buf.back()), std::__to_address(__buf.back() + __block_size));
       }
@@ -2267,11 +2267,11 @@ void deque<_Tp, _Allocator>::__add_back_capacity(size_type __n) {
     }
 #endif // _LIBCPP_HAS_EXCEPTIONS
     for (; __front_capacity > 0; --__front_capacity) {
-      __buf.push_back(__map_.front());
+      __buf.emplace_back(__map_.front());
       __map_.pop_front();
     }
     for (__map_pointer __i = __map_.end(); __i != __map_.begin();)
-      __buf.push_front(*--__i);
+      __buf.emplace_front(*--__i);
     std::swap(__map_.__first_, __buf.__first_);
     std::swap(__map_.__begin_, __buf.__begin_);
     std::swap(__map_.__end_, __buf.__end_);



More information about the libcxx-commits mailing list