This uses fewer comparisons than the previous code (61% as
many for large random inputs), but produces identical results;
it actually performs the exact same series of swap operations.

Standard heapsort, when sifting down, performs two comparisons
per level: One to find the greater child, and a second to see
if the current node should be exchanged with that child.

Bottom-up heapsort observes that it's better to postpone the second
comparison and search for the leaf where -infinity would be sent to,
then search back *up* for the current node's destination.

Since sifting down usually proceeds to the leaf level (that's where
half the nodes are), this does many fewer second comparisons.  That
saves a lot of (expensive since Spectre) indirect function calls.

The one time it's worse than the previous code is if there are large
numbers of duplicate keys, when the top-down algorithm is O(n) and
bottom-up is O(n log n).  For distinct keys, it's provably always better.

(The code is not significantly more complex.  This patch also merges
the heap-building and -extracting sift-down loops, resulting in a
net code size savings.)

x86-64 code size 885 -> 770 bytes (-115)

(I see the checkpatch complaint about "else if (n -= size)".
The alternative is significantly uglier.)

Signed-off-by: George Spelvin <l...@sdf.org>
---
 lib/sort.c | 102 +++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 75 insertions(+), 27 deletions(-)

diff --git a/lib/sort.c b/lib/sort.c
index dff2ab2e196e..2aef4631e7d3 100644
--- a/lib/sort.c
+++ b/lib/sort.c
@@ -117,6 +117,32 @@ static void generic_swap(void *a, void *b, int size)
        } while (n);
 }
 
+/**
+ * parent - given the offset of the child, find the offset of the parent.
+ * @i: the offset of the heap element whose parent is sought.  Non-zero.
+ * @lsbit: a precomputed 1-bit mask, equal to "size & -size"
+ * @size: size of each element
+ *
+ * In terms of array indexes, the parent of element j = i/size is simply
+ * (j-1)/2.  But when working in byte offsets, we can't use implicit
+ * truncation of integer divides.
+ *
+ * Fortunately, we only need one bit of the quotient, not the full divide.
+ * size has a least significant bit.  That bit will be clear if i is
+ * an even multiple of size, and set if it's an odd multiple.
+ *
+ * Logically, we're doing "if (i & lsbit) i -= size;", but since the
+ * branch is unpredictable, it's done with a bit of clever branch-free
+ * code instead.
+ */
+__attribute_const__ __always_inline
+static size_t parent(size_t i, unsigned int lsbit, size_t size)
+{
+       i -= size;
+       i -= size & -(i & lsbit);
+       return i / 2;
+}
+
 /**
  * sort - sort an array of elements
  * @base: pointer to data to sort
@@ -125,21 +151,26 @@ static void generic_swap(void *a, void *b, int size)
  * @cmp_func: pointer to comparison function
  * @swap_func: pointer to swap function or NULL
  *
- * This function does a heapsort on the given array. You may provide a
- * swap_func function optimized to your element type.
+ * This function does a heapsort on the given array.  You may provide a
+ * swap_func function if you need to do something more than a memory copy
+ * (e.g. fix up pointers or auxiliary data), but the built-in swap isn't
+ * usually a bottleneck.
  *
  * Sorting time is O(n log n) both on average and worst-case. While
  * qsort is about 20% faster on average, it suffers from exploitable
  * O(n*n) worst-case behavior and extra memory requirements that make
  * it less suitable for kernel use.
  */
-
 void sort(void *base, size_t num, size_t size,
          int (*cmp_func)(const void *, const void *),
          void (*swap_func)(void *, void *, int size))
 {
        /* pre-scale counters for performance */
-       int i = (num/2 - 1) * size, n = num * size, c, r;
+       size_t n = num * size, a = (num/2) * size;
+       unsigned const lsbit = size & -size;    /* Used to find parent */
+
+       if (!n)
+               return;
 
        if (!swap_func) {
                if (alignment_ok(base, size, 8))
@@ -150,30 +181,47 @@ void sort(void *base, size_t num, size_t size,
                        swap_func = generic_swap;
        }
 
-       /* heapify */
-       for ( ; i >= 0; i -= size) {
-               for (r = i; r * 2 + size < n; r  = c) {
-                       c = r * 2 + size;
-                       if (c < n - size &&
-                                       cmp_func(base + c, base + c + size) < 0)
-                               c += size;
-                       if (cmp_func(base + r, base + c) >= 0)
-                               break;
-                       swap_func(base + r, base + c, size);
-               }
-       }
+       /*
+        * Loop invariants:
+        * 1. elements [a,n) satisfy the heap property (compare greater than
+        *    all of their children),
+        * 2. elements [n,num*size) are sorted, and
+        * 3. a <= b <= c <= d <= n (whenever they are valid).
+        */
+       for (;;) {
+               size_t b, c, d;
 
-       /* sort */
-       for (i = n - size; i > 0; i -= size) {
-               swap_func(base, base + i, size);
-               for (r = 0; r * 2 + size < i; r = c) {
-                       c = r * 2 + size;
-                       if (c < i - size &&
-                                       cmp_func(base + c, base + c + size) < 0)
-                               c += size;
-                       if (cmp_func(base + r, base + c) >= 0)
-                               break;
-                       swap_func(base + r, base + c, size);
+               if (a)                  /* Building heap: sift down --a */
+                       a -= size;
+               else if (n -= size)     /* Sorting: Extract root to --n */
+                       swap_func(base, base + n, size);
+               else                    /* Sort complete */
+                       break;
+
+               /*
+                * Sift element at "a" down into heap.  This is the
+                * "bottom-up" variant, which significantly reduces
+                * calls to cmp_func(): we find the sift-down path all
+                * the way to the leaves (one compare per level), then
+                * backtrack to find where to insert the target element.
+                *
+                * Because elements tend to sift down close to the leaves,
+                * this uses fewer compares than doing two per level
+                * on the way down.  (A bit more than half as many on
+                * average, 3/4 worst-case.)
+                */
+               for (b = a; c = 2*b + size, (d = c + size) < n;)
+                       b = cmp_func(base + c, base + d) >= 0 ? c : d;
+               if (d == n)     /* Special case last leaf with no sibling */
+                       b = c;
+
+               /* Now backtrack from "b" to the correct location for "a" */
+               while (b != a && cmp_func(base + a, base + b) >= 0)
+                       b = parent(b, lsbit, size);
+               c = b;                  /* Where "a" belongs */
+               while (b != a) {        /* Shift it into place */
+                       b = parent(b, lsbit, size);
+                       swap_func(base + b, base + c, size);
                }
        }
 }
-- 
2.20.1

Reply via email to