about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMara Bos <m-ou.se@m-ou.se>2020-11-12 19:46:11 +0100
committerGitHub <noreply@github.com>2020-11-12 19:46:11 +0100
commit40889819ee8bf53aa880bace5ae472bdb84c797d (patch)
treeb1d5fd0b6300b5ad4b1f03424d880df26dfbdc40
parent755dd14e00fd9007a67779b129e81b47c6435113 (diff)
parent387568cd564317ca7491e6960ddcbe13beecae13 (diff)
downloadrust-40889819ee8bf53aa880bace5ae472bdb84c797d.tar.gz
rust-40889819ee8bf53aa880bace5ae472bdb84c797d.zip
Rollup merge of #78857 - SkiFire13:bheap-opt, r=KodrAus
Improve BinaryHeap performance

By changing the condition in the loops from `child < end` to `child < end - 1` we're guaranteed that `right = child + 1 < end` and since finding the index of the biggest sibling can be done with an arithmetic operation we can remove a branch from the loop body. The case where there's no right child, i.e. `child == end - 1` is instead handled outside the loop, after it ends; note that if the loops ends early we can use `return` instead of `break` since the check `child == end - 1` will surely fail.

I've also removed a call to `<[T]>::swap` that was hiding a bound check that [wasn't being optimized by LLVM](https://godbolt.org/z/zrhdGM).

A quick benchmarks on my pc shows that the gains are pretty significant:

|name                 |before ns/iter  |after ns/iter  |diff ns/iter  |diff %    |speedup |
|---------------------|----------------|---------------|--------------|----------|--------|
|find_smallest_1000   | 352,565        | 260,098       |     -92,467  | -26.23%  | x 1.36 |
|from_vec             | 676,795        | 473,934       |    -202,861  | -29.97%  | x 1.43 |
|into_sorted_vec      | 469,511        | 304,275       |    -165,236  | -35.19%  | x 1.54 |
|pop                  | 483,198        | 373,778       |    -109,420  | -22.64%  | x 1.29 |

The other 2 benchmarks for `BinaryHeap` (`peek_mut_deref_mut` and `push`) weren't impacted and as such didn't show any significant change.
-rw-r--r--library/alloc/src/collections/binary_heap.rs32
1 files changed, 19 insertions, 13 deletions
diff --git a/library/alloc/src/collections/binary_heap.rs b/library/alloc/src/collections/binary_heap.rs
index b67c72d7136..97ebc12175f 100644
--- a/library/alloc/src/collections/binary_heap.rs
+++ b/library/alloc/src/collections/binary_heap.rs
@@ -495,7 +495,14 @@ impl<T: Ord> BinaryHeap<T> {
         let mut end = self.len();
         while end > 1 {
             end -= 1;
-            self.data.swap(0, end);
+            // SAFETY: `end` goes from `self.len() - 1` to 1 (both included),
+            //  so it's always a valid index to access.
+            //  It is safe to access index 0 (i.e. `ptr`), because
+            //  1 <= end < self.len(), which means self.len() >= 2.
+            unsafe {
+                let ptr = self.data.as_mut_ptr();
+                ptr::swap(ptr, ptr.add(end));
+            }
             self.sift_down_range(0, end);
         }
         self.into_vec()
@@ -531,19 +538,19 @@ impl<T: Ord> BinaryHeap<T> {
         unsafe {
             let mut hole = Hole::new(&mut self.data, pos);
             let mut child = 2 * pos + 1;
-            while child < end {
-                let right = child + 1;
+            while child < end - 1 {
                 // compare with the greater of the two children
-                if right < end && hole.get(child) <= hole.get(right) {
-                    child = right;
-                }
+                child += (hole.get(child) <= hole.get(child + 1)) as usize;
                 // if we are already in order, stop.
                 if hole.element() >= hole.get(child) {
-                    break;
+                    return;
                 }
                 hole.move_to(child);
                 child = 2 * hole.pos() + 1;
             }
+            if child == end - 1 && hole.element() < hole.get(child) {
+                hole.move_to(child);
+            }
         }
     }
 
@@ -563,15 +570,14 @@ impl<T: Ord> BinaryHeap<T> {
         unsafe {
             let mut hole = Hole::new(&mut self.data, pos);
             let mut child = 2 * pos + 1;
-            while child < end {
-                let right = child + 1;
-                // compare with the greater of the two children
-                if right < end && hole.get(child) <= hole.get(right) {
-                    child = right;
-                }
+            while child < end - 1 {
+                child += (hole.get(child) <= hole.get(child + 1)) as usize;
                 hole.move_to(child);
                 child = 2 * hole.pos() + 1;
             }
+            if child == end - 1 {
+                hole.move_to(child);
+            }
             pos = hole.pos;
         }
         self.sift_up(start, pos);