[libc++] Optimize make_heap

Author: philnik777

libcxx/include/__algorithm/make_heap.h

@@ -12,9 +12,11 @@
 #include <__algorithm/comp.h>
 #include <__algorithm/comp_ref_type.h>
 #include <__algorithm/iterator_operations.h>
+#include <__algorithm/push_heap.h>
 #include <__algorithm/sift_down.h>
 #include <__config>
 #include <__iterator/iterator_traits.h>
+#include <__type_traits/is_arithmetic.h>
 #include <__utility/move.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -31,13 +33,23 @@ inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
 __make_heap(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare&& __comp) {
   __comp_ref_type<_Compare> __comp_ref = __comp;
 
-  using difference_type = typename iterator_traits<_RandomAccessIterator>::difference_type;
-  difference_type __n   = __last - __first;
+  using __diff_t     = __iter_diff_t<_RandomAccessIterator>;
+  const __diff_t __n = __last - __first;
+
+  static const bool __assume_both_children = is_arithmetic<__iter_value_type<_RandomAccessIterator> >::value;
+
+  // While it would be correct to always assume we have both children, in practice we observed this to be a performance
+  // improvement only for arithmetic types.
+  const __diff_t __sift_down_n = __assume_both_children ? ((__n & 1) ? __n : __n - 1) : __n;
+
   if (__n > 1) {
     // start from the first parent, there is no need to consider children
-    for (difference_type __start = (__n - 2) / 2; __start >= 0; --__start) {
-      std::__sift_down<_AlgPolicy>(__first, __comp_ref, __n, __first + __start);
+
+    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);
     }
+    if _LIBCPP_CONSTEXPR (__assume_both_children)
+      std::__sift_up<_AlgPolicy>(__first, __last, __comp, __n);
   }
 }
 

libcxx/include/__algorithm/partial_sort.h

@@ -45,7 +45,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _RandomAccessIterator __part
   for (; __i != __last; ++__i) {
     if (__comp(*__i, *__first)) {
       _IterOps<_AlgPolicy>::iter_swap(__i, __first);
-      std::__sift_down<_AlgPolicy>(__first, __comp, __len, __first);
+      std::__sift_down<_AlgPolicy, false>(__first, __comp, __len, 0);
     }
   }
   std::__sort_heap<_AlgPolicy>(std::move(__first), std::move(__middle), __comp);

libcxx/include/__algorithm/partial_sort_copy.h

@@ -60,7 +60,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair<_InputIterator, _Random
     for (; __first != __last; ++__first)
       if (std::__invoke(__comp, std::__invoke(__proj1, *__first), std::__invoke(__proj2, *__result_first))) {
         *__result_first = *__first;
-        std::__sift_down<_AlgPolicy>(__result_first, __projected_comp, __len, __result_first);
+        std::__sift_down<_AlgPolicy, false>(__result_first, __projected_comp, __len, 0);
       }
     std::__sort_heap<_AlgPolicy>(__result_first, __r, __projected_comp);
   }

libcxx/include/__algorithm/sift_down.h

@@ -24,59 +24,60 @@ _LIBCPP_PUSH_MACROS
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-template <class _AlgPolicy, class _Compare, class _RandomAccessIterator>
+template <class _AlgPolicy, bool __assume_both_children, class _Compare, class _RandomAccessIterator>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
 __sift_down(_RandomAccessIterator __first,
             _Compare&& __comp,
-            typename iterator_traits<_RandomAccessIterator>::difference_type __len,
-            _RandomAccessIterator __start) {
+            __iter_diff_t<_RandomAccessIterator> __len,
+            __iter_diff_t<_RandomAccessIterator> __start) {
   using _Ops = _IterOps<_AlgPolicy>;
 
   typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;
   typedef typename iterator_traits<_RandomAccessIterator>::value_type value_type;
   // left-child of __start is at 2 * __start + 1
   // right-child of __start is at 2 * __start + 2
-  difference_type __child = __start - __first;
+  difference_type __child = __start;
 
   if (__len < 2 || (__len - 2) / 2 < __child)
     return;
 
-  __child                         = 2 * __child + 1;
-  _RandomAccessIterator __child_i = __first + __child;
+  __child = 2 * __child + 1;
 
-  if ((__child + 1) < __len && __comp(*__child_i, *(__child_i + difference_type(1)))) {
+  if _LIBCPP_CONSTEXPR (__assume_both_children) {
+    // right-child exists and is greater than left-child
+    __child += __comp(__first[__child], __first[__child + 1]);
+  } else if ((__child + 1) < __len && __comp(__first[__child], __first[__child + 1])) {
     // right-child exists and is greater than left-child
-    ++__child_i;
     ++__child;
   }
 
   // check if we are in heap-order
-  if (__comp(*__child_i, *__start))
+  if (__comp(__first[__child], __first[__start]))
     // we are, __start is larger than its largest child
     return;
 
-  value_type __top(_Ops::__iter_move(__start));
+  value_type __top(_Ops::__iter_move(__first + __start));
   do {
     // we are not in heap-order, swap the parent with its largest child
-    *__start = _Ops::__iter_move(__child_i);
-    __start  = __child_i;
+    __first[__start] = _Ops::__iter_move(__first + __child);
+    __start          = __child;
 
     if ((__len - 2) / 2 < __child)
       break;
 
     // recompute the child based off of the updated parent
-    __child   = 2 * __child + 1;
-    __child_i = __first + __child;
+    __child = 2 * __child + 1;
 
-    if ((__child + 1) < __len && __comp(*__child_i, *(__child_i + difference_type(1)))) {
+    if _LIBCPP_CONSTEXPR (__assume_both_children) {
+      __child += __comp(__first[__child], __first[__child + 1]);
+    } else if ((__child + 1) < __len && __comp(__first[__child], __first[__child + 1])) {
       // right-child exists and is greater than left-child
-      ++__child_i;
       ++__child;
     }
 
     // check if we are in heap-order
-  } while (!__comp(*__child_i, __top));
-  *__start = std::move(__top);
+  } while (!__comp(__first[__child], __top));
+  __first[__start] = std::move(__top);
 }
 
 template <class _AlgPolicy, class _Compare, class _RandomAccessIterator>

libcxx/test/libcxx/algorithms/bad_iterator_traits.verify.cpp

@@ -57,5 +57,5 @@ struct BadIter {
 // behavior when these iterators are passed to standard algorithms.
 void test() {
   std::sort(BadIter(), BadIter());
-  //expected-error-re@*:* {{static assertion failed {{.*}}It looks like your iterator's `iterator_traits<It>::reference` does not match the return type of dereferencing the iterator}}
+  //expected-error-re@*:* 2 {{static assertion failed {{.*}}It looks like your iterator's `iterator_traits<It>::reference` does not match the return type of dereferencing the iterator}}
 }