[libcxx-commits] [PATCH] D118003: [libc++] Floyd's improvement to pop_heap

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone created this revision.
Quuxplusone added reviewers: ldionne, howard.hinnant, mclow.lists, respindola, Mordante, libc++, var-const, philnik, jloser.
Quuxplusone added a project: libc++.
Quuxplusone requested review of this revision.
Herald added a subscriber: libcxx-commits.
Herald added 1 blocking reviewer(s): libc++.

Fixes https://github.com/llvm/llvm-project/issues/10008

As the issue description says:

> [`__sift_down`] is normally used to put in the correct position an element that was previously a leaf of a heap
> (as in the `pop_heap` implementation). [When] that element was a leaf, it is likely that it will move down when
> placed in the root.
>
> An optimization is to move down the empty space at the root by swapping it with the largest child.
> When the empty space becomes a leaf, the last element is moved to its place and moved up if necessary.
>
> The advantage is that only one comparison per step is needed to move the empty space down.

A demo is here: https://godbolt.org/z/M8do8sWhT
For the same 100,000 random elements made into a heap using `make_heap`:
libstdc++ `sort_heap` takes 1,934,727 moves; 1,534,715 comparisons.
libc++ trunk `sort_heap` takes 1,900,731 moves; 2,831,757 comparisons.
libc++ after this patch takes 1,764,997 moves; 1,534,701 comparisons.
(Note that this really //is// apples to apples: hashing proves that libstdc++'s `make_heap` produces the exact same 100,000-element sequence as libc++'s `make_heap`.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118003

Files:
  libcxx/include/__algorithm/pop_heap.h
  libcxx/include/__algorithm/sift_down.h


Index: libcxx/include/__algorithm/sift_down.h
===================================================================
--- libcxx/include/__algorithm/sift_down.h
+++ libcxx/include/__algorithm/sift_down.h
@@ -10,6 +10,7 @@
 #define _LIBCPP___ALGORITHM_SIFT_DOWN_H
 
 #include <__config>
+#include <__debug>
 #include <__iterator/iterator_traits.h>
 #include <__utility/move.h>
 
@@ -73,6 +74,39 @@
     *__start = _VSTD::move(__top);
 }
 
+template <class _Compare, class _RandomAccessIterator>
+_LIBCPP_CONSTEXPR_AFTER_CXX11 _RandomAccessIterator
+__floyd_sift_down(_RandomAccessIterator __first, _Compare __comp,
+                  typename iterator_traits<_RandomAccessIterator>::difference_type __len)
+{
+    typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;
+    _LIBCPP_ASSERT(__len >= 2, "shouldn't be called unless __len >= 2");
+
+    _RandomAccessIterator __hole = __first;
+    _RandomAccessIterator __child_i = __first;
+    difference_type __child = 0;
+
+    while (true) {
+        __child_i += difference_type(__child + 1);
+        __child = 2 * __child + 1;
+
+        if ((__child + 1) < __len && __comp(*__child_i, *(__child_i + difference_type(1)))) {
+            // right-child exists and is greater than left-child
+            ++__child_i;
+            ++__child;
+        }
+
+        // swap __hole with its largest child
+        *__hole = _VSTD::move(*__child_i);
+        __hole = _VSTD::move(__child_i);
+
+        // if __hole is now a leaf, we're done
+        if (__child > (__len - 2) / 2) {
+            return __hole;
+        }
+    }
+}
+
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ALGORITHM_SIFT_DOWN_H
Index: libcxx/include/__algorithm/pop_heap.h
===================================================================
--- libcxx/include/__algorithm/pop_heap.h
+++ libcxx/include/__algorithm/pop_heap.h
@@ -11,10 +11,11 @@
 
 #include <__algorithm/comp.h>
 #include <__algorithm/comp_ref_type.h>
+#include <__algorithm/push_heap.h>
 #include <__algorithm/sift_down.h>
 #include <__config>
 #include <__iterator/iterator_traits.h>
-#include <__utility/swap.h>
+#include <__utility/move.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
@@ -28,10 +29,21 @@
 __pop_heap(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare __comp,
            typename iterator_traits<_RandomAccessIterator>::difference_type __len)
 {
+    typedef typename iterator_traits<_RandomAccessIterator>::value_type value_type;
+
     if (__len > 1)
     {
-        swap(*__first, *--__last);
-        _VSTD::__sift_down<_Compare>(__first, __comp, __len - 1, __first);
+        value_type __top = _VSTD::move(*__first);  // create a hole at __first
+        _RandomAccessIterator __hole = _VSTD::__floyd_sift_down<_Compare>(__first, __comp, __len);
+        --__last;
+        if (__hole == __last) {
+            *__hole = _VSTD::move(__top);
+        } else {
+            *__hole = _VSTD::move(*__last);
+            ++__hole;
+            *__last = _VSTD::move(__top);
+            _VSTD::__sift_up<_Compare>(__first, __hole, __comp, __hole - __first);
+        }
     }
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118003.402364.patch
Type: text/x-patch
Size: 3182 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220123/0a2a3241/attachment.bin>