Merge pull request #3479 from Xinok/issue12966

Fix Issue 12966 - Optimization for BinaryHeap

std/algorithm/sorting.d

@@ -1123,7 +1123,7 @@ private void quickSortImpl(alias less, Range)(Range r, size_t depth)
     {
         if (depth == 0)
         {
-            HeapSortImpl!(less, Range).heapSort(r);
+            HeapOps!(less, Range).heapSort(r);
             return;
         }
         depth = depth >= depth.max / 2 ? (depth / 3) * 2 : (depth * 2) / 3;
@@ -1167,8 +1167,8 @@ private void quickSortImpl(alias less, Range)(Range r, size_t depth)
     }
 }
 
-// Bottom-Up Heap-Sort Implementation
+// Heap operations for random-access ranges
-private template HeapSortImpl(alias less, Range)
+package(std) template HeapOps(alias less, Range)
 {
     import std.algorithm.mutation : swapAt;
 
@@ -1185,21 +1185,27 @@ private template HeapSortImpl(alias less, Range)
         if(r.length < 2) return;
 
         // Build Heap
-        size_t i = r.length / 2;
+        buildHeap(r);
-        while(i > 0) sift(r, --i, r.length);
 
         // Sort
-        i = r.length - 1;
+        size_t i = r.length - 1;
         while(i > 0)
         {
             swapAt(r, 0, i);
-            sift(r, 0, i);
+            percolate(r, 0, i);
             --i;
         }
     }
 
     //template because of @@@12410@@@
-    void sift()(Range r, size_t parent, immutable size_t end)
+    void buildHeap()(Range r)
+    {
+        size_t i = r.length / 2;
+        while(i > 0) percolate(r, --i, r.length);
+    }
+
+    //template because of @@@12410@@@
+    void percolate()(Range r, size_t parent, immutable size_t end)
     {
         immutable root = parent;
         size_t child = void;

std/container/binaryheap.d

@@ -59,9 +59,16 @@ if (isRandomAccessRange!(Store) || isRandomAccessRange!(typeof(Store.init[])))
 {
     import std.functional : binaryFun;
     import std.exception : enforce;
-    import std.algorithm : move, min;
+    import std.algorithm : move, min, HeapOps, swapAt;
     import std.typecons : RefCounted, RefCountedAutoInitialize;
 
+    static if(isRandomAccessRange!Store)
+        alias Range = Store;
+    else
+        alias Range = typeof(Store.init[]);
+    alias percolate = HeapOps!(less, Range).percolate;
+    alias buildHeap = HeapOps!(less, Range).buildHeap;
+
 // Really weird @@BUG@@: if you comment out the "private:" label below,
 // std.algorithm can't unittest anymore
 //private:
@@ -103,30 +110,6 @@ if (isRandomAccessRange!(Store) || isRandomAccessRange!(typeof(Store.init[])))
         }
     }
 
-    // Assuming the element at index i perturbs the heap property in
-    // store r, percolates it down the heap such that the heap
-    // property is restored.
-    private void percolateDown(Store r, size_t i, size_t length)
-    {
-        for (;;)
-        {
-            auto left = i * 2 + 1, right = left + 1;
-            if (right == length)
-            {
-                if (comp(r[i], r[left])) swap(r, i, left);
-                return;
-            }
-            if (right > length) return;
-            assert(left < length && right < length);
-            auto largest = comp(r[i], r[left])
-                ? (comp(r[left], r[right]) ? right : left)
-                : (comp(r[i], r[right]) ? right : i);
-            if (largest == i) return;
-            swap(r, i, largest);
-            i = largest;
-        }
-    }
-
     // @@@BUG@@@: add private here, std.algorithm doesn't unittest anymore
     /*private*/ void pop(Store store)
     {
@@ -136,29 +119,7 @@ if (isRandomAccessRange!(Store) || isRandomAccessRange!(typeof(Store.init[])))
         auto t2 = moveBack(store[]);
         store.front = move(t2);
         store.back = move(t1);
-        percolateDown(store, 0, store.length - 1);
+        percolate(store[], 0, store.length - 1);
-    }
-
-    /*private*/ static void swap(Store _store, size_t i, size_t j)
-    {
-        static if (is(typeof(swap(_store[i], _store[j]))))
-        {
-            swap(_store[i], _store[j]);
-        }
-        else static if (is(typeof(_store.moveAt(i))))
-        {
-            auto t1 = _store.moveAt(i);
-            auto t2 = _store.moveAt(j);
-            _store[i] = move(t2);
-            _store[j] = move(t1);
-        }
-        else // assume it's a container and access its range with []
-        {
-            auto t1 = _store[].moveAt(i);
-            auto t2 = _store[].moveAt(j);
-            _store[i] = move(t2);
-            _store[j] = move(t1);
-        }
     }
 
 public:
@@ -186,11 +147,7 @@ the heap work incorrectly.
         _store = move(s);
         _length = min(_store.length, initialSize);
         if (_length < 2) return;
-        for (auto i = (_length - 2) / 2; ; )
+        buildHeap(s[]);
-        {
-            this.percolateDown(_store, i, _length);
-            if (i-- == 0) break;
-        }
         assertValid();
     }
 
@@ -320,7 +277,7 @@ and $(D length == capacity), throws an exception.
             auto parentIdx = (n - 1) / 2;
             if (!comp(_store[parentIdx], _store[n])) break; // done!
             // must swap and continue
-            swap(_store, parentIdx, n);
+            swapAt(_store, parentIdx, n);
             n = parentIdx;
         }
         ++_length;
@@ -342,7 +299,7 @@ Removes the largest element from the heap.
             _store[_length - 1] = move(t1);
         }
         --_length;
-        percolateDown(_store, 0, _length);
+        percolate(_store[], 0, _length);
     }
 
     /// ditto
@@ -368,7 +325,7 @@ Replaces the largest element in the store with $(D value).
         // must replace the top
         assert(!empty, "Cannot call replaceFront on an empty heap.");
         _store.front = value;
-        percolateDown(_store, 0, _length);
+        percolate(_store[], 0, _length);
         debug(BinaryHeap) assertValid();
     }
 
@@ -392,7 +349,7 @@ must be collected.
         assert(!_store.empty, "Cannot replace front of an empty heap.");
         if (!comp(value, _store.front)) return false; // value >= largest
         _store.front = value;
-        percolateDown(_store, 0, _length);
+        percolate(_store[], 0, _length);
         debug(BinaryHeap) assertValid();
         return true;
     }