* [PATCH] prio-queue: use cascade-down sift for faster extract-min
@ 2026-05-31 17:57 Kristofer Karlsson via GitGitGadget
2026-06-01 0:09 ` Junio C Hamano
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Kristofer Karlsson via GitGitGadget @ 2026-05-31 17:57 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson
From: Kristofer Karlsson <krka@spotify.com>
Replace the standard sift-down in prio_queue_get() with a
cascade-down approach.
The standard approach places the last array element at the root,
then sifts it down. At each level this requires two comparisons
(left vs right child, then element vs winner) and, when the
element is larger, a swap (three 16-byte copies).
The cascade approach instead promotes the smaller child into the
vacant root slot at each level — one comparison and one copy.
The vacancy sinks to a leaf, where the last array element is
placed and sifted up if needed — typically zero levels since the
last array element tends to be large.
In the common case, work per extract drops from 2d comparisons
+ 3d copies to d comparisons + d copies: roughly half the
comparisons and a third of the data movement. The sift-up phase
can add work when the last element is smaller than ancestors of
the leaf vacancy, but this is rare in practice.
Simplify prio_queue_replace() to a plain get+put sequence. This
is semantically equivalent: the old implementation wrote to slot 0
and sifted down, which has the same observable effect as removing
the root and inserting a new element. No caller observes queue
state between the two operations. The previous implementation
shared sift_down_root() with get, but the cascade approach no
longer accommodates that cleanly since sift_down_root() now
expects the element to reinsert at queue->array[queue->nr], left
there by prio_queue_get() after decrementing nr. This is fine in
practice: replace is only called from pop_most_recent_commit()
(fetch-pack, object-name, walker) and show-branch — none of
which appear in any hot path.
A synthetic benchmark (10 rounds of 10M put+get cycles, ascending
integer keys, CPU-pinned, median of 3 runs, same compiler and
Makefile flags) shows consistent improvement across all queue
sizes, with no regressions:
queue width baseline cascade speedup
------------------------------------------------
10 4.32s 3.97s 1.09x
100 7.95s 6.49s 1.23x
1,000 11.30s 9.66s 1.17x
10,000 16.34s 14.15s 1.16x
100,000 21.43s 18.66s 1.15x
With descending keys (worst case — the last element always sinks
to a leaf in both approaches) the cascade still wins slightly
(1-4%) by replacing swaps with copies, and never regresses.
In end-to-end git commands the improvement is modest because
sift_down_root is only ~8% of total runtime. Profiling
rev-list --count on a 2.5M-commit monorepo shows sift_down_root
dropping from 8.2% to 0.4% of total runtime. The improvement
scales with DAG width: wider DAGs produce larger priority queues,
amplifying the per-level savings. In small or narrow repos the
queues stay shallow and the effect is negligible.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
prio-queue: use cascade-down sift for faster extract-min
Hi, I am not sure this is just noise or not but I thought it at least
was interesting.
I looked into the internals of prio_queue and found it was technically
doing too much work and could be simplified/optimized. I found I could
optimize it by ~20% for the common case (adding commits that would
typically end up far back in the queue) but only ~1% for the reverse
case (adding things to the front of the prio queue). The average speedup
is somewhere in between I suppose. That said, this is not really the
bottleneck so the overall boost seems to be around ~3-4% improvement for
repos with wide DAGs.
I would normally classify this as not urgent or important, but I think
the advantage is that the change is very small and simple and it already
has good unit tests (t/unit-tests/u-prio-queue.c).
With that said, here are the details:
The prio_queue_get impl is based on removing the root entry, then moving
the very last element into the root slot, then sifting it down into the
right place. This uses both comparisons between sibling elements in the
heap as well as comparisons between the element to add and one of the
siblings. Then it uses swap operations to move things correctly.
This patch instead promotes the smaller child upward at each level,
leaving a vacancy that sinks to a leaf, then places the removed element
there with a short sift-up to keep the heap balanced.
We can analytically compare this - for a sift-distance of d we can
reason about the number of operations to execute.
Before: 2d comparisons + 3d copies
After: d comparisons + d copies
After changing sift_down in this way, the replace operation can't simply
depend on it anymore, so I reimplemented it as a sequence of get + put.
This is technically correct but maybe not as efficient. However, I am
not sure that it matters, since I couldn't see any usage of the replace
operation in any hot path.
Performance: Profiling git rev-list --count on a 2.5M-commit monorepo
shows sift_down_root dropping from 8.2% to 0.4% of total runtime,
effectively eliminated as significant overhead.
Synthetic benchmark 10 rounds of 10M put+get cycles, CPU-pinned, median
of 3 runs, same compiler and Makefile flags.
Ascending keys (git's typical pattern -- parents have lower priority
than children):
queue width baseline patched speedup
10 4.32s 3.97s 1.09x
100 7.95s 6.49s 1.23x
1,000 11.30s 9.66s 1.17x
10,000 16.34s 14.15s 1.16x
100,000 21.43s 18.66s 1.15x
Descending keys (worst case — last element always sinks to leaf in both
approaches):
queue width baseline patched speedup
10 4.84s 4.78s 1.01x
100 9.43s 9.20s 1.03x
1,000 15.28s 14.71s 1.04x
10,000 23.61s 23.49s 1.01x
100,000 29.16s 28.22s 1.03x
No regressions in any scenario.
End-to-end benchmarks
All benchmarks use a benchmark setup of 1 warmup run followed by 10
timed runs. Each configuration is built from the same source tree and
tested on the same repo in alternating order.
linux kernel (1.4M commits) — range v5.0..v6.0 (311K commits):
Command baseline patched speedup
rev-list --count v5.0..v6.0 455ms 440ms 1.04x
I also ran it on git.git but did not see any performance diff at all,
due to the size and narrow DAG.
The improvement scales with DAG width: wider DAGs produce larger
priority queues, amplifying the per-level savings. In small or narrow
repositories the priority queues stay shallow and the sift-down cost is
already negligible, so the change is not noticeable.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2132%2Fspkrka%2Fcascade-sift-down-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2132/spkrka/cascade-sift-down-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2132
prio-queue.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/prio-queue.c b/prio-queue.c
index 9748528ce6..18005c43c4 100644
--- a/prio-queue.c
+++ b/prio-queue.c
@@ -62,17 +62,21 @@ static void sift_down_root(struct prio_queue *queue)
{
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 */
+ for (ix = 0; (child = ix * 2 + 1) < queue->nr; ix = child) {
if (child + 1 < queue->nr &&
compare(queue, child, child + 1) >= 0)
child++; /* use right child */
+ queue->array[ix] = queue->array[child];
+ }
- if (compare(queue, ix, child) <= 0)
+ /* Place queue->array[queue->nr] (left by caller) and sift up. */
+ queue->array[ix] = queue->array[queue->nr];
+ while (ix) {
+ size_t parent = (ix - 1) / 2;
+ if (compare(queue, parent, ix) <= 0)
break;
-
- swap(queue, child, ix);
+ swap(queue, parent, ix);
+ ix = parent;
}
}
@@ -89,7 +93,6 @@ void *prio_queue_get(struct prio_queue *queue)
if (!--queue->nr)
return result;
- queue->array[0] = queue->array[queue->nr];
sift_down_root(queue);
return result;
}
@@ -111,8 +114,7 @@ void prio_queue_replace(struct prio_queue *queue, void *thing)
queue->array[queue->nr - 1].ctr = queue->insertion_ctr++;
queue->array[queue->nr - 1].data = thing;
} else {
- queue->array[0].ctr = queue->insertion_ctr++;
- queue->array[0].data = thing;
- sift_down_root(queue);
+ prio_queue_get(queue);
+ prio_queue_put(queue, thing);
}
}
base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
--
gitgitgadget
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] prio-queue: use cascade-down sift for faster extract-min
2026-05-31 17:57 [PATCH] prio-queue: use cascade-down sift for faster extract-min Kristofer Karlsson via GitGitGadget
@ 2026-06-01 0:09 ` Junio C Hamano
2026-06-01 6:16 ` Junio C Hamano
2026-06-01 8:17 ` [PATCH v2] prio-queue: use cascade-down " Kristofer Karlsson via GitGitGadget
2 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2026-06-01 0:09 UTC (permalink / raw)
To: René Scharfe, Kristofer Karlsson via GitGitGadget
Cc: git, Kristofer Karlsson
"Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:
I'll add René the recipients, as _replace() was added by him as
optimization, so "this new one is functionally equivalent to the
original" somewhat misses the point, even though we may all agree
that the change is a very good one overall in the end when we look
at the entire picture.
> From: Kristofer Karlsson <krka@spotify.com>
>
> Replace the standard sift-down in prio_queue_get() with a
> cascade-down approach.
>
> The standard approach places the last array element at the root,
> then sifts it down. At each level this requires two comparisons
> (left vs right child, then element vs winner) and, when the
> element is larger, a swap (three 16-byte copies).
>
> The cascade approach instead promotes the smaller child into the
> vacant root slot at each level — one comparison and one copy.
> The vacancy sinks to a leaf, where the last array element is
> placed and sifted up if needed — typically zero levels since the
> last array element tends to be large.
>
> In the common case, work per extract drops from 2d comparisons
> + 3d copies to d comparisons + d copies: roughly half the
> comparisons and a third of the data movement. The sift-up phase
> can add work when the last element is smaller than ancestors of
> the leaf vacancy, but this is rare in practice.
>
> Simplify prio_queue_replace() to a plain get+put sequence. This
> is semantically equivalent: the old implementation wrote to slot 0
> and sifted down, which has the same observable effect as removing
> the root and inserting a new element. No caller observes queue
> state between the two operations. The previous implementation
> shared sift_down_root() with get, but the cascade approach no
> longer accommodates that cleanly since sift_down_root() now
> expects the element to reinsert at queue->array[queue->nr], left
> there by prio_queue_get() after decrementing nr. This is fine in
> practice: replace is only called from pop_most_recent_commit()
> (fetch-pack, object-name, walker) and show-branch — none of
> which appear in any hot path.
>
> A synthetic benchmark (10 rounds of 10M put+get cycles, ascending
> integer keys, CPU-pinned, median of 3 runs, same compiler and
> Makefile flags) shows consistent improvement across all queue
> sizes, with no regressions:
>
> queue width baseline cascade speedup
> ------------------------------------------------
> 10 4.32s 3.97s 1.09x
> 100 7.95s 6.49s 1.23x
> 1,000 11.30s 9.66s 1.17x
> 10,000 16.34s 14.15s 1.16x
> 100,000 21.43s 18.66s 1.15x
>
> With descending keys (worst case — the last element always sinks
> to a leaf in both approaches) the cascade still wins slightly
> (1-4%) by replacing swaps with copies, and never regresses.
>
> In end-to-end git commands the improvement is modest because
> sift_down_root is only ~8% of total runtime. Profiling
> rev-list --count on a 2.5M-commit monorepo shows sift_down_root
> dropping from 8.2% to 0.4% of total runtime. The improvement
> scales with DAG width: wider DAGs produce larger priority queues,
> amplifying the per-level savings. In small or narrow repos the
> queues stay shallow and the effect is negligible.
>
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---
> prio-queue: use cascade-down sift for faster extract-min
>
> Hi, I am not sure this is just noise or not but I thought it at least
> was interesting.
>
> I looked into the internals of prio_queue and found it was technically
> doing too much work and could be simplified/optimized. I found I could
> optimize it by ~20% for the common case (adding commits that would
> typically end up far back in the queue) but only ~1% for the reverse
> case (adding things to the front of the prio queue). The average speedup
> is somewhere in between I suppose. That said, this is not really the
> bottleneck so the overall boost seems to be around ~3-4% improvement for
> repos with wide DAGs.
>
> I would normally classify this as not urgent or important, but I think
> the advantage is that the change is very small and simple and it already
> has good unit tests (t/unit-tests/u-prio-queue.c).
>
> With that said, here are the details:
>
> The prio_queue_get impl is based on removing the root entry, then moving
> the very last element into the root slot, then sifting it down into the
> right place. This uses both comparisons between sibling elements in the
> heap as well as comparisons between the element to add and one of the
> siblings. Then it uses swap operations to move things correctly.
>
> This patch instead promotes the smaller child upward at each level,
> leaving a vacancy that sinks to a leaf, then places the removed element
> there with a short sift-up to keep the heap balanced.
>
> We can analytically compare this - for a sift-distance of d we can
> reason about the number of operations to execute.
>
> Before: 2d comparisons + 3d copies
> After: d comparisons + d copies
>
>
> After changing sift_down in this way, the replace operation can't simply
> depend on it anymore, so I reimplemented it as a sequence of get + put.
> This is technically correct but maybe not as efficient. However, I am
> not sure that it matters, since I couldn't see any usage of the replace
> operation in any hot path.
>
> Performance: Profiling git rev-list --count on a 2.5M-commit monorepo
> shows sift_down_root dropping from 8.2% to 0.4% of total runtime,
> effectively eliminated as significant overhead.
>
> Synthetic benchmark 10 rounds of 10M put+get cycles, CPU-pinned, median
> of 3 runs, same compiler and Makefile flags.
>
> Ascending keys (git's typical pattern -- parents have lower priority
> than children):
>
> queue width baseline patched speedup
> 10 4.32s 3.97s 1.09x
> 100 7.95s 6.49s 1.23x
> 1,000 11.30s 9.66s 1.17x
> 10,000 16.34s 14.15s 1.16x
> 100,000 21.43s 18.66s 1.15x
>
>
> Descending keys (worst case — last element always sinks to leaf in both
> approaches):
>
> queue width baseline patched speedup
> 10 4.84s 4.78s 1.01x
> 100 9.43s 9.20s 1.03x
> 1,000 15.28s 14.71s 1.04x
> 10,000 23.61s 23.49s 1.01x
> 100,000 29.16s 28.22s 1.03x
>
>
> No regressions in any scenario.
>
> End-to-end benchmarks
>
> All benchmarks use a benchmark setup of 1 warmup run followed by 10
> timed runs. Each configuration is built from the same source tree and
> tested on the same repo in alternating order.
>
> linux kernel (1.4M commits) — range v5.0..v6.0 (311K commits):
>
> Command baseline patched speedup
> rev-list --count v5.0..v6.0 455ms 440ms 1.04x
>
>
> I also ran it on git.git but did not see any performance diff at all,
> due to the size and narrow DAG.
>
> The improvement scales with DAG width: wider DAGs produce larger
> priority queues, amplifying the per-level savings. In small or narrow
> repositories the priority queues stay shallow and the sift-down cost is
> already negligible, so the change is not noticeable.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2132%2Fspkrka%2Fcascade-sift-down-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2132/spkrka/cascade-sift-down-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2132
>
> prio-queue.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/prio-queue.c b/prio-queue.c
> index 9748528ce6..18005c43c4 100644
> --- a/prio-queue.c
> +++ b/prio-queue.c
> @@ -62,17 +62,21 @@ static void sift_down_root(struct prio_queue *queue)
> {
> 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 */
> + for (ix = 0; (child = ix * 2 + 1) < queue->nr; ix = child) {
> if (child + 1 < queue->nr &&
> compare(queue, child, child + 1) >= 0)
> child++; /* use right child */
> + queue->array[ix] = queue->array[child];
> + }
>
> - if (compare(queue, ix, child) <= 0)
> + /* Place queue->array[queue->nr] (left by caller) and sift up. */
> + queue->array[ix] = queue->array[queue->nr];
> + while (ix) {
> + size_t parent = (ix - 1) / 2;
> + if (compare(queue, parent, ix) <= 0)
> break;
> -
> - swap(queue, child, ix);
> + swap(queue, parent, ix);
> + ix = parent;
> }
> }
>
> @@ -89,7 +93,6 @@ void *prio_queue_get(struct prio_queue *queue)
> if (!--queue->nr)
> return result;
>
> - queue->array[0] = queue->array[queue->nr];
> sift_down_root(queue);
> return result;
> }
> @@ -111,8 +114,7 @@ void prio_queue_replace(struct prio_queue *queue, void *thing)
> queue->array[queue->nr - 1].ctr = queue->insertion_ctr++;
> queue->array[queue->nr - 1].data = thing;
> } else {
> - queue->array[0].ctr = queue->insertion_ctr++;
> - queue->array[0].data = thing;
> - sift_down_root(queue);
> + prio_queue_get(queue);
> + prio_queue_put(queue, thing);
> }
> }
>
> base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] prio-queue: use cascade-down sift for faster extract-min
2026-05-31 17:57 [PATCH] prio-queue: use cascade-down sift for faster extract-min Kristofer Karlsson via GitGitGadget
2026-06-01 0:09 ` Junio C Hamano
@ 2026-06-01 6:16 ` Junio C Hamano
2026-06-01 6:21 ` Kristofer Karlsson
2026-06-01 8:17 ` [PATCH v2] prio-queue: use cascade-down " Kristofer Karlsson via GitGitGadget
2 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2026-06-01 6:16 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget; +Cc: git, Kristofer Karlsson
"Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> diff --git a/prio-queue.c b/prio-queue.c
> index 9748528ce6..18005c43c4 100644
> --- a/prio-queue.c
> +++ b/prio-queue.c
> @@ -62,17 +62,21 @@ static void sift_down_root(struct prio_queue *queue)
> {
> 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 */
> + for (ix = 0; (child = ix * 2 + 1) < queue->nr; ix = child) {
> if (child + 1 < queue->nr &&
> compare(queue, child, child + 1) >= 0)
> child++; /* use right child */
> + queue->array[ix] = queue->array[child];
> + }
>
> - if (compare(queue, ix, child) <= 0)
> + /* Place queue->array[queue->nr] (left by caller) and sift up. */
> + queue->array[ix] = queue->array[queue->nr];
Here we always sift/bubble up the last element.
I am wondering if it makes sense to teach sift_down_root to take an
extra argument, "struct prio_queue_entry entry" (passed by value)
and sift/bubble it up, not always queue->array[queue->nr], and ...
> + while (ix) {
> + size_t parent = (ix - 1) / 2;
> + if (compare(queue, parent, ix) <= 0)
> break;
> -
> - swap(queue, child, ix);
> + swap(queue, parent, ix);
> + ix = parent;
> }
> }
>
> @@ -89,7 +93,6 @@ void *prio_queue_get(struct prio_queue *queue)
> if (!--queue->nr)
> return result;
>
> - queue->array[0] = queue->array[queue->nr];
> sift_down_root(queue);
> return result;
> }
> @@ -111,8 +114,7 @@ void prio_queue_replace(struct prio_queue *queue, void *thing)
> queue->array[queue->nr - 1].ctr = queue->insertion_ctr++;
> queue->array[queue->nr - 1].data = thing;
> } else {
> - queue->array[0].ctr = queue->insertion_ctr++;
> - queue->array[0].data = thing;
> - sift_down_root(queue);
> + prio_queue_get(queue);
> + prio_queue_put(queue, thing);
... update this part in the else clause to do something like
struct prio_queue_entry entry;
entry.ctr = queue->insertion_ctr++;
entry.data = thing;
sift_down_root(queue, entry);
to retain the optimization? It would perform a single cascade-down
sift, followed by a single sift-up, so it would save a comparison, a
copy, and a swap in the worset case compared to the get+put sequence?
Of course, the original sift_down_root() caller (i.e. prio_queue_get())
needs to pass queue->array[queue->nr] as the second parameter to match.
> }
> }
>
> base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] prio-queue: use cascade-down sift for faster extract-min
2026-06-01 6:16 ` Junio C Hamano
@ 2026-06-01 6:21 ` Kristofer Karlsson
0 siblings, 0 replies; 24+ messages in thread
From: Kristofer Karlsson @ 2026-06-01 6:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristofer Karlsson via GitGitGadget, git
Thanks for the quick and very valid feedback! I already started
investigating - I think I was too quick (and wrong) when I reasoned
about the replace operation.I will rework it a bit and come back with
a patch version 2 soon that ensures that neither get and replace have
regressed in any way.
- Kristofer
On Mon, 1 Jun 2026 at 08:16, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > diff --git a/prio-queue.c b/prio-queue.c
> > index 9748528ce6..18005c43c4 100644
> > --- a/prio-queue.c
> > +++ b/prio-queue.c
> > @@ -62,17 +62,21 @@ static void sift_down_root(struct prio_queue *queue)
> > {
> > 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 */
> > + for (ix = 0; (child = ix * 2 + 1) < queue->nr; ix = child) {
> > if (child + 1 < queue->nr &&
> > compare(queue, child, child + 1) >= 0)
> > child++; /* use right child */
> > + queue->array[ix] = queue->array[child];
> > + }
> >
> > - if (compare(queue, ix, child) <= 0)
> > + /* Place queue->array[queue->nr] (left by caller) and sift up. */
> > + queue->array[ix] = queue->array[queue->nr];
>
> Here we always sift/bubble up the last element.
>
> I am wondering if it makes sense to teach sift_down_root to take an
> extra argument, "struct prio_queue_entry entry" (passed by value)
> and sift/bubble it up, not always queue->array[queue->nr], and ...
>
> > + while (ix) {
> > + size_t parent = (ix - 1) / 2;
> > + if (compare(queue, parent, ix) <= 0)
> > break;
> > -
> > - swap(queue, child, ix);
> > + swap(queue, parent, ix);
> > + ix = parent;
> > }
> > }
> >
> > @@ -89,7 +93,6 @@ void *prio_queue_get(struct prio_queue *queue)
> > if (!--queue->nr)
> > return result;
> >
> > - queue->array[0] = queue->array[queue->nr];
> > sift_down_root(queue);
> > return result;
> > }
> > @@ -111,8 +114,7 @@ void prio_queue_replace(struct prio_queue *queue, void *thing)
> > queue->array[queue->nr - 1].ctr = queue->insertion_ctr++;
> > queue->array[queue->nr - 1].data = thing;
> > } else {
> > - queue->array[0].ctr = queue->insertion_ctr++;
> > - queue->array[0].data = thing;
> > - sift_down_root(queue);
> > + prio_queue_get(queue);
> > + prio_queue_put(queue, thing);
>
> ... update this part in the else clause to do something like
>
> struct prio_queue_entry entry;
> entry.ctr = queue->insertion_ctr++;
> entry.data = thing;
> sift_down_root(queue, entry);
>
> to retain the optimization? It would perform a single cascade-down
> sift, followed by a single sift-up, so it would save a comparison, a
> copy, and a swap in the worset case compared to the get+put sequence?
>
> Of course, the original sift_down_root() caller (i.e. prio_queue_get())
> needs to pass queue->array[queue->nr] as the second parameter to match.
>
> > }
> > }
> >
> > base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-05-31 17:57 [PATCH] prio-queue: use cascade-down sift for faster extract-min Kristofer Karlsson via GitGitGadget
2026-06-01 0:09 ` Junio C Hamano
2026-06-01 6:16 ` Junio C Hamano
@ 2026-06-01 8:17 ` Kristofer Karlsson via GitGitGadget
2026-06-02 16:36 ` René Scharfe
2026-07-08 17:49 ` [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min Kristofer Karlsson via GitGitGadget
2 siblings, 2 replies; 24+ messages in thread
From: Kristofer Karlsson via GitGitGadget @ 2026-06-01 8:17 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson
From: Kristofer Karlsson <krka@spotify.com>
Add sift_up_rebalance(), an alternative to sift_down_root() that
halves the number of comparisons per extract-min.
The standard extract places the last array element at the root and
sifts it down. At each level this requires two comparisons (left
vs right child, then element vs winner) and a swap.
sift_up_rebalance() instead promotes the smaller child into the
root slot at each level — one comparison and one copy — until the
vacancy reaches a leaf. The last array element is placed at the
vacancy and sifted up to restore heap order. In practice the
sift-up rarely moves more than a level or two because the last
array element tends to be large.
Work per extract drops from 2d comparisons + d swaps to
d comparisons + d copies + a short sift-up.
prio_queue_get() now calls sift_up_rebalance() instead of placing
the last element at root and calling sift_down_root().
sift_down_root() and prio_queue_replace() are left unchanged.
Synthetic benchmark (10 rounds of 10M put+get cycles, CPU-pinned,
same compiler and Makefile flags):
Ascending keys (git's typical pattern — parents have lower
priority than children):
queue width baseline patched speedup
10 4.39s 3.91s 1.12x
100 9.10s 6.61s 1.38x
1,000 11.84s 9.25s 1.28x
10,000 17.50s 13.92s 1.26x
100,000 23.97s 20.19s 1.19x
Descending keys (worst case — last element always sinks to leaf):
queue width baseline patched speedup
10 4.94s 4.95s 1.00x
100 9.75s 9.42s 1.03x
1,000 15.01s 15.29s 0.98x
10,000 24.79s 23.88s 1.04x
100,000 29.69s 28.24s 1.05x
Random keys:
queue width baseline patched speedup
10 5.05s 4.99s 1.01x
100 9.90s 9.50s 1.04x
1,000 15.35s 14.77s 1.04x
10,000 25.35s 24.21s 1.05x
100,000 65.71s 63.38s 1.04x
No regressions in any scenario.
End-to-end benchmark on the linux kernel repo (1.4M commits,
range v5.0..v6.0, 311K commits, 20 interleaved runs, 1 warmup):
Command baseline patched speedup
rev-list --count v5.0..v6.0 484ms 474ms 1.02x
The improvement scales with DAG width: wider DAGs produce larger
priority queues, amplifying the per-level savings. In small or
narrow repositories the queues stay shallow and the sift-down
cost is already negligible.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
prio-queue: use cascade-down sift for faster extract-min
This is a small optimization to prio_queue_get() that reduces the number
of comparisons per extract-min from 2d to d (where d is the sift
distance).
The standard extract places the last array element at the root and sifts
it down, comparing against both children at each level. The new
sift_up_rebalance() instead promotes the smaller child at each level
(one comparison and one copy) leaving a vacancy that sinks to a leaf.
The last element is placed there and sifted up, which in practice rarely
moves more than a level or two.
The improvement shows clearly in synthetic benchmarks (up to 1.38x for
ascending keys at queue width 100) but is modest end-to-end since
sift_down_root is only a fraction of total runtime. On the linux kernel
repo, rev-list --count v5.0..v6.0 improves by ~2%. The effect scales
with DAG width.
Changes since v1:
* Kept sift_down_root() and prio_queue_replace() completely unchanged,
preserving René's optimization that avoids the get+put overhead for
replace. The cascade approach now only applies to prio_queue_get().
* Extracted the new logic into a separate sift_up_rebalance() function
rather than inlining it in prio_queue_get().
* Updated benchmark numbers for ascending, descending and random
insertion ordering. No regressions in any scenario.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2132%2Fspkrka%2Fcascade-sift-down-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2132/spkrka/cascade-sift-down-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2132
Range-diff vs v1:
1: 9ca2fab4dc ! 1: 6051d44e59 prio-queue: use cascade-down sift for faster extract-min
@@ Metadata
Author: Kristofer Karlsson <krka@spotify.com>
## Commit message ##
- prio-queue: use cascade-down sift for faster extract-min
-
- Replace the standard sift-down in prio_queue_get() with a
- cascade-down approach.
-
- The standard approach places the last array element at the root,
- then sifts it down. At each level this requires two comparisons
- (left vs right child, then element vs winner) and, when the
- element is larger, a swap (three 16-byte copies).
-
- The cascade approach instead promotes the smaller child into the
- vacant root slot at each level — one comparison and one copy.
- The vacancy sinks to a leaf, where the last array element is
- placed and sifted up if needed — typically zero levels since the
- last array element tends to be large.
-
- In the common case, work per extract drops from 2d comparisons
- + 3d copies to d comparisons + d copies: roughly half the
- comparisons and a third of the data movement. The sift-up phase
- can add work when the last element is smaller than ancestors of
- the leaf vacancy, but this is rare in practice.
-
- Simplify prio_queue_replace() to a plain get+put sequence. This
- is semantically equivalent: the old implementation wrote to slot 0
- and sifted down, which has the same observable effect as removing
- the root and inserting a new element. No caller observes queue
- state between the two operations. The previous implementation
- shared sift_down_root() with get, but the cascade approach no
- longer accommodates that cleanly since sift_down_root() now
- expects the element to reinsert at queue->array[queue->nr], left
- there by prio_queue_get() after decrementing nr. This is fine in
- practice: replace is only called from pop_most_recent_commit()
- (fetch-pack, object-name, walker) and show-branch — none of
- which appear in any hot path.
-
- A synthetic benchmark (10 rounds of 10M put+get cycles, ascending
- integer keys, CPU-pinned, median of 3 runs, same compiler and
- Makefile flags) shows consistent improvement across all queue
- sizes, with no regressions:
-
- queue width baseline cascade speedup
- ------------------------------------------------
- 10 4.32s 3.97s 1.09x
- 100 7.95s 6.49s 1.23x
- 1,000 11.30s 9.66s 1.17x
- 10,000 16.34s 14.15s 1.16x
- 100,000 21.43s 18.66s 1.15x
-
- With descending keys (worst case — the last element always sinks
- to a leaf in both approaches) the cascade still wins slightly
- (1-4%) by replacing swaps with copies, and never regresses.
-
- In end-to-end git commands the improvement is modest because
- sift_down_root is only ~8% of total runtime. Profiling
- rev-list --count on a 2.5M-commit monorepo shows sift_down_root
- dropping from 8.2% to 0.4% of total runtime. The improvement
- scales with DAG width: wider DAGs produce larger priority queues,
- amplifying the per-level savings. In small or narrow repos the
- queues stay shallow and the effect is negligible.
+ prio-queue: use cascade-down for faster extract-min
+
+ Add sift_up_rebalance(), an alternative to sift_down_root() that
+ halves the number of comparisons per extract-min.
+
+ The standard extract places the last array element at the root and
+ sifts it down. At each level this requires two comparisons (left
+ vs right child, then element vs winner) and a swap.
+
+ sift_up_rebalance() instead promotes the smaller child into the
+ root slot at each level — one comparison and one copy — until the
+ vacancy reaches a leaf. The last array element is placed at the
+ vacancy and sifted up to restore heap order. In practice the
+ sift-up rarely moves more than a level or two because the last
+ array element tends to be large.
+
+ Work per extract drops from 2d comparisons + d swaps to
+ d comparisons + d copies + a short sift-up.
+
+ prio_queue_get() now calls sift_up_rebalance() instead of placing
+ the last element at root and calling sift_down_root().
+
+ sift_down_root() and prio_queue_replace() are left unchanged.
+
+ Synthetic benchmark (10 rounds of 10M put+get cycles, CPU-pinned,
+ same compiler and Makefile flags):
+
+ Ascending keys (git's typical pattern — parents have lower
+ priority than children):
+
+ queue width baseline patched speedup
+ 10 4.39s 3.91s 1.12x
+ 100 9.10s 6.61s 1.38x
+ 1,000 11.84s 9.25s 1.28x
+ 10,000 17.50s 13.92s 1.26x
+ 100,000 23.97s 20.19s 1.19x
+
+ Descending keys (worst case — last element always sinks to leaf):
+
+ queue width baseline patched speedup
+ 10 4.94s 4.95s 1.00x
+ 100 9.75s 9.42s 1.03x
+ 1,000 15.01s 15.29s 0.98x
+ 10,000 24.79s 23.88s 1.04x
+ 100,000 29.69s 28.24s 1.05x
+
+ Random keys:
+
+ queue width baseline patched speedup
+ 10 5.05s 4.99s 1.01x
+ 100 9.90s 9.50s 1.04x
+ 1,000 15.35s 14.77s 1.04x
+ 10,000 25.35s 24.21s 1.05x
+ 100,000 65.71s 63.38s 1.04x
+
+ No regressions in any scenario.
+
+ End-to-end benchmark on the linux kernel repo (1.4M commits,
+ range v5.0..v6.0, 311K commits, 20 interleaved runs, 1 warmup):
+
+ Command baseline patched speedup
+ rev-list --count v5.0..v6.0 484ms 474ms 1.02x
+
+ The improvement scales with DAG width: wider DAGs produce larger
+ priority queues, amplifying the per-level savings. In small or
+ narrow repositories the queues stay shallow and the sift-down
+ cost is already negligible.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
## prio-queue.c ##
@@ prio-queue.c: static void sift_down_root(struct prio_queue *queue)
- {
- 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 */
++static void sift_up_rebalance(struct prio_queue *queue)
++{
++ size_t ix, child;
++
++ /* Cascade: promote smaller child at each level. */
+ for (ix = 0; (child = ix * 2 + 1) < queue->nr; ix = child) {
- if (child + 1 < queue->nr &&
- compare(queue, child, child + 1) >= 0)
- child++; /* use right child */
++ if (child + 1 < queue->nr &&
++ compare(queue, child, child + 1) >= 0)
++ child++;
+ queue->array[ix] = queue->array[child];
+ }
-
-- if (compare(queue, ix, child) <= 0)
-+ /* Place queue->array[queue->nr] (left by caller) and sift up. */
++
++ /* Place the last element at the vacancy and sift up. */
+ queue->array[ix] = queue->array[queue->nr];
+ while (ix) {
+ size_t parent = (ix - 1) / 2;
+ if (compare(queue, parent, ix) <= 0)
- break;
--
-- swap(queue, child, ix);
++ break;
+ swap(queue, parent, ix);
+ ix = parent;
- }
- }
-
++ }
++}
++
+ void *prio_queue_get(struct prio_queue *queue)
+ {
+ void *result;
@@ prio-queue.c: void *prio_queue_get(struct prio_queue *queue)
if (!--queue->nr)
return result;
- queue->array[0] = queue->array[queue->nr];
- sift_down_root(queue);
+- sift_down_root(queue);
++ sift_up_rebalance(queue);
return result;
}
-@@ prio-queue.c: void prio_queue_replace(struct prio_queue *queue, void *thing)
- queue->array[queue->nr - 1].ctr = queue->insertion_ctr++;
- queue->array[queue->nr - 1].data = thing;
- } else {
-- queue->array[0].ctr = queue->insertion_ctr++;
-- queue->array[0].data = thing;
-- sift_down_root(queue);
-+ prio_queue_get(queue);
-+ prio_queue_put(queue, thing);
- }
- }
+
prio-queue.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/prio-queue.c b/prio-queue.c
index 9748528ce6..66d445b078 100644
--- a/prio-queue.c
+++ b/prio-queue.c
@@ -76,6 +76,29 @@ static void sift_down_root(struct prio_queue *queue)
}
}
+static void sift_up_rebalance(struct prio_queue *queue)
+{
+ size_t ix, child;
+
+ /* Cascade: promote smaller child at each level. */
+ for (ix = 0; (child = ix * 2 + 1) < queue->nr; ix = child) {
+ if (child + 1 < queue->nr &&
+ compare(queue, child, child + 1) >= 0)
+ child++;
+ queue->array[ix] = queue->array[child];
+ }
+
+ /* Place the last element at the vacancy and sift up. */
+ queue->array[ix] = queue->array[queue->nr];
+ while (ix) {
+ size_t parent = (ix - 1) / 2;
+ if (compare(queue, parent, ix) <= 0)
+ break;
+ swap(queue, parent, ix);
+ ix = parent;
+ }
+}
+
void *prio_queue_get(struct prio_queue *queue)
{
void *result;
@@ -89,8 +112,7 @@ void *prio_queue_get(struct prio_queue *queue)
if (!--queue->nr)
return result;
- queue->array[0] = queue->array[queue->nr];
- sift_down_root(queue);
+ sift_up_rebalance(queue);
return result;
}
base-commit: 1666c1265231b0bc5f613fbbf3f0a9896cdef76e
--
gitgitgadget
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-06-01 8:17 ` [PATCH v2] prio-queue: use cascade-down " Kristofer Karlsson via GitGitGadget
@ 2026-06-02 16:36 ` René Scharfe
2026-06-02 22:40 ` Kristofer Karlsson
2026-07-08 17:49 ` [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min Kristofer Karlsson via GitGitGadget
1 sibling, 1 reply; 24+ messages in thread
From: René Scharfe @ 2026-06-02 16:36 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget, git; +Cc: Kristofer Karlsson
On 6/1/26 10:17 AM, Kristofer Karlsson via GitGitGadget wrote:
>
> Changes since v1:
>
> * Kept sift_down_root() and prio_queue_replace() completely unchanged,
> preserving René's optimization that avoids the get+put overhead for
> replace. The cascade approach now only applies to prio_queue_get().
The prospect of no longer needing prio_queue_replace() had me excited in
round 1. The benchmarks from commits that added its callers [1][2][3]
did show performance regressions with your patch 1 plus changes to
revert prio_queue_peek()+prio_queue_replace() to prio_queue_get()+
prio_queue_put(), but for two of them low enough to be in the noise.
'git describe $(git rev-list v2.41.0..v2.47.0)' took a 50%+ hit, though.
[1] a79e3519d6 (commit: use prio_queue_replace() in pop_most_recent_commit(), 2025-07-18)
[2] 08bb69d70f (describe: use prio_queue_replace(), 2025-08-03)
[3] abf05d856f (show-branch: use prio_queue, 2025-12-26)
> * Extracted the new logic into a separate sift_up_rebalance() function
> rather than inlining it in prio_queue_get().
>
> * Updated benchmark numbers for ascending, descending and random
> insertion ordering. No regressions in any scenario.
I don't see any regression for the benchmarks mentioned above with
patch 2 alone, unsurprisingly. The describe command still takes that
50%+ performance hit after reverting [2] on top.
Would you be interested in benchmarking the following patch for making
prio_queue_replace() unnecessary by doing its optimization
automatically? I get a 1% performance hit for the describe command
that I can't explain. And it leaves the heap unbalanced after a
prio_queue_get(), which complicates things, so I found it lacking.
But I wonder how it stacks up against your cascade approach for your
use case and if there's anything to salvage.
René
---
prio-queue.c | 60 +++++++++++++++++++++++++++++++++++++++++-------------------
prio-queue.h | 1 +
2 files changed, 42 insertions(+), 19 deletions(-)
diff --git a/prio-queue.c b/prio-queue.c
index 9748528ce6..ba6b460a46 100644
--- a/prio-queue.c
+++ b/prio-queue.c
@@ -34,12 +34,46 @@ void clear_prio_queue(struct prio_queue *queue)
queue->nr = 0;
queue->alloc = 0;
queue->insertion_ctr = 0;
+ queue->sift_down_root_pending = false;
+}
+
+static void sift_down_root(struct prio_queue *queue)
+{
+ 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 (compare(queue, ix, child) <= 0)
+ break;
+
+ swap(queue, child, ix);
+ }
+ queue->sift_down_root_pending = false;
}
void prio_queue_put(struct prio_queue *queue, void *thing)
{
size_t ix, parent;
+ if (queue->sift_down_root_pending) {
+ /*
+ * Restore the original heap size. The last item is
+ * still in the right place.
+ */
+ queue->nr++;
+
+ /* Now fill the hole at the root with the new item. */
+ queue->array[0].ctr = queue->insertion_ctr++;
+ queue->array[0].data = thing;
+ sift_down_root(queue);
+ return;
+ }
+
/* Append at the end */
ALLOC_GROW(queue->array, queue->nr + 1, queue->alloc);
queue->array[queue->nr].ctr = queue->insertion_ctr++;
@@ -58,24 +92,6 @@ void prio_queue_put(struct prio_queue *queue, void *thing)
}
}
-static void sift_down_root(struct prio_queue *queue)
-{
- 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 (compare(queue, ix, child) <= 0)
- break;
-
- swap(queue, child, ix);
- }
-}
-
void *prio_queue_get(struct prio_queue *queue)
{
void *result;
@@ -85,12 +101,14 @@ void *prio_queue_get(struct prio_queue *queue)
if (!queue->compare)
return queue->array[--queue->nr].data; /* LIFO */
+ if (queue->sift_down_root_pending)
+ sift_down_root(queue);
result = queue->array[0].data;
if (!--queue->nr)
return result;
queue->array[0] = queue->array[queue->nr];
- sift_down_root(queue);
+ queue->sift_down_root_pending = true;
return result;
}
@@ -100,6 +118,8 @@ void *prio_queue_peek(struct prio_queue *queue)
return NULL;
if (!queue->compare)
return queue->array[queue->nr - 1].data;
+ if (queue->sift_down_root_pending)
+ sift_down_root(queue);
return queue->array[0].data;
}
@@ -111,6 +131,8 @@ void prio_queue_replace(struct prio_queue *queue, void *thing)
queue->array[queue->nr - 1].ctr = queue->insertion_ctr++;
queue->array[queue->nr - 1].data = thing;
} else {
+ if (queue->sift_down_root_pending)
+ sift_down_root(queue);
queue->array[0].ctr = queue->insertion_ctr++;
queue->array[0].data = thing;
sift_down_root(queue);
diff --git a/prio-queue.h b/prio-queue.h
index da7fad2f1f..5977fba438 100644
--- a/prio-queue.h
+++ b/prio-queue.h
@@ -32,6 +32,7 @@ struct prio_queue {
void *cb_data;
size_t alloc, nr;
struct prio_queue_entry *array;
+ bool sift_down_root_pending;
};
/*
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-06-02 16:36 ` René Scharfe
@ 2026-06-02 22:40 ` Kristofer Karlsson
2026-06-05 20:39 ` Kristofer Karlsson
0 siblings, 1 reply; 24+ messages in thread
From: Kristofer Karlsson @ 2026-06-02 22:40 UTC (permalink / raw)
To: René Scharfe; +Cc: Kristofer Karlsson via GitGitGadget, git
On Tue, 2 Jun 2026 at 18:37, René Scharfe <l.s.r@web.de> wrote:
>
> Would you be interested in benchmarking the following patch for making
> prio_queue_replace() unnecessary by doing its optimization
> automatically? I get a 1% performance hit for the describe command
> that I can't explain. And it leaves the heap unbalanced after a
> prio_queue_get(), which complicates things, so I found it lacking.
> But I wonder how it stacks up against your cascade approach for your
> use case and if there's anything to salvage.
>
> René
Thank you for the detailed feedback and the patch! It was very
helpful to have a concrete alternative to compare against.
I spent some time benchmarking the different approaches on a
large monorepo with a wide DAG.
All measurements include the nonstale O(1) tracking from my other
series as a common base, since that dominates the merge-base path.
The approaches I compared:
1. cascade-only: the sift_up_rebalance from this patch (v2)
2. rene-lazy: your deferred sift_down_root patch
3. cascade+lazy: cascade for unfused gets, lazy fusion for
get+put pairs
Results (10 runs, 1 warmup, CPU pinned to performance):
merge-base --all master master~1000 (~4s workload):
cascade-only 4.18s (median)
rene-lazy 4.25s
cascade+lazy 4.24s
rev-list --count master~1000..master (~3.8s workload):
cascade-only 3.86s
rene-lazy 3.75s
cascade+lazy 3.74s
The lazy approaches show a small win on rev-list (~3%) where get+put
pairs are common in limit_list. On merge-base --all, everything is
within noise, the prio_queue is a small fraction of total runtime
there. Combining cascade with lazy fusion didn't produce additional
gains beyond what each gives individually.
Looking at your patch, I think the deferred sift-down logic is
essentially the same optimization as the lazy_queue wrapper you
wrote for describe.c - both defer the work from get and fuse it
with a following put. So I'd be hesitant to add a second form of
that deferral directly into prio_queue when lazy_queue already
"owns" that responsibility as a wrapper.
That said, I think it would make sense to fold lazy_queue entirely
into prio_queue. It's an optimization that never hurts as far as I can
tell, and it would simplify several callers. pop_most_recent_commit
and show-branch both independently re-implement the same
peek+replace pattern that lazy_queue formalizes. Making it automatic
in prio_queue would clean up all of them.
I have a local branch exploring that direction. Maybe it makes more
sense to do the lazy_queue fold first, and then see if the cascade
change is still worth adding on top?
Either way, I think the two directions are complementary - cascade
reduces comparisons per sift, while lazy fusion can eliminate full
rebalance cycles.
I'm on a company offsite now so I may be slow to answer, but I will
definitely resume this when I get back home.
- Kristofer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-06-02 22:40 ` Kristofer Karlsson
@ 2026-06-05 20:39 ` Kristofer Karlsson
2026-06-07 7:30 ` René Scharfe
0 siblings, 1 reply; 24+ messages in thread
From: Kristofer Karlsson @ 2026-06-05 20:39 UTC (permalink / raw)
To: René Scharfe; +Cc: Kristofer Karlsson via GitGitGadget, git
I did some more benchmarking to understand how these approaches
interact, with four variants based on origin/next on my large monorepo:
1. base: next as-is
2. cascade: base + sift_up_rebalance from this patch (v2)
3. lazy-fold: base + lazy get fusion folded into prio_queue
4. cascade+lazy: both combined
Note that alt 3 is not yet shared with the mailing list so it's hard for you
to reason about it, though it's quite straightforward. I will submit a new
patch for that one soon, not necessarily with the primary goal to merge it,
but rather show how it is implemented.
merge-base --all master master~1000:
base 4.27s
cascade 4.07s (1.05x)
lazy-fold 4.12s (1.03x)
cascade+lazy 4.01s (1.06x)
rev-list --count master~1000..master:
base 3.60s
cascade 3.35s (1.08x)
lazy-fold 3.37s (1.07x)
cascade+lazy 3.30s (1.09x)
So both optimizations are valuable both on their own, and when combined,
which I think helps to reason about it. This cascading sift seems to have a
larger effect, but folding lazy_queue into prio_queue also speeds up other
use cases and simplifies the code a bit.
Based on this, my (very subjective) approach would be:
1. Land this cascade patch first since it's a pure algorithmic improvement,
2. Follow up with a separate patch that folds lazy_queue into
prio_queue. Will post it separately soon, as I mentioned.
- Kristofer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-06-05 20:39 ` Kristofer Karlsson
@ 2026-06-07 7:30 ` René Scharfe
2026-06-07 12:07 ` Kristofer Karlsson
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: René Scharfe @ 2026-06-07 7:30 UTC (permalink / raw)
To: Kristofer Karlsson; +Cc: Kristofer Karlsson via GitGitGadget, git
On 6/5/26 10:39 PM, Kristofer Karlsson wrote:
> I did some more benchmarking to understand how these approaches
> interact, with four variants based on origin/next on my large monorepo:
>
> 1. base: next as-is
> 2. cascade: base + sift_up_rebalance from this patch (v2)
> 3. lazy-fold: base + lazy get fusion folded into prio_queue
> 4. cascade+lazy: both combined
>
> Note that alt 3 is not yet shared with the mailing list so it's hard for you
> to reason about it, though it's quite straightforward. I will submit a new
> patch for that one soon, not necessarily with the primary goal to merge it,
> but rather show how it is implemented.
>
> merge-base --all master master~1000:
> base 4.27s
> cascade 4.07s (1.05x)
> lazy-fold 4.12s (1.03x)
> cascade+lazy 4.01s (1.06x)
>
> rev-list --count master~1000..master:
> base 3.60s
> cascade 3.35s (1.08x)
> lazy-fold 3.37s (1.07x)
> cascade+lazy 3.30s (1.09x)
>
> So both optimizations are valuable both on their own, and when combined,
> which I think helps to reason about it. This cascading sift seems to have a
> larger effect, but folding lazy_queue into prio_queue also speeds up other
> use cases and simplifies the code a bit.
Right. I was wondering, though: Why is sift-down so much faster than
cascade in the describe benchmark from 30598ccc4d (describe: use oidset
in finish_depth_computation(), 2025-09-02)?
I think I mostly understand it now: cascade is better in prio_queue_get()
because the sift-down item is from the bottom and will likely end up back
at the bottom, just of a different branch of the heap. Thus a sift-down
costs 3 compares times the number of levels, while a cascade costs just
2 compares times the number of levels and there is likely little to no
need to sift it back up.
For prio_queue_replace() we sift down a random item, though; we don't
know where it will end up. If it belongs at the very top then sift-down
just needs 3 compares, while cascade needs 2 compares times the number
of levels to bring the hole down and the same to bring the item up.
Below is a diff on top of your second cascade patch to use sift-down
only for the root and cascade otherwise. It comes remarkably close to
the performance of a full sift-down. I don't know how to find the
optimal number of levels to try sift-down before switching to cascade
for a given random item, though.
So I guess we keep the full sift-down for prio_queue_replace(), knowing
that sometimes we have a lot of items that end up at or close to the
root of the heap.
Benchmark 1: ./git_main describe $(git rev-list v2.41.0..v2.47.0)
Time (mean ± σ): 602.4 ms ± 1.2 ms [User: 539.2 ms, System: 47.7 ms]
Range (min … max): 600.5 ms … 604.7 ms 10 runs
Benchmark 2: ./git_cascade1 describe $(git rev-list v2.41.0..v2.47.0)
Time (mean ± σ): 993.9 ms ± 1.7 ms [User: 930.2 ms, System: 48.2 ms]
Range (min … max): 991.1 ms … 996.6 ms 10 runs
Benchmark 3: ./git_cascade2 describe $(git rev-list v2.41.0..v2.47.0)
Time (mean ± σ): 602.4 ms ± 1.7 ms [User: 539.1 ms, System: 47.6 ms]
Range (min … max): 599.9 ms … 606.2 ms 10 runs
Benchmark 4: ./git describe $(git rev-list v2.41.0..v2.47.0)
Time (mean ± σ): 625.4 ms ± 1.7 ms [User: 561.8 ms, System: 48.0 ms]
Range (min … max): 623.4 ms … 627.9 ms 10 runs
Summary
./git_main describe $(git rev-list v2.41.0..v2.47.0) ran
1.00 ± 0.00 times faster than ./git_cascade2 describe $(git rev-list v2.41.0..v2.47.0)
1.04 ± 0.00 times faster than ./git describe $(git rev-list v2.41.0..v2.47.0)
1.65 ± 0.00 times faster than ./git_cascade1 describe $(git rev-list v2.41.0..v2.47.0)
git_main and git_cascade2 (your v2): sift-down
git_cascade1 (your v1): cascade
git (your v2 and the patch below): sift-down for root then cascade
René
diff --git a/prio-queue.c b/prio-queue.c
index 66d445b078..4d7debc2ba 100644
--- a/prio-queue.c
+++ b/prio-queue.c
@@ -58,30 +58,12 @@ void prio_queue_put(struct prio_queue *queue, void *thing)
}
}
-static void sift_down_root(struct prio_queue *queue)
+static void sift_up_rebalance(struct prio_queue *queue, size_t ix)
{
- 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 (compare(queue, ix, child) <= 0)
- break;
-
- swap(queue, child, ix);
- }
-}
-
-static void sift_up_rebalance(struct prio_queue *queue)
-{
- size_t ix, child;
+ size_t child;
/* Cascade: promote smaller child at each level. */
- for (ix = 0; (child = ix * 2 + 1) < queue->nr; ix = child) {
+ for (; (child = ix * 2 + 1) < queue->nr; ix = child) {
if (child + 1 < queue->nr &&
compare(queue, child, child + 1) >= 0)
child++;
@@ -112,7 +94,7 @@ void *prio_queue_get(struct prio_queue *queue)
if (!--queue->nr)
return result;
- sift_up_rebalance(queue);
+ sift_up_rebalance(queue, 0);
return result;
}
@@ -132,9 +114,20 @@ void prio_queue_replace(struct prio_queue *queue, void *thing)
} else if (!queue->compare) {
queue->array[queue->nr - 1].ctr = queue->insertion_ctr++;
queue->array[queue->nr - 1].data = thing;
+ } else if (queue->nr < 3) {
+ prio_queue_get(queue);
+ prio_queue_put(queue, thing);
} else {
- queue->array[0].ctr = queue->insertion_ctr++;
+ size_t child = compare(queue, 1, 2) <= 0 ? 1 : 2;
+ queue->array[0].ctr = queue->insertion_ctr;
queue->array[0].data = thing;
- sift_down_root(queue);
+ if (compare(queue, 0, child) <= 0) {
+ queue->insertion_ctr++;
+ } else {
+ queue->array[0] = queue->array[child];
+ queue->nr--;
+ sift_up_rebalance(queue, child);
+ prio_queue_put(queue, thing);
+ }
}
}
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-06-07 7:30 ` René Scharfe
@ 2026-06-07 12:07 ` Kristofer Karlsson
2026-06-29 21:16 ` Junio C Hamano
2026-06-08 11:56 ` Junio C Hamano
2026-07-06 21:52 ` Kristofer Karlsson
2 siblings, 1 reply; 24+ messages in thread
From: Kristofer Karlsson @ 2026-06-07 12:07 UTC (permalink / raw)
To: René Scharfe; +Cc: Kristofer Karlsson via GitGitGadget, git
On Sat, 7 Jun 2026 at 09:30, Rene Scharfe <l.s.r@web.de> wrote:
>
> Right. I was wondering, though: Why is sift-down so much faster than
> cascade in the describe benchmark from 30598ccc4d (describe: use oidset
> in finish_depth_computation(), 2025-09-02)?
>
> I think I mostly understand it now: cascade is better in prio_queue_get()
> because the sift-down item is from the bottom and will likely end up back
> at the bottom, just of a different branch of the heap. Thus a sift-down
> costs 3 compares times the number of levels, while a cascade costs just
> 2 compares times the number of levels and there is likely little to no
> need to sift it back up.
>
> For prio_queue_replace() we sift down a random item, though; we don't
> know where it will end up. If it belongs at the very top then sift-down
> just needs 3 compares, while cascade needs 2 compares times the number
> of levels to bring the hole down and the same to bring the item up.
Yes, I think that reasoning is correct. It depends on where the item
will land.
For get() the last array element came from the bottom of the heap
and will almost certainly end up back near the bottom, so
cascade's blind descent is a good default.
For replace() the new element is arbitrary -- in describe's pattern
it often belongs near the root, so sift-down's early exit after
2-3 compares dominates.
Your benchmark confirms this: v2 (cascade only in get) matches the
baseline exactly for describe, while the hybrid is 4% slower.
> So I guess we keep the full sift-down for prio_queue_replace(), knowing
> that sometimes we have a lot of items that end up at or close to the
> root of the heap.
Agreed. And with the lazy-fold series (v3 just sent), replace() is
removed as a public API entirely. But the same principle applies to
the fused replace path inside prio_queue_put(): when get_pending is
set and a put arrives, we write the new element at the root and
sift it down -- that path should keep sift-down for the same reason
your analysis shows.
If (that's a big if) both my patches eventually land,
the split would be:
- unfused get-flush (in get() and peek()): use cascade
- fused replace (in put()): keep sift-down
Which is exactly the split your analysis predicts is optimal.
Now I am thinking it would be easier to reason about this if the other
patch lands first, since the cascade change becomes simpler to evaluate
when replace is already gone and only the unfused paths remain.
- Kristofer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-06-07 7:30 ` René Scharfe
2026-06-07 12:07 ` Kristofer Karlsson
@ 2026-06-08 11:56 ` Junio C Hamano
2026-07-06 21:52 ` Kristofer Karlsson
2 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2026-06-08 11:56 UTC (permalink / raw)
To: René Scharfe
Cc: Kristofer Karlsson, Kristofer Karlsson via GitGitGadget, git
René Scharfe <l.s.r@web.de> writes:
> I think I mostly understand it now: cascade is better in prio_queue_get()
> because the sift-down item is from the bottom and will likely end up back
> at the bottom, just of a different branch of the heap. Thus a sift-down
> costs 3 compares times the number of levels, while a cascade costs just
> 2 compares times the number of levels and there is likely little to no
> need to sift it back up.
>
> For prio_queue_replace() we sift down a random item, though; we don't
> know where it will end up. If it belongs at the very top then sift-down
> just needs 3 compares, while cascade needs 2 compares times the number
> of levels to bring the hole down and the same to bring the item up.
An excellent observation, showing clear and analytic mind. This is
one of the reasons why I love reading review messages from you (and
also explanation in the proposed commit log messages in your
patches).
Thanks.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-06-07 12:07 ` Kristofer Karlsson
@ 2026-06-29 21:16 ` Junio C Hamano
0 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2026-06-29 21:16 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: René Scharfe, Kristofer Karlsson via GitGitGadget, git
Kristofer Karlsson <krka@spotify.com> writes:
> Now I am thinking it would be easier to reason about this if the other
> patch lands first, since the cascade change becomes simpler to evaluate
> when replace is already gone and only the unfused paths remain.
Sorry, I should have noticed this message and responded earlier.
Let's make sure that the "other patch" is ready then and merge it
down.
Since June 8th, nothing seemed to have happened to the thread for
the "other patch".
https://lore.kernel.org/git/pull.2140.v4.git.1780945851.gitgitgadget@gmail.com/
Is everybody happy with these two patches?
Thanks.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-06-07 7:30 ` René Scharfe
2026-06-07 12:07 ` Kristofer Karlsson
2026-06-08 11:56 ` Junio C Hamano
@ 2026-07-06 21:52 ` Kristofer Karlsson
2026-07-08 10:43 ` René Scharfe
2 siblings, 1 reply; 24+ messages in thread
From: Kristofer Karlsson @ 2026-07-06 21:52 UTC (permalink / raw)
To: René Scharfe, Junio C Hamano
Cc: Kristofer Karlsson via GitGitGadget, git
On Sun, 7 Jun 2026 at 09:30, René Scharfe <l.s.r@web.de> wrote:
>
> So I guess we keep the full sift-down for prio_queue_replace(), knowing
> that sometimes we have a lot of items that end up at or close to the
> root of the heap.
The lazy-fold series (kk/prio-queue-get-put-fusion) is in next now.
I rebased this cascade patch on top of it to check if it's still
useful.
With lazy-fold in place the regression scenario you identified
is resolved. The only remaining change is in flush_get(),
where unfused gets now cascade instead of sifting down:
- queue->array[0] = queue->array[--queue->nr_];
- sift_down_root(queue);
+ --queue->nr_;
+ sift_up_rebalance(queue);
plus the ~20-line sift_up_rebalance() implementation.
I benchmarked this on the linux kernel repo and on a large
merge-heavy repo.
The results are consistent: a real but small 1-2% end-to-end
improvement across commands. A prio-queue microbenchmark
would likely show a larger difference, but the queue
is only a fraction of the total work in any real git operation.
The lazy-fold optimization cannibalized some of the value here,
so cascade only helps the remaining unfused gets. As you observed,
cascade is better there, but there are fewer of them now that there
is more fusing happening.
I am on the fence about whether 1-2% end-to-end justifies adding
another sift function. If you (René and Junio) think the benefit
is too small for the code cost, I am happy to drop this patch.
Otherwise I can submit a small reroll on top of
kk/prio-queue-get-put-fusion (or rather next, in practice).
Thanks,
Kristofer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-07-06 21:52 ` Kristofer Karlsson
@ 2026-07-08 10:43 ` René Scharfe
2026-07-08 10:59 ` Kristofer Karlsson
0 siblings, 1 reply; 24+ messages in thread
From: René Scharfe @ 2026-07-08 10:43 UTC (permalink / raw)
To: Kristofer Karlsson, Junio C Hamano
Cc: Kristofer Karlsson via GitGitGadget, git
On 7/6/26 11:52 PM, Kristofer Karlsson wrote:
> On Sun, 7 Jun 2026 at 09:30, René Scharfe <l.s.r@web.de> wrote:
>>
>> So I guess we keep the full sift-down for prio_queue_replace(), knowing
>> that sometimes we have a lot of items that end up at or close to the
>> root of the heap.
>
> The lazy-fold series (kk/prio-queue-get-put-fusion) is in next now.
> I rebased this cascade patch on top of it to check if it's still
> useful.
>
> With lazy-fold in place the regression scenario you identified
> is resolved. The only remaining change is in flush_get(),
> where unfused gets now cascade instead of sifting down:
>
> - queue->array[0] = queue->array[--queue->nr_];
> - sift_down_root(queue);
> + --queue->nr_;
> + sift_up_rebalance(queue);
>
> plus the ~20-line sift_up_rebalance() implementation.
>
> I benchmarked this on the linux kernel repo and on a large
> merge-heavy repo.
>
> The results are consistent: a real but small 1-2% end-to-end
> improvement across commands. A prio-queue microbenchmark
> would likely show a larger difference, but the queue
> is only a fraction of the total work in any real git operation.
>
> The lazy-fold optimization cannibalized some of the value here,
> so cascade only helps the remaining unfused gets. As you observed,
> cascade is better there, but there are fewer of them now that there
> is more fusing happening.
>
> I am on the fence about whether 1-2% end-to-end justifies adding
> another sift function. If you (René and Junio) think the benefit
> is too small for the code cost, I am happy to drop this patch.
> Otherwise I can submit a small reroll on top of
> kk/prio-queue-get-put-fusion (or rather next, in practice).
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?
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
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.
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;
}
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.
René
$ for n in $(seq 2 2)
do
t/helper/test-tool prio-queue permute get $n |
awk -v n=$n -v max=0 '
{sum+=$2}
max < $2 {max=$2}
!min || min > $2 {min=$2}
END {printf "%2d %3d %3d %5.1f \n", n, min, max, sum/NR}
'
done
---
Makefile | 1 +
t/helper/test-prio-queue.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
4 files changed, 94 insertions(+)
diff --git a/Makefile b/Makefile
index 1f3f099f5c5..ba7d293cf5f 100644
--- a/Makefile
+++ b/Makefile
@@ -843,6 +843,7 @@ TEST_BUILTINS_OBJS += test-partial-clone.o
TEST_BUILTINS_OBJS += test-path-utils.o
TEST_BUILTINS_OBJS += test-path-walk.o
TEST_BUILTINS_OBJS += test-pcre2-config.o
+TEST_BUILTINS_OBJS += test-prio-queue.o
TEST_BUILTINS_OBJS += test-pkt-line.o
TEST_BUILTINS_OBJS += test-proc-receive.o
TEST_BUILTINS_OBJS += test-progress.o
diff --git a/t/helper/test-prio-queue.c b/t/helper/test-prio-queue.c
new file mode 100644
index 00000000000..c175021b12b
--- /dev/null
+++ b/t/helper/test-prio-queue.c
@@ -0,0 +1,91 @@
+#include "test-tool.h"
+#include "prio-queue.h"
+
+/* Generate all permutations using Heap's algorithm. */
+static int permute_ints(size_t n, void (*fn)(int *, size_t))
+{
+ int *arr;
+ size_t *c;
+
+ ALLOC_ARRAY(arr, n);
+ for (size_t i = 0; i < n; i++)
+ arr[i] = i + 1;
+ CALLOC_ARRAY(c, n);
+
+ fn(arr, n);
+ for (size_t i = 1; i < n; i++) {
+ if (c[i] < i) {
+ SWAP(arr[i & 1 ? c[i] : 0], arr[i]);
+ fn(arr, n);
+ c[i]++;
+ i = 0;
+ } else {
+ c[i] = 0;
+ }
+ }
+
+ free(arr);
+ free(c);
+
+ return 0;
+}
+
+static uintmax_t nr_of_compares;
+
+static int compare_ints(const void *a_, const void *b_, void *cb_data UNUSED)
+{
+ const int *a = a_;
+ const int *b = b_;
+ nr_of_compares++;
+ return *a - *b;
+}
+
+static void report(const char *name, const int *arr, size_t n)
+{
+ printf("%s %"PRIuMAX" for", name, nr_of_compares);
+ for (size_t i = 0; i < n; i++)
+ printf(" %d", arr[i]);
+ putchar('\n');
+}
+
+static void get_permutation(int *arr, size_t n)
+{
+ static struct prio_queue queue = { compare_ints };
+
+ for (size_t i = 0; i < n; i++)
+ prio_queue_put(&queue, &arr[i]);
+
+ nr_of_compares = 0;
+ for (size_t i = 0; i < n; i++)
+ prio_queue_get(&queue);
+
+ report("get", arr, n);
+}
+
+static void put_permutation(int *arr, size_t n)
+{
+ struct prio_queue queue = { compare_ints };
+
+ nr_of_compares = 0;
+ for (size_t i = 0; i < n; i++)
+ prio_queue_put(&queue, &arr[i]);
+
+ report("put", arr, n);
+
+ clear_prio_queue(&queue);
+}
+
+int cmd__prio_queue(int argc, const char **argv)
+{
+ if (argc == 4 && !strcmp(argv[1], "permute")) {
+ size_t n = strtoul(argv[3], NULL, 10);
+ if (!strcmp(argv[2], "get"))
+ return permute_ints(n, get_permutation);
+ if (!strcmp(argv[2], "put"))
+ return permute_ints(n, put_permutation);
+ }
+
+ fprintf(stderr, "usage: test-tool prio-queue permute get <n>\n");
+ fprintf(stderr, " or: test-tool prio-queue permute put <n>\n");
+ return 129;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index b71a22b43bb..69352f541f4 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -57,6 +57,7 @@ static struct test_cmd cmds[] = {
{ "path-walk", cmd__path_walk },
{ "pcre2-config", cmd__pcre2_config },
{ "pkt-line", cmd__pkt_line },
+ { "prio-queue", cmd__prio_queue },
{ "proc-receive", cmd__proc_receive },
{ "progress", cmd__progress },
{ "reach", cmd__reach },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index f2885b33d58..ab0d3e01d1e 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -50,6 +50,7 @@ int cmd__path_utils(int argc, const char **argv);
int cmd__path_walk(int argc, const char **argv);
int cmd__pcre2_config(int argc, const char **argv);
int cmd__pkt_line(int argc, const char **argv);
+int cmd__prio_queue(int argc, const char **argv);
int cmd__proc_receive(int argc, const char **argv);
int cmd__progress(int argc, const char **argv);
int cmd__reach(int argc, const char **argv);
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-07-08 10:43 ` René Scharfe
@ 2026-07-08 10:59 ` Kristofer Karlsson
2026-07-08 11:55 ` René Scharfe
0 siblings, 1 reply; 24+ messages in thread
From: Kristofer Karlsson @ 2026-07-08 10:59 UTC (permalink / raw)
To: René Scharfe
Cc: Junio C Hamano, Kristofer Karlsson via GitGitGadget, git
On Wed, 8 Jul 2026 at 12:44, René Scharfe <l.s.r@web.de> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-07-08 10:59 ` Kristofer Karlsson
@ 2026-07-08 11:55 ` René Scharfe
2026-07-08 12:44 ` Kristofer Karlsson
0 siblings, 1 reply; 24+ messages in thread
From: René Scharfe @ 2026-07-08 11:55 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: Junio C Hamano, Kristofer Karlsson via GitGitGadget, git
On 7/8/26 12:59 PM, Kristofer Karlsson wrote:
> On Wed, 8 Jul 2026 at 12:44, René Scharfe <l.s.r@web.de> wrote:
>>
>> I didn't
>> find this method used anywhere else, which is a warning sign, but I
>> can't find any catch.
>
> 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 meant that I didn't find this optimization in other priority queue
implementations or papers, but admittedly I didn't do an exhaustive
search. Given it's benefits I would have expected to find prior art
on it pretty easily, though.
René
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
2026-07-08 11:55 ` René Scharfe
@ 2026-07-08 12:44 ` Kristofer Karlsson
0 siblings, 0 replies; 24+ messages in thread
From: Kristofer Karlsson @ 2026-07-08 12:44 UTC (permalink / raw)
To: René Scharfe
Cc: Junio C Hamano, Kristofer Karlsson via GitGitGadget, git
On Wed, 8 Jul 2026 at 13:55, René Scharfe <l.s.r@web.de> wrote:
>
> I meant that I didn't find this optimization in other priority queue
> implementations or papers, but admittedly I didn't do an exhaustive
> search. Given it's benefits I would have expected to find prior art
> on it pretty easily, though.
Aha! Got it, I misunderstood you first.
It's actually described here[1]:
> Bottom-up heapsort conceptually replaces the root with a value of −∞
> and sifts it down using only one comparison per level
> (since no child can possibly be less than −∞)
> until the leaves are reached,
> then replaces the −∞ with the correct value and sifts it up
> (again, using one comparison per level) until the correct position
> is found.
It's for heapsort, not an interactive heap, but the algorithm
still matches.
Also, your idea to split out sift up/down into helper functions did
work, and was quite clean - will share patch shortly once I have
cleaned up the commits and benchmark data.
Thanks again,
Kristofer
[1] https://en.wikipedia.org/wiki/Heapsort#Bottom-up_heapsort
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min
2026-06-01 8:17 ` [PATCH v2] prio-queue: use cascade-down " Kristofer Karlsson via GitGitGadget
2026-06-02 16:36 ` René Scharfe
@ 2026-07-08 17:49 ` Kristofer Karlsson via GitGitGadget
2026-07-08 17:49 ` [PATCH v3 1/2] prio-queue: extract sift_up() from prio_queue_put() Kristofer Karlsson via GitGitGadget
` (2 more replies)
1 sibling, 3 replies; 24+ messages in thread
From: Kristofer Karlsson via GitGitGadget @ 2026-07-08 17:49 UTC (permalink / raw)
To: git; +Cc: René Scharfe, Kristofer Karlsson, Kristofer Karlsson
This tweaks the prio_queue implementation to use a bottom-up approach
sifting for get [1].
In practice, the performance boost is small, but measurable for reasonably
large prio_queue:s (thousands of elements, not millions) but it should never
increase the work.
Minor note on v3: After the most recent discussion I am not 100% sure how to
reason about the value of this change - both the value gain and code cost
seem small, but since there was some interest and research done by René I
wanted to complete this v3 anyway so it can be properly discussed (though
still maybe ultimately closed).
Here's how it works:
Instead of placing the last element at the root and sifting it down with two
comparisons per level, cascade the vacancy down by promoting the smaller
child (one comparison per level), then place the last element at the vacancy
and sift it up. Since the displaced element is likely to belong near the
bottom of the heap, sift_up() typically does very little work.
sift_down_root() is kept as-is for the fused replace path in
prio_queue_put(), where the new element is arbitrary and may belong near the
root -- Rene's testing showed that cascade regresses on git-describe for
this reason.
Benchmarks (rev-list --all --count) on public repos confirm no regression on
git.git and linux.git. On a large example repo with thousands of active
branches the cascade yields a measurable (~2%) end-to-end improvement; the
gain is modest because the lazy-fold optimization (now in next) already
fuses most get+put pairs, leaving only the remaining unfused gets to benefit
from cascade.
René's exhaustive analysis [2] of all permutations up to n=12 confirms that
cascade never requires more comparisons than standard sift-down for a full
drain.
Note: sift_up() currently uses swap, matching the existing code style. It
could be further optimized to use copy (hold the element in a temp, shift
parents down, write once), but that would require changing compare() to
accept element values instead of array indices. Left for a potential
follow-up.
Changes since v2:
* Rebased on kk/prio-queue-get-put-fusion (now in next).
* Split into two commits - refactoring and then introducing cascade_down.
Changes since v1:
* Kept sift_down_root() and prio_queue_replace() completely unchanged,
preserving René's optimization that avoids the get+put overhead for
replace. The cascade approach now only applies to prio_queue_get().
* Extracted the new logic into a separate sift_up_rebalance() function
rather than inlining it in prio_queue_get().
* Updated benchmark numbers for ascending, descending and random insertion
ordering. No regressions in any scenario.
[1] https://en.wikipedia.org/wiki/Heapsort#Bottom-up_heapsort [2]
https://lore.kernel.org/git/pull.2132.git.1780250236304.gitgitgadget@gmail.com/T/#m114df6e1c2845acbbc64d875ed7dc1d7d9193ed5
Kristofer Karlsson (2):
prio-queue: extract sift_up() from prio_queue_put()
prio-queue: use cascade for unfused gets
prio-queue.c | 43 ++++++++++++++++++++++++++++++++-----------
1 file changed, 32 insertions(+), 11 deletions(-)
base-commit: 00534a21ce949ef80a5b8b9d7fc20b7d381038e9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2132%2Fspkrka%2Fcascade-sift-down-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2132/spkrka/cascade-sift-down-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2132
Range-diff vs v2:
-: ---------- > 1: ec6a448563 prio-queue: extract sift_up() from prio_queue_put()
1: 6051d44e59 ! 2: 89a22c6a75 prio-queue: use cascade-down for faster extract-min
@@ Metadata
Author: Kristofer Karlsson <krka@spotify.com>
## Commit message ##
- prio-queue: use cascade-down for faster extract-min
+ prio-queue: use cascade for unfused gets
- Add sift_up_rebalance(), an alternative to sift_down_root() that
- halves the number of comparisons per extract-min.
+ When flush_get() removes the root without an immediate replacement,
+ use a cascade-then-sift-up strategy instead of sift-down.
- The standard extract places the last array element at the root and
- sifts it down. At each level this requires two comparisons (left
- vs right child, then element vs winner) and a swap.
+ Standard sift-down places the last element at the root and sifts it
+ down. This needs two comparisons per level (pick the smaller child,
+ then compare against the element), even though the displaced element
+ almost always ends up near the bottom where it came from.
- sift_up_rebalance() instead promotes the smaller child into the
- root slot at each level — one comparison and one copy — until the
- vacancy reaches a leaf. The last array element is placed at the
- vacancy and sifted up to restore heap order. In practice the
- sift-up rarely moves more than a level or two because the last
- array element tends to be large.
+ cascade_down() instead moves the vacancy down by promoting the
+ smaller child at each level (one comparison per level), leaving the
+ vacancy at a leaf. The last element is then placed at the vacancy
+ and sift_up() floats it to its correct position, which is typically
+ very little work since it already belongs near the bottom.
- Work per extract drops from 2d comparisons + d swaps to
- d comparisons + d copies + a short sift-up.
+ This is the well-known "bottom-up" variant of sift-down [1].
- prio_queue_get() now calls sift_up_rebalance() instead of placing
- the last element at root and calling sift_down_root().
-
- sift_down_root() and prio_queue_replace() are left unchanged.
-
- Synthetic benchmark (10 rounds of 10M put+get cycles, CPU-pinned,
- same compiler and Makefile flags):
-
- Ascending keys (git's typical pattern — parents have lower
- priority than children):
-
- queue width baseline patched speedup
- 10 4.39s 3.91s 1.12x
- 100 9.10s 6.61s 1.38x
- 1,000 11.84s 9.25s 1.28x
- 10,000 17.50s 13.92s 1.26x
- 100,000 23.97s 20.19s 1.19x
-
- Descending keys (worst case — last element always sinks to leaf):
-
- queue width baseline patched speedup
- 10 4.94s 4.95s 1.00x
- 100 9.75s 9.42s 1.03x
- 1,000 15.01s 15.29s 0.98x
- 10,000 24.79s 23.88s 1.04x
- 100,000 29.69s 28.24s 1.05x
-
- Random keys:
-
- queue width baseline patched speedup
- 10 5.05s 4.99s 1.01x
- 100 9.90s 9.50s 1.04x
- 1,000 15.35s 14.77s 1.04x
- 10,000 25.35s 24.21s 1.05x
- 100,000 65.71s 63.38s 1.04x
-
- No regressions in any scenario.
-
- End-to-end benchmark on the linux kernel repo (1.4M commits,
- range v5.0..v6.0, 311K commits, 20 interleaved runs, 1 warmup):
-
- Command baseline patched speedup
- rev-list --count v5.0..v6.0 484ms 474ms 1.02x
-
- The improvement scales with DAG width: wider DAGs produce larger
- priority queues, amplifying the per-level savings. In small or
- narrow repositories the queues stay shallow and the sift-down
- cost is already negligible.
+ [1] https://en.wikipedia.org/wiki/Heapsort#Bottom-up_heapsort
+ Helped-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
## prio-queue.c ##
@@ prio-queue.c: static void sift_down_root(struct prio_queue *queue)
}
}
-+static void sift_up_rebalance(struct prio_queue *queue)
++/* Cascade vacancy toward a leaf, promoting the smaller child at each level */
++static size_t cascade_down(struct prio_queue *queue)
+{
+ size_t ix, child;
+
-+ /* Cascade: promote smaller child at each level. */
-+ for (ix = 0; (child = ix * 2 + 1) < queue->nr; ix = child) {
-+ if (child + 1 < queue->nr &&
++ for (ix = 0; (child = ix * 2 + 1) < queue->nr_; ix = child) {
++ if (child + 1 < queue->nr_ &&
+ compare(queue, child, child + 1) >= 0)
+ child++;
+ queue->array[ix] = queue->array[child];
+ }
-+
-+ /* Place the last element at the vacancy and sift up. */
-+ queue->array[ix] = queue->array[queue->nr];
-+ while (ix) {
-+ size_t parent = (ix - 1) / 2;
-+ if (compare(queue, parent, ix) <= 0)
-+ break;
-+ swap(queue, parent, ix);
-+ ix = parent;
-+ }
++ return ix;
+}
+
- void *prio_queue_get(struct prio_queue *queue)
+ static inline void flush_get(struct prio_queue *queue)
{
- void *result;
-@@ prio-queue.c: void *prio_queue_get(struct prio_queue *queue)
- if (!--queue->nr)
- return result;
-
-- queue->array[0] = queue->array[queue->nr];
++ size_t ix;
++
+ if (!queue->get_pending)
+ return;
+ queue->get_pending = 0;
+- queue->array[0] = queue->array[--queue->nr_];
- sift_down_root(queue);
-+ sift_up_rebalance(queue);
- return result;
++ --queue->nr_;
++ ix = cascade_down(queue);
++ queue->array[ix] = queue->array[queue->nr_];
++ sift_up(queue, ix);
}
+ void prio_queue_put(struct prio_queue *queue, void *thing)
--
gitgitgadget
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 1/2] prio-queue: extract sift_up() from prio_queue_put()
2026-07-08 17:49 ` [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min Kristofer Karlsson via GitGitGadget
@ 2026-07-08 17:49 ` Kristofer Karlsson via GitGitGadget
2026-07-08 17:49 ` [PATCH v3 2/2] prio-queue: use cascade for unfused gets Kristofer Karlsson via GitGitGadget
2026-07-10 16:37 ` [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min René Scharfe
2 siblings, 0 replies; 24+ messages in thread
From: Kristofer Karlsson via GitGitGadget @ 2026-07-08 17:49 UTC (permalink / raw)
To: git
Cc: René Scharfe, Kristofer Karlsson, Kristofer Karlsson,
Kristofer Karlsson
From: Kristofer Karlsson <krka@spotify.com>
Factor out the bubble-up loop from prio_queue_put() into a
standalone sift_up() function. This is a pure refactor with
no behavior change, preparing for reuse in a subsequent commit.
Suggested-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
prio-queue.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/prio-queue.c b/prio-queue.c
index 199775d5af..926fc04e85 100644
--- a/prio-queue.c
+++ b/prio-queue.c
@@ -37,6 +37,17 @@ void clear_prio_queue(struct prio_queue *queue)
queue->get_pending = 0;
}
+static void sift_up(struct prio_queue *queue, size_t ix)
+{
+ while (ix) {
+ size_t parent = (ix - 1) / 2;
+ if (compare(queue, parent, ix) <= 0)
+ break;
+ swap(queue, parent, ix);
+ ix = parent;
+ }
+}
+
static void sift_down_root(struct prio_queue *queue)
{
size_t ix, child;
@@ -66,8 +77,6 @@ static inline void flush_get(struct prio_queue *queue)
void prio_queue_put(struct prio_queue *queue, void *thing)
{
- size_t ix, parent;
-
if (queue->get_pending) {
queue->get_pending = 0;
queue->array[0].ctr = queue->insertion_ctr++;
@@ -85,13 +94,7 @@ void prio_queue_put(struct prio_queue *queue, void *thing)
return; /* LIFO */
/* Bubble up the new one */
- for (ix = queue->nr_ - 1; ix; ix = parent) {
- parent = (ix - 1) / 2;
- if (compare(queue, parent, ix) <= 0)
- break;
-
- swap(queue, parent, ix);
- }
+ sift_up(queue, queue->nr_ - 1);
}
void *prio_queue_get(struct prio_queue *queue)
--
gitgitgadget
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 2/2] prio-queue: use cascade for unfused gets
2026-07-08 17:49 ` [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min Kristofer Karlsson via GitGitGadget
2026-07-08 17:49 ` [PATCH v3 1/2] prio-queue: extract sift_up() from prio_queue_put() Kristofer Karlsson via GitGitGadget
@ 2026-07-08 17:49 ` Kristofer Karlsson via GitGitGadget
2026-07-10 16:37 ` René Scharfe
2026-07-10 16:37 ` [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min René Scharfe
2 siblings, 1 reply; 24+ messages in thread
From: Kristofer Karlsson via GitGitGadget @ 2026-07-08 17:49 UTC (permalink / raw)
To: git
Cc: René Scharfe, Kristofer Karlsson, Kristofer Karlsson,
Kristofer Karlsson
From: Kristofer Karlsson <krka@spotify.com>
When flush_get() removes the root without an immediate replacement,
use a cascade-then-sift-up strategy instead of sift-down.
Standard sift-down places the last element at the root and sifts it
down. This needs two comparisons per level (pick the smaller child,
then compare against the element), even though the displaced element
almost always ends up near the bottom where it came from.
cascade_down() instead moves the vacancy down by promoting the
smaller child at each level (one comparison per level), leaving the
vacancy at a leaf. The last element is then placed at the vacancy
and sift_up() floats it to its correct position, which is typically
very little work since it already belongs near the bottom.
This is the well-known "bottom-up" variant of sift-down [1].
[1] https://en.wikipedia.org/wiki/Heapsort#Bottom-up_heapsort
Helped-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
prio-queue.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/prio-queue.c b/prio-queue.c
index 926fc04e85..230d6f5e33 100644
--- a/prio-queue.c
+++ b/prio-queue.c
@@ -66,13 +66,31 @@ static void sift_down_root(struct prio_queue *queue)
}
}
+/* Cascade vacancy toward a leaf, promoting the smaller child at each level */
+static size_t cascade_down(struct prio_queue *queue)
+{
+ size_t ix, child;
+
+ for (ix = 0; (child = ix * 2 + 1) < queue->nr_; ix = child) {
+ if (child + 1 < queue->nr_ &&
+ compare(queue, child, child + 1) >= 0)
+ child++;
+ queue->array[ix] = queue->array[child];
+ }
+ return ix;
+}
+
static inline void flush_get(struct prio_queue *queue)
{
+ size_t ix;
+
if (!queue->get_pending)
return;
queue->get_pending = 0;
- queue->array[0] = queue->array[--queue->nr_];
- sift_down_root(queue);
+ --queue->nr_;
+ ix = cascade_down(queue);
+ queue->array[ix] = queue->array[queue->nr_];
+ sift_up(queue, ix);
}
void prio_queue_put(struct prio_queue *queue, void *thing)
--
gitgitgadget
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/2] prio-queue: use cascade for unfused gets
2026-07-08 17:49 ` [PATCH v3 2/2] prio-queue: use cascade for unfused gets Kristofer Karlsson via GitGitGadget
@ 2026-07-10 16:37 ` René Scharfe
2026-07-10 17:28 ` Kristofer Karlsson
0 siblings, 1 reply; 24+ messages in thread
From: René Scharfe @ 2026-07-10 16:37 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget, git; +Cc: Kristofer Karlsson
On 7/8/26 7:49 PM, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify.com>
>
> When flush_get() removes the root without an immediate replacement,
> use a cascade-then-sift-up strategy instead of sift-down.
>
> Standard sift-down places the last element at the root and sifts it
> down. This needs two comparisons per level (pick the smaller child,
> then compare against the element), even though the displaced element
> almost always ends up near the bottom where it came from.
>
> cascade_down() instead moves the vacancy down by promoting the
> smaller child at each level (one comparison per level), leaving the
> vacancy at a leaf. The last element is then placed at the vacancy
> and sift_up() floats it to its correct position, which is typically
> very little work since it already belongs near the bottom.
>
> This is the well-known "bottom-up" variant of sift-down [1].
>
> [1] https://en.wikipedia.org/wiki/Heapsort#Bottom-up_heapsort
On an Apple M1 I get a 1% slowdown for bulk describe on Git's repo:
Benchmark 1: ./git_next describe $(git rev-list v2.41.0..v2.47.0)
Time (mean ± σ): 939.5 ms ± 3.6 ms [User: 576.8 ms, System: 65.0 ms]
Range (min … max): 935.0 ms … 946.2 ms 10 runs
Benchmark 2: ./git describe $(git rev-list v2.41.0..v2.47.0)
Time (mean ± σ): 945.5 ms ± 3.3 ms [User: 581.6 ms, System: 67.5 ms]
Range (min … max): 940.1 ms … 950.5 ms 10 runs
Summary
./git_next describe $(git rev-list v2.41.0..v2.47.0) ran
1.01 ± 0.01 times faster than ./git describe $(git rev-list v2.41.0..v2.47.0)
... and on Linux's repo:
Benchmark 1: ./git_next -C ../linux describe $(git -C ../linux rev-list v4.0..v4.1)
Time (mean ± σ): 4.880 s ± 0.014 s [User: 3.914 s, System: 0.252 s]
Range (min … max): 4.864 s … 4.905 s 10 runs
Benchmark 2: ./git -C ../linux describe $(git -C ../linux rev-list v4.0..v4.1)
Time (mean ± σ): 4.917 s ± 0.011 s [User: 3.948 s, System: 0.254 s]
Range (min … max): 4.902 s … 4.938 s 10 runs
Summary
./git_next -C ../linux describe $(git -C ../linux rev-list v4.0..v4.1) ran
1.01 ± 0.00 times faster than ./git -C ../linux describe $(git -C ../linux rev-list v4.0..v4.1)
I see a 1% slowdown on an Apple M5 as well in both cases. I can't
reproduce it on a Ryzen laptop, but that's too noisy to measure 1%
changes anyway.
Checked the total number of prio_queue comparisons with the crude patch
below, and as expected they go down, from 70386235 to 60682175 for Git
and from 473983445 to 439809087 for Linux. So there's less work to do,
still user time goes up -- no idea why.
Also this -- what's up with the system time here:
Benchmark 1: ./git_next rev-list --all --count
Time (mean ± σ): 115.2 ms ± 0.8 ms [User: 95.6 ms, System: 17.7 ms]
Range (min … max): 113.0 ms … 117.1 ms 24 runs
Benchmark 2: ./git rev-list --all --count
Time (mean ± σ): 116.5 ms ± 0.8 ms [User: 95.4 ms, System: 19.0 ms]
Range (min … max): 115.1 ms … 118.6 ms 24 runs
Summary
./git_next rev-list --all --count ran
1.01 ± 0.01 times faster than ./git rev-list --all --count
But:
Benchmark 1: ./git_next -C ../linux rev-list --all --count
Time (mean ± σ): 937.6 ms ± 2.2 ms [User: 887.2 ms, System: 45.5 ms]
Range (min … max): 933.2 ms … 939.9 ms 10 runs
Benchmark 2: ./git -C ../linux rev-list --all --count
Time (mean ± σ): 937.3 ms ± 1.7 ms [User: 887.8 ms, System: 45.0 ms]
Range (min … max): 934.6 ms … 940.3 ms 10 runs
Summary
./git -C ../linux rev-list --all --count ran
1.00 ± 0.00 times faster than ./git_next -C ../linux rev-list --all --count
:-?
> Helped-by: Rene Scharfe <l.s.r@web.de>
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---
> prio-queue.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/prio-queue.c b/prio-queue.c
> index 926fc04e85..230d6f5e33 100644
> --- a/prio-queue.c
> +++ b/prio-queue.c
> @@ -66,13 +66,31 @@ static void sift_down_root(struct prio_queue *queue)
> }
> }
>
> +/* Cascade vacancy toward a leaf, promoting the smaller child at each level */
> +static size_t cascade_down(struct prio_queue *queue)
> +{
> + size_t ix, child;
> +
> + for (ix = 0; (child = ix * 2 + 1) < queue->nr_; ix = child) {
> + if (child + 1 < queue->nr_ &&
> + compare(queue, child, child + 1) >= 0)
> + child++;
> + queue->array[ix] = queue->array[child];
> + }
> + return ix;
> +}
> +
> static inline void flush_get(struct prio_queue *queue)
> {
> + size_t ix;
> +
> if (!queue->get_pending)
> return;
> queue->get_pending = 0;
> - queue->array[0] = queue->array[--queue->nr_];
> - sift_down_root(queue);
> + --queue->nr_;
> + ix = cascade_down(queue);
> + queue->array[ix] = queue->array[queue->nr_];
> + sift_up(queue, ix);
> }
>
> void prio_queue_put(struct prio_queue *queue, void *thing)
The patch looks fine, though. It introduces struct assignments, but
they should be OK. Tried replacing them with swap() instead (which
does a useless extra write), but that didn't change the performance
(still 1% slowdown). Odd.
René
diff --git a/builtin/describe.c b/builtin/describe.c
index c0abc931a59..4a6ad976d30 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -791,5 +791,6 @@ int cmd_describe(int argc,
while (argc-- > 0)
describe(*argv++, argc == 0);
}
+ print_compares();
return 0;
}
diff --git a/prio-queue.c b/prio-queue.c
index 199775d5afd..b0189bf80e6 100644
--- a/prio-queue.c
+++ b/prio-queue.c
@@ -1,6 +1,13 @@
#include "git-compat-util.h"
#include "prio-queue.h"
+static uintmax_t compares;
+
+void print_compares(void)
+{
+ fprintf(stderr, "compares: %lu\n", compares);
+}
+
static inline int compare(struct prio_queue *queue, size_t i, size_t j)
{
int cmp = queue->compare(queue->array[i].data, queue->array[j].data,
@@ -8,6 +15,7 @@ static inline int compare(struct prio_queue *queue, size_t i, size_t j)
if (!cmp)
cmp = (queue->array[i].ctr > queue->array[j].ctr) -
(queue->array[i].ctr < queue->array[j].ctr);
+ compares++;
return cmp;
}
diff --git a/prio-queue.h b/prio-queue.h
index 570b48e6485..e4cc0c4fb83 100644
--- a/prio-queue.h
+++ b/prio-queue.h
@@ -68,4 +68,6 @@ void clear_prio_queue(struct prio_queue *);
/* Reverse the LIFO elements */
void prio_queue_reverse(struct prio_queue *);
+void print_compares(void);
+
#endif /* PRIO_QUEUE_H */
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min
2026-07-08 17:49 ` [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min Kristofer Karlsson via GitGitGadget
2026-07-08 17:49 ` [PATCH v3 1/2] prio-queue: extract sift_up() from prio_queue_put() Kristofer Karlsson via GitGitGadget
2026-07-08 17:49 ` [PATCH v3 2/2] prio-queue: use cascade for unfused gets Kristofer Karlsson via GitGitGadget
@ 2026-07-10 16:37 ` René Scharfe
2026-07-10 17:40 ` Kristofer Karlsson
2 siblings, 1 reply; 24+ messages in thread
From: René Scharfe @ 2026-07-10 16:37 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget, git; +Cc: Kristofer Karlsson
On 7/8/26 7:49 PM, Kristofer Karlsson via GitGitGadget wrote:
> Note: sift_up() currently uses swap, matching the existing code style. It
> could be further optimized to use copy (hold the element in a temp, shift
> parents down, write once), but that would require changing compare() to
> accept element values instead of array indices. Left for a potential
> follow-up.
Same for sift_down_root(), I guess? It could almost halve the number of
writes, right? I wonder how much of that benefit will be eaten by
caching.
René
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/2] prio-queue: use cascade for unfused gets
2026-07-10 16:37 ` René Scharfe
@ 2026-07-10 17:28 ` Kristofer Karlsson
0 siblings, 0 replies; 24+ messages in thread
From: Kristofer Karlsson @ 2026-07-10 17:28 UTC (permalink / raw)
To: René Scharfe; +Cc: Kristofer Karlsson via GitGitGadget, git
On Fri, 10 Jul 2026 at 18:37, René Scharfe <l.s.r@web.de> wrote:
>
> I see a 1% slowdown on an Apple M5 as well in both cases. I can't
> reproduce it on a Ryzen laptop, but that's too noisy to measure 1%
> changes anyway.
>
> Checked the total number of prio_queue comparisons with the crude patch
> below, and as expected they go down, from 70386235 to 60682175 for Git
> and from 473983445 to 439809087 for Linux. So there's less work to do,
> still user time goes up -- no idea why.
[snip]
> The patch looks fine, though. It introduces struct assignments, but
> they should be OK. Tried replacing them with swap() instead (which
> does a useless extra write), but that didn't change the performance
> (still 1% slowdown). Odd.
First of all, thanks again for the very comprehensive
investigation on your own hardware!
I don't have any Apple machine to test on but I reran your
exact operation on my machine
(Lenovo Thinkpad Intel(R) Core(TM) Ultra 7 155U)
and just saw noise.
As you say, this feels _logically_ better since it's fewer
compares but perhaps this boils down to the cost difference
between executing CPU operations versus memory
access and the CPU cache?
My random guess:
cascade_down() has fewer operations and compares
but needs to visit all levels of the heap,
while sift_down_root perhaps stops slightly earlier,
so the memory region right before the end
gets fewer visits and reduces pressure on
the cache.
I have no idea if my guess is correct,
it's maybe more subtle than that, but
I think ultimately this points to the fact
that while the change is a theoretical
improvement, the real world hardware
tradeoffs make it a non-obvious change.
I think this means we should simply drop the change
and move on -- it produced bigger gains
before making the lazy prio_queue the default,
but now it seems like it is pure noise, unless
the comparator function grows more expensive
in the future.
Thanks,
Kristofer
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min
2026-07-10 16:37 ` [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min René Scharfe
@ 2026-07-10 17:40 ` Kristofer Karlsson
0 siblings, 0 replies; 24+ messages in thread
From: Kristofer Karlsson @ 2026-07-10 17:40 UTC (permalink / raw)
To: René Scharfe; +Cc: Kristofer Karlsson via GitGitGadget, git
On Fri, 10 Jul 2026 at 18:37, René Scharfe <l.s.r@web.de> wrote:
>
> On 7/8/26 7:49 PM, Kristofer Karlsson via GitGitGadget wrote:
> > Note: sift_up() currently uses swap, matching the existing code style. It
> > could be further optimized to use copy (hold the element in a temp, shift
> > parents down, write once), but that would require changing compare() to
> > accept element values instead of array indices. Left for a potential
> > follow-up.
>
> Same for sift_down_root(), I guess? It could almost halve the number of
> writes, right? I wonder how much of that benefit will be eaten by
> caching.
Hm yes indeed, I stopped looking past sift_up() when I realized I should
not expand the scope of the change. But I think the CPU cache
effectively makes the swap almost as cheap in practice.
- Kristofer
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2026-07-10 17:41 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-31 17:57 [PATCH] prio-queue: use cascade-down sift for faster extract-min Kristofer Karlsson via GitGitGadget
2026-06-01 0:09 ` Junio C Hamano
2026-06-01 6:16 ` Junio C Hamano
2026-06-01 6:21 ` Kristofer Karlsson
2026-06-01 8:17 ` [PATCH v2] prio-queue: use cascade-down " Kristofer Karlsson via GitGitGadget
2026-06-02 16:36 ` René Scharfe
2026-06-02 22:40 ` Kristofer Karlsson
2026-06-05 20:39 ` Kristofer Karlsson
2026-06-07 7:30 ` René Scharfe
2026-06-07 12:07 ` Kristofer Karlsson
2026-06-29 21:16 ` Junio C Hamano
2026-06-08 11:56 ` Junio C Hamano
2026-07-06 21:52 ` Kristofer Karlsson
2026-07-08 10:43 ` René Scharfe
2026-07-08 10:59 ` Kristofer Karlsson
2026-07-08 11:55 ` René Scharfe
2026-07-08 12:44 ` Kristofer Karlsson
2026-07-08 17:49 ` [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min Kristofer Karlsson via GitGitGadget
2026-07-08 17:49 ` [PATCH v3 1/2] prio-queue: extract sift_up() from prio_queue_put() Kristofer Karlsson via GitGitGadget
2026-07-08 17:49 ` [PATCH v3 2/2] prio-queue: use cascade for unfused gets Kristofer Karlsson via GitGitGadget
2026-07-10 16:37 ` René Scharfe
2026-07-10 17:28 ` Kristofer Karlsson
2026-07-10 16:37 ` [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min René Scharfe
2026-07-10 17:40 ` Kristofer Karlsson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.