[libcxx-commits] [libcxx] [libc++] Optimize heap operations (PR #159917)

via libcxx-commits libcxx-commits at lists.llvm.org
Wed Oct 1 07:02:19 PDT 2025


https://github.com/nukyan updated https://github.com/llvm/llvm-project/pull/159917

>From d7714681a78a9604f2d3c16da022e85c347174e9 Mon Sep 17 00:00:00 2001
From: Yang Kun <ikspress at outlook.com>
Date: Sun, 14 Sep 2025 19:52:05 +0800
Subject: [PATCH] [libc++] Optimize heap operations

---
 libcxx/include/__algorithm/make_heap.h        |  6 +--
 .../include/__algorithm/partial_sort_copy.h   |  5 +-
 libcxx/include/__algorithm/pop_heap.h         | 36 +++++++-------
 libcxx/include/__algorithm/push_heap.h        | 48 +++++++++----------
 libcxx/include/__algorithm/ranges_pop_heap.h  |  4 +-
 libcxx/include/__algorithm/sift_down.h        | 29 +++++------
 libcxx/include/__algorithm/sort_heap.h        |  4 +-
 7 files changed, 66 insertions(+), 66 deletions(-)

diff --git a/libcxx/include/__algorithm/make_heap.h b/libcxx/include/__algorithm/make_heap.h
index 8aff8ce588568..2bd26eb00aa02 100644
--- a/libcxx/include/__algorithm/make_heap.h
+++ b/libcxx/include/__algorithm/make_heap.h
@@ -45,11 +45,11 @@ __make_heap(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compar
   if (__n > 1) {
     // start from the first parent, there is no need to consider children
 
-    for (__diff_t __start = (__sift_down_n - 2) / 2; __start >= 0; --__start) {
-      std::__sift_down<_AlgPolicy, __assume_both_children>(__first, __comp_ref, __sift_down_n, __start);
+    for (__diff_t __start = __sift_down_n / 2; __start != 0;) {
+      std::__sift_down<_AlgPolicy, __assume_both_children>(__first, __comp_ref, __sift_down_n, --__start);
     }
     if _LIBCPP_CONSTEXPR (__assume_both_children)
-      std::__sift_up<_AlgPolicy>(__first, __last, __comp, __n);
+      std::__sift_up<_AlgPolicy>(__first, --__last, __comp);
   }
 }
 
diff --git a/libcxx/include/__algorithm/partial_sort_copy.h b/libcxx/include/__algorithm/partial_sort_copy.h
index 2230dfc9cc4ad..0b8a0fb7e2830 100644
--- a/libcxx/include/__algorithm/partial_sort_copy.h
+++ b/libcxx/include/__algorithm/partial_sort_copy.h
@@ -52,16 +52,17 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair<_InputIterator, _Random
   _RandomAccessIterator __r = __result_first;
   auto&& __projected_comp   = std::__make_projected(__comp, __proj2);
 
-  if (__r != __result_last) {
+  if (__result_first != __result_last) {
     for (; __first != __last && __r != __result_last; ++__first, (void)++__r)
       *__r = *__first;
     std::__make_heap<_AlgPolicy>(__result_first, __r, __projected_comp);
     typename iterator_traits<_RandomAccessIterator>::difference_type __len = __r - __result_first;
-    for (; __first != __last; ++__first)
+    for (; __first != __last; ++__first) {
       if (std::__invoke(__comp, std::__invoke(__proj1, *__first), std::__invoke(__proj2, *__result_first))) {
         *__result_first = *__first;
         std::__sift_down<_AlgPolicy, false>(__result_first, __projected_comp, __len, 0);
       }
+    }
     std::__sort_heap<_AlgPolicy>(__result_first, __r, __projected_comp);
   }
 
diff --git a/libcxx/include/__algorithm/pop_heap.h b/libcxx/include/__algorithm/pop_heap.h
index 6d23830097ff9..57f079ace0d6e 100644
--- a/libcxx/include/__algorithm/pop_heap.h
+++ b/libcxx/include/__algorithm/pop_heap.h
@@ -33,28 +33,20 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 template <class _AlgPolicy, class _Compare, class _RandomAccessIterator>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
 __pop_heap(_RandomAccessIterator __first,
-           _RandomAccessIterator __last,
+           _RandomAccessIterator __bottom,
            _Compare& __comp,
            typename iterator_traits<_RandomAccessIterator>::difference_type __len) {
-  // Calling `pop_heap` on an empty range is undefined behavior, but in practice it will be a no-op.
-  _LIBCPP_ASSERT_PEDANTIC(__len > 0, "The heap given to pop_heap must be non-empty");
+  using value_type = typename iterator_traits<_RandomAccessIterator>::value_type;
 
-  __comp_ref_type<_Compare> __comp_ref = __comp;
+  value_type __top             = _IterOps<_AlgPolicy>::__iter_move(__first); // create a hole at __first
+  _RandomAccessIterator __hole = std::__floyd_sift_down<_AlgPolicy>(__first, __comp, __len);
 
-  using value_type = typename iterator_traits<_RandomAccessIterator>::value_type;
-  if (__len > 1) {
-    value_type __top             = _IterOps<_AlgPolicy>::__iter_move(__first); // create a hole at __first
-    _RandomAccessIterator __hole = std::__floyd_sift_down<_AlgPolicy>(__first, __comp_ref, __len);
-    --__last;
-
-    if (__hole == __last) {
-      *__hole = std::move(__top);
-    } else {
-      *__hole = _IterOps<_AlgPolicy>::__iter_move(__last);
-      ++__hole;
-      *__last = std::move(__top);
-      std::__sift_up<_AlgPolicy>(__first, __hole, __comp_ref, __hole - __first);
-    }
+  if (__hole == __bottom) {
+    *__hole = std::move(__top);
+  } else {
+    *__hole   = _IterOps<_AlgPolicy>::__iter_move(__bottom);
+    *__bottom = std::move(__top);
+    std::__sift_up<_AlgPolicy>(__first, __hole, __comp);
   }
 }
 
@@ -64,8 +56,14 @@ pop_heap(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare _
   static_assert(std::is_copy_constructible<_RandomAccessIterator>::value, "Iterators must be copy constructible.");
   static_assert(std::is_copy_assignable<_RandomAccessIterator>::value, "Iterators must be copy assignable.");
 
+  // Calling `pop_heap` on an empty range is undefined behavior, but in practice it will be a no-op.
+  _LIBCPP_ASSERT_PEDANTIC(__len > 0, "The heap given to pop_heap must be non-empty");
+
+  __comp_ref_type<_Compare> __comp_ref = __comp;
+
   typename iterator_traits<_RandomAccessIterator>::difference_type __len = __last - __first;
-  std::__pop_heap<_ClassicAlgPolicy>(std::move(__first), std::move(__last), __comp, __len);
+  if (__len > 1)
+    std::__pop_heap<_ClassicAlgPolicy>(std::move(__first), std::move(--__last), __comp_ref, __len);
 }
 
 template <class _RandomAccessIterator>
diff --git a/libcxx/include/__algorithm/push_heap.h b/libcxx/include/__algorithm/push_heap.h
index ec0b445f2b70f..0de948bcc0226 100644
--- a/libcxx/include/__algorithm/push_heap.h
+++ b/libcxx/include/__algorithm/push_heap.h
@@ -29,37 +29,35 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _AlgPolicy, class _Compare, class _RandomAccessIterator>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
-__sift_up(_RandomAccessIterator __first,
-          _RandomAccessIterator __last,
-          _Compare&& __comp,
-          typename iterator_traits<_RandomAccessIterator>::difference_type __len) {
-  using value_type = typename iterator_traits<_RandomAccessIterator>::value_type;
-
-  if (__len > 1) {
-    __len                       = (__len - 2) / 2;
-    _RandomAccessIterator __ptr = __first + __len;
-
-    if (__comp(*__ptr, *--__last)) {
-      value_type __t(_IterOps<_AlgPolicy>::__iter_move(__last));
-      do {
-        *__last = _IterOps<_AlgPolicy>::__iter_move(__ptr);
-        __last  = __ptr;
-        if (__len == 0)
-          break;
-        __len = (__len - 1) / 2;
-        __ptr = __first + __len;
-      } while (__comp(*__ptr, __t));
-
-      *__last = std::move(__t);
-    }
+__sift_up(_RandomAccessIterator __first, _RandomAccessIterator __bottom, _Compare&& __comp) {
+  using difference_type = typename iterator_traits<_RandomAccessIterator>::difference_type;
+  using value_type      = typename iterator_traits<_RandomAccessIterator>::value_type;
+
+  difference_type __parent = __bottom - __first;
+  _LIBCPP_ASSERT_INTERNAL(__parent > 0, "shouldn't be called unless __bottom - __first > 0");
+  __parent                         = (__parent - 1) / 2;
+  _RandomAccessIterator __parent_i = __first + __parent;
+
+  if (__comp(*__parent_i, *__bottom)) {
+    value_type __t(_IterOps<_AlgPolicy>::__iter_move(__bottom));
+    do {
+      *__bottom = _IterOps<_AlgPolicy>::__iter_move(__parent_i);
+      __bottom  = __parent_i;
+      if (__parent == 0)
+        break;
+      __parent   = (__parent - 1) / 2;
+      __parent_i = __first + __parent;
+    } while (__comp(*__parent_i, __t));
+
+    *__bottom = std::move(__t);
   }
 }
 
 template <class _AlgPolicy, class _RandomAccessIterator, class _Compare>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
 __push_heap(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare& __comp) {
-  typename iterator_traits<_RandomAccessIterator>::difference_type __len = __last - __first;
-  std::__sift_up<_AlgPolicy, __comp_ref_type<_Compare> >(std::move(__first), std::move(__last), __comp, __len);
+  if (__first != __last)
+    std::__sift_up<_AlgPolicy, __comp_ref_type<_Compare> >(std::move(__first), std::move(--__last), __comp);
 }
 
 template <class _RandomAccessIterator, class _Compare>
diff --git a/libcxx/include/__algorithm/ranges_pop_heap.h b/libcxx/include/__algorithm/ranges_pop_heap.h
index eccf54c094e3d..b6e994464a1f5 100644
--- a/libcxx/include/__algorithm/ranges_pop_heap.h
+++ b/libcxx/include/__algorithm/ranges_pop_heap.h
@@ -48,7 +48,9 @@ struct __pop_heap {
     auto __len       = __last_iter - __first;
 
     auto&& __projected_comp = std::__make_projected(__comp, __proj);
-    std::__pop_heap<_RangeAlgPolicy>(std::move(__first), __last_iter, __projected_comp, __len);
+
+    if (__len > 1)
+      std::__pop_heap<_RangeAlgPolicy>(std::move(__first), ranges::prev(__last_iter), __projected_comp, __len);
 
     return __last_iter;
   }
diff --git a/libcxx/include/__algorithm/sift_down.h b/libcxx/include/__algorithm/sift_down.h
index e01c9b2b00f86..3a8956bfc267a 100644
--- a/libcxx/include/__algorithm/sift_down.h
+++ b/libcxx/include/__algorithm/sift_down.h
@@ -38,7 +38,7 @@ __sift_down(_RandomAccessIterator __first,
   // right-child of __start is at 2 * __start + 2
   difference_type __child = __start;
 
-  if (__len < 2 || (__len - 2) / 2 < __child)
+  if (__len < 2)
     return;
 
   __child = 2 * __child + 1;
@@ -62,7 +62,7 @@ __sift_down(_RandomAccessIterator __first,
     __first[__start] = _Ops::__iter_move(__first + __child);
     __start          = __child;
 
-    if ((__len - 2) / 2 < __child)
+    if (__len / 2 - 1 < __child)
       break;
 
     // recompute the child based off of the updated parent
@@ -85,16 +85,18 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _RandomAccessIterator __floy
     _RandomAccessIterator __first,
     _Compare&& __comp,
     typename iterator_traits<_RandomAccessIterator>::difference_type __len) {
-  using difference_type = typename iterator_traits<_RandomAccessIterator>::difference_type;
-  _LIBCPP_ASSERT_INTERNAL(__len >= 2, "shouldn't be called unless __len >= 2");
+  _LIBCPP_ASSERT_INTERNAL(__len > 1, "shouldn't be called unless __len > 1");
 
-  _RandomAccessIterator __hole    = __first;
-  _RandomAccessIterator __child_i = __first;
-  difference_type __child         = 0;
+  using _Ops = _IterOps<_AlgPolicy>;
 
-  while (true) {
-    __child_i += difference_type(__child + 1);
-    __child = 2 * __child + 1;
+  typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;
+
+  difference_type __child      = 1;
+  _RandomAccessIterator __hole = __first, __child_i = __first;
+
+  do {
+    __child_i += __child;
+    __child *= 2;
 
     if ((__child + 1) < __len && __comp(*__child_i, *(__child_i + difference_type(1)))) {
       // right-child exists and is greater than left-child
@@ -103,13 +105,12 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _RandomAccessIterator __floy
     }
 
     // swap __hole with its largest child
-    *__hole = _IterOps<_AlgPolicy>::__iter_move(__child_i);
+    *__hole = _Ops::__iter_move(__child_i);
     __hole  = __child_i;
 
     // if __hole is now a leaf, we're done
-    if (__child > (__len - 2) / 2)
-      return __hole;
-  }
+  } while (__child <= __len / 2);
+  return __hole;
 }
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/__algorithm/sort_heap.h b/libcxx/include/__algorithm/sort_heap.h
index f20b110c7fd12..f2d73ab793650 100644
--- a/libcxx/include/__algorithm/sort_heap.h
+++ b/libcxx/include/__algorithm/sort_heap.h
@@ -36,8 +36,8 @@ __sort_heap(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compar
   __comp_ref_type<_Compare> __comp_ref = __comp;
 
   using difference_type = typename iterator_traits<_RandomAccessIterator>::difference_type;
-  for (difference_type __n = __last - __first; __n > 1; --__last, (void)--__n)
-    std::__pop_heap<_AlgPolicy>(__first, __last, __comp_ref, __n);
+  for (difference_type __n = __last - __first; __n > 1; --__n)
+    std::__pop_heap<_AlgPolicy>(__first, --__last, __comp_ref, __n);
   std::__check_strict_weak_ordering_sorted(__first, __saved_last, __comp_ref);
 }
 



More information about the libcxx-commits mailing list