Thread (24 messages) 24 messages, 4 authors, 13d ago

Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min

From: Kristofer Karlsson <hidden>
Date: 2026-07-08 11:00:09

On Wed, 8 Jul 2026 at 12:44, René Scharfe [off-list ref] wrote:
tl;dr: Yes, please, but I'm biased.

The text size of prio-queue.o on Apple silicon increases from 1351 to
1563 bytes for me, 212 bytes or 16% more.  OK.

It makes intuitive sense to find the new position of the last item by
searching from the bottom up instead of from the top down.  Timings
confirm it.  Are there pathologic cases that perform worse, though?  I
don't see how to construct one.  It would require an unbalanced heap,
where the bottom items from one branch would rise high in other
branches.  Is this even possible?
Agreed, I also struggle to come up with such a case. Perhaps
theoretically possible to construct, but would not invalidate
the general heuristic?
For a full drain (only _get(), no _put()) of up to 12 items the answer
is no, at least.  Cascade never needs more comparisons for any
permutation; test code below.  Here are the aggregate numbers:

       next           cascade
    n  min max  mean  min max  mean
    2    0   0   0.0    0   0   0.0
    3    1   1   1.0    1   1   1.0
    4    3   3   3.0    3   3   3.0
    5    5   6   5.8    5   6   5.6
    6    7  10   8.7    7   9   8.0
    7   10  14  12.0    9  12  10.9
    8   14  18  16.3   12  16  13.9
    9   18  23  20.9   15  20  17.4
   10   22  29  25.5   18  24  20.7
   11   26  35  30.5   21  28  24.4
   12   30  41  35.5   24  33  27.9
I am sincerely grateful that you took the time to analyze it
to this level of detail. Very nice analysis and data!
sift_up_rebalance() is a combination of sift_down_root() with an empty
root and the bubble-up operation from prio_queue_put().  The latter can
easily be factored out into a sift-up function, reducing code
duplication.
Good point! I am not sure how messy this gets in practice, but I
will see if I can implement this split for the next patch.
Extending sift_down_root() to deal with an empty root would be easy as
well, but also a bit tricky to avoid pointless checks for each caller.
Not sure it's worth it.  Like this perhaps?

static inline size_t sift_down_root(struct prio_queue *queue, bool empty)
{
        size_t ix, child;

        /* Push down the one at the root */
        for (ix = 0; ix * 2 + 1 < queue->nr_; ix = child) {
                child = ix * 2 + 1; /* left */
                if (child + 1 < queue->nr_ &&
                    compare(queue, child, child + 1) >= 0)
                        child++; /* use right child */

                if (empty)
                        queue->array[ix] = queue->array[child];
                else if (compare(queue, ix, child) <= 0)
                        break;
                else
                        swap(queue, child, ix);
        }
        return ix;
}
Yes, something like that would work, but I agree -- ideally we
could have something that's even nicer and avoids the boolean flag
for split behavior.
Anyway, my point is that it's not "adding another sift function", but
remixing existing ones, which I only count as half. :)

I'd very much like to see this go in because it seems to be strictly
faster, makes intuitive sense and adds only little code.   I didn't
find this method used anywhere else, which is a warning sign, but I
can't find any catch.
Thanks, I think that is enough motivation for me to at least attempt
another version and then it will be easier to reason about dropping
or keeping.

I am not sure why it's a warning sign to have no other usages,
especially when it's a file local static function. I guess it could
be inlined instead (though I would not prefer that).

Thanks for the very thorough and insightful review,
Kristofer
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help