<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Jul 22, 2014, at 8:48 PM, David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr">Should this be merged for 3.5?</div></blockquote><div><br></div>Yes.</div><div><br></div><div>— Marshall</div><div><br><blockquote type="cite"><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jul 21, 2014 at 11:07 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: majnemer<br>
Date: Tue Jul 22 01:07:09 2014<br>
New Revision: 213615<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=213615&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=213615&view=rev</a><br>
Log:<br>
Fix std::make_heap's worst case time complexity<br>
<br>
std::make_heap is currently implemented by iteratively applying a<br>
siftup-type algorithm.  Since sift-up is O(ln n), this gives<br>
std::make_heap a worst case time complexity of O(n ln n).<br>
<br>
The C++ standard mandates that std::make_heap make no more than O(3n)<br>
comparisons, this makes our std::make_heap out of spec.<br>
<br>
Fix this by introducing an implementation of __sift_down and switch<br>
std::make_heap to create the heap using it.<br>
This gives std::make_heap linear time complexity in the worst case.<br>
<br>
This fixes PR20161.<br>
<br>
Modified:<br>
    libcxx/trunk/include/algorithm<br>
    libcxx/trunk/test/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp<br>
<br>
Modified: libcxx/trunk/include/algorithm<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/algorithm?rev=213615&r1=213614&r2=213615&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/algorithm?rev=213615&r1=213614&r2=213615&view=diff</a><br>

==============================================================================<br>
--- libcxx/trunk/include/algorithm (original)<br>
+++ libcxx/trunk/include/algorithm Tue Jul 22 01:07:09 2014<br>
@@ -4794,49 +4794,8 @@ is_heap(_RandomAccessIterator __first, _<br>
<br>
 template <class _Compare, class _RandomAccessIterator><br>
 void<br>
-__push_heap_front(_RandomAccessIterator __first, _RandomAccessIterator, _Compare __comp,<br>
-                  typename iterator_traits<_RandomAccessIterator>::difference_type __len)<br>
-{<br>
-    typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;<br>
-    typedef typename iterator_traits<_RandomAccessIterator>::value_type value_type;<br>
-    if (__len > 1)<br>
-    {<br>
-        difference_type __p = 0;<br>
-        _RandomAccessIterator __pp = __first;<br>
-        difference_type __c = 2;<br>
-        _RandomAccessIterator __cp = __first + __c;<br>
-        if (__c == __len || __comp(*__cp, *(__cp - 1)))<br>
-        {<br>
-            --__c;<br>
-            --__cp;<br>
-        }<br>
-        if (__comp(*__pp, *__cp))<br>
-        {<br>
-            value_type __t(_VSTD::move(*__pp));<br>
-            do<br>
-            {<br>
-                *__pp = _VSTD::move(*__cp);<br>
-                __pp = __cp;<br>
-                __p = __c;<br>
-                __c = (__p + 1) * 2;<br>
-                if (__c > __len)<br>
-                    break;<br>
-                __cp = __first + __c;<br>
-                if (__c == __len || __comp(*__cp, *(__cp - 1)))<br>
-                {<br>
-                    --__c;<br>
-                    --__cp;<br>
-                }<br>
-            } while (__comp(__t, *__cp));<br>
-            *__pp = _VSTD::move(__t);<br>
-        }<br>
-    }<br>
-}<br>
-<br>
-template <class _Compare, class _RandomAccessIterator><br>
-void<br>
-__push_heap_back(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare __comp,<br>
-                 typename iterator_traits<_RandomAccessIterator>::difference_type __len)<br>
+__sift_up(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare __comp,<br>
+          typename iterator_traits<_RandomAccessIterator>::difference_type __len)<br>
 {<br>
     typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;<br>
     typedef typename iterator_traits<_RandomAccessIterator>::value_type value_type;<br>
@@ -4869,10 +4828,10 @@ push_heap(_RandomAccessIterator __first,<br>
 #ifdef _LIBCPP_DEBUG<br>
     typedef typename add_lvalue_reference<__debug_less<_Compare> >::type _Comp_ref;<br>
     __debug_less<_Compare> __c(__comp);<br>
-    __push_heap_back<_Comp_ref>(__first, __last, __c, __last - __first);<br>
+    __sift_up<_Comp_ref>(__first, __last, __c, __last - __first);<br>
 #else  // _LIBCPP_DEBUG<br>
     typedef typename add_lvalue_reference<_Compare>::type _Comp_ref;<br>
-    __push_heap_back<_Comp_ref>(__first, __last, __comp, __last - __first);<br>
+    __sift_up<_Comp_ref>(__first, __last, __comp, __last - __first);<br>
 #endif  // _LIBCPP_DEBUG<br>
 }<br>
<br>
@@ -4887,6 +4846,60 @@ push_heap(_RandomAccessIterator __first,<br>
 // pop_heap<br>
<br>
 template <class _Compare, class _RandomAccessIterator><br>
+void<br>
+__sift_down(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare __comp,<br>
+            typename iterator_traits<_RandomAccessIterator>::difference_type __len,<br>
+            _RandomAccessIterator __start)<br>
+{<br>
+    typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;<br>
+    typedef typename iterator_traits<_RandomAccessIterator>::value_type value_type;<br>
+    // left-child of __start is at 2 * __start + 1<br>
+    // right-child of __start is at 2 * __start + 2<br>
+    difference_type __child = __start - __first;<br>
+<br>
+    if (__len < 2 || (__len - 2) / 2 < __child)<br>
+        return;<br>
+<br>
+    __child = 2 * __child + 1;<br>
+    _RandomAccessIterator __child_i = __first + __child;<br>
+<br>
+    if ((__child + 1) < __len && __comp(*__child_i, *(__child_i + 1))) {<br>
+        // right-child exists and is greater than left-child<br>
+        ++__child_i;<br>
+        ++__child;<br>
+    }<br>
+<br>
+    // check if we are in heap-order<br>
+    if (__comp(*__child_i, *__start))<br>
+        // we are, __start is larger than it's largest child<br>
+        return;<br>
+<br>
+    value_type __top(_VSTD::move(*__start));<br>
+    do<br>
+    {<br>
+        // we are not in heap-order, swap the parent with it's largest child<br>
+        *__start = _VSTD::move(*__child_i);<br>
+        __start = __child_i;<br>
+<br>
+        if ((__len - 2) / 2 < __child)<br>
+            break;<br>
+<br>
+        // recompute the child based off of the updated parent<br>
+        __child = 2 * __child + 1;<br>
+        __child_i = __first + __child;<br>
+<br>
+        if ((__child + 1) < __len && __comp(*__child_i, *(__child_i + 1))) {<br>
+            // right-child exists and is greater than left-child<br>
+            ++__child_i;<br>
+            ++__child;<br>
+        }<br>
+<br>
+        // check if we are in heap-order<br>
+    } while (!__comp(*__child_i, __top));<br>
+    *__start = _VSTD::move(__top);<br>
+}<br>
+<br>
+template <class _Compare, class _RandomAccessIterator><br>
 inline _LIBCPP_INLINE_VISIBILITY<br>
 void<br>
 __pop_heap(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare __comp,<br>
@@ -4895,7 +4908,7 @@ __pop_heap(_RandomAccessIterator __first<br>
     if (__len > 1)<br>
     {<br>
         swap(*__first, *--__last);<br>
-        __push_heap_front<_Compare>(__first, __last, __comp, __len-1);<br>
+        __sift_down<_Compare>(__first, __last, __comp, __len - 1, __first);<br>
     }<br>
 }<br>
<br>
@@ -4932,10 +4945,11 @@ __make_heap(_RandomAccessIterator __firs<br>
     difference_type __n = __last - __first;<br>
     if (__n > 1)<br>
     {<br>
-        __last = __first;<br>
-        ++__last;<br>
-        for (difference_type __i = 1; __i < __n;)<br>
-            __push_heap_back<_Compare>(__first, ++__last, __comp, ++__i);<br>
+        // start from the first parent, there is no need to consider children<br>
+        for (difference_type __start = (__n - 2) / 2; __start >= 0; --__start)<br>
+        {<br>
+            __sift_down<_Compare>(__first, __last, __comp, __n, __first + __start);<br>
+        }<br>
     }<br>
 }<br>
<br>
@@ -5010,7 +5024,7 @@ __partial_sort(_RandomAccessIterator __f<br>
         if (__comp(*__i, *__first))<br>
         {<br>
             swap(*__i, *__first);<br>
-            __push_heap_front<_Compare>(__first, __middle, __comp, __len);<br>
+            __sift_down<_Compare>(__first, __middle, __comp, __len, __first);<br>
         }<br>
     }<br>
     __sort_heap<_Compare>(__first, __middle, __comp);<br>
@@ -5051,15 +5065,15 @@ __partial_sort_copy(_InputIterator __fir<br>
     _RandomAccessIterator __r = __result_first;<br>
     if (__r != __result_last)<br>
     {<br>
-        typename iterator_traits<_RandomAccessIterator>::difference_type __len = 0;<br>
-        for (; __first != __last && __r != __result_last; ++__first, ++__r, ++__len)<br>
+        for (; __first != __last && __r != __result_last; ++__first, ++__r)<br>
             *__r = *__first;<br>
         __make_heap<_Compare>(__result_first, __r, __comp);<br>
+        typename iterator_traits<_RandomAccessIterator>::difference_type __len = __r - __result_first;<br>
         for (; __first != __last; ++__first)<br>
             if (__comp(*__first, *__result_first))<br>
             {<br>
                 *__result_first = *__first;<br>
-                __push_heap_front<_Compare>(__result_first, __r, __comp, __len);<br>
+                __sift_down<_Compare>(__result_first, __r, __comp, __len, __result_first);<br>
             }<br>
         __sort_heap<_Compare>(__result_first, __r, __comp);<br>
     }<br>
<br>
Modified: libcxx/trunk/test/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp?rev=213615&r1=213614&r2=213615&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp?rev=213615&r1=213614&r2=213615&view=diff</a><br>

==============================================================================<br>
--- libcxx/trunk/test/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp (original)<br>
+++ libcxx/trunk/test/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp Tue Jul 22 01:07:09 2014<br>
@@ -35,14 +35,35 @@ struct indirect_less<br>
 void test(unsigned N)<br>
 {<br>
     int* ia = new int [N];<br>
+    {<br>
     for (int i = 0; i < N; ++i)<br>
         ia[i] = i;<br>
-    {<br>
     std::random_shuffle(ia, ia+N);<br>
     std::make_heap(ia, ia+N, std::greater<int>());<br>
     assert(std::is_heap(ia, ia+N, std::greater<int>()));<br>
     }<br>
<br>
+//  Ascending<br>
+    {<br>
+    binary_counting_predicate<std::greater<int>, int, int> pred ((std::greater<int>()));<br>
+    for (int i = 0; i < N; ++i)<br>
+        ia[i] = i;<br>
+    std::make_heap(ia, ia+N, std::ref(pred));<br>
+    assert(pred.count() <= 3*N);<br>
+    assert(std::is_heap(ia, ia+N, pred));<br>
+    }<br>
+<br>
+//  Descending<br>
+    {<br>
+    binary_counting_predicate<std::greater<int>, int, int> pred ((std::greater<int>()));<br>
+    for (int i = 0; i < N; ++i)<br>
+        ia[N-1-i] = i;<br>
+    std::make_heap(ia, ia+N, std::ref(pred));<br>
+    assert(pred.count() <= 3*N);<br>
+    assert(std::is_heap(ia, ia+N, pred));<br>
+    }<br>
+<br>
+//  Random<br>
     {<br>
     binary_counting_predicate<std::greater<int>, int, int> pred ((std::greater<int>()));<br>
     std::random_shuffle(ia, ia+N);<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</blockquote></div><br></body></html>