]> git.ipfire.org Git - thirdparty/git.git/commitdiff
prio-queue: fold lazy_queue into prio_queue for automatic get+put fusion
authorKristofer Karlsson <krka@spotify.com>
Mon, 8 Jun 2026 19:10:51 +0000 (19:10 +0000)
committerJunio C Hamano <gitster@pobox.com>
Tue, 9 Jun 2026 18:11:46 +0000 (11:11 -0700)
Defer the actual removal in prio_queue_get() until the next
operation.  If that next operation is a prio_queue_put(), the
removal and insertion are fused into a single replace — writing
the new element at the root and sifting it down — which avoids
a full remove-rebalance-insert cycle.

This matches the dominant usage pattern in git's commit traversal:
get a commit, then put its parents.  The first parent insertion
after each get is now a replace operation automatically.

This generalizes the lazy_queue pattern from builtin/describe.c
(introduced in 08bb69d70f) into prio_queue itself.  Three callers
independently implemented the same get+put fusion:

  - builtin/describe.c had a full lazy_queue wrapper
  - commit.c:pop_most_recent_commit() used peek+replace
  - builtin/show-branch.c:join_revs() used peek+replace

All three now collapse to plain _get() and _put(), with the data
structure handling the fusion internally.  This simplifies callers
and means every prio_queue user gets the optimization for free
without needing to implement it manually.

Remove prio_queue_replace() since no external callers remain.

Benchmarked on a 1.8M-commit monorepo (30 interleaved runs,
paired t-test, Xeon @ 2.20GHz):

Code paths that previously did eager get+put (new optimization):

  Command                       base    patched  change      p
  merge-base --all A A~1000     3828ms  3725ms   -2.69%  0.0001
  rev-list --count A~1000..A    3055ms  2986ms   -2.27%  0.0601
  log --oneline A~1000..A       3408ms  3350ms   -1.71%  0.0482

Code paths that already had manual get+put fusion (expect
neutral — the optimization moves into prio_queue but the number
of heap operations stays the same):

  Command                       base    patched  change      p
  show-branch A A~1000          9156ms  9127ms   -0.32%  0.3470
  describe (4751 revs, 81K repo) 1983ms 1963ms  -1.02%  <0.001

No regressions in any scenario.

Suggested-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/describe.c
builtin/show-branch.c
commit.c
prio-queue.c
prio-queue.h
t/unit-tests/u-prio-queue.c

index 8e88bdeea6cf1c6a320b397bf8e61a0d7bac82c2..64424543ef229c7d87ef23334d7ced09755d6a11 100644 (file)
@@ -251,61 +251,19 @@ static int compare_pt(const void *a_, const void *b_)
        return 0;
 }
 
-struct lazy_queue {
-       struct prio_queue queue;
-       bool get_pending;
-};
-
-#define LAZY_QUEUE_INIT { { compare_commits_by_commit_date }, false }
-
-static void *lazy_queue_get(struct lazy_queue *queue)
-{
-       if (queue->get_pending)
-               prio_queue_get(&queue->queue);
-       else
-               queue->get_pending = true;
-       return prio_queue_peek(&queue->queue);
-}
-
-static void lazy_queue_put(struct lazy_queue *queue, void *thing)
-{
-       if (queue->get_pending)
-               prio_queue_replace(&queue->queue, thing);
-       else
-               prio_queue_put(&queue->queue, thing);
-       queue->get_pending = false;
-}
-
-static bool lazy_queue_empty(const struct lazy_queue *queue)
-{
-       return prio_queue_size(&queue->queue) == (queue->get_pending ? 1 : 0);
-}
-
-static void lazy_queue_clear(struct lazy_queue *queue)
-{
-       clear_prio_queue(&queue->queue);
-       queue->get_pending = false;
-}
-
-static unsigned long finish_depth_computation(struct lazy_queue *queue,
+static unsigned long finish_depth_computation(struct prio_queue *queue,
                                              struct possible_tag *best)
 {
        unsigned long seen_commits = 0;
        struct oidset unflagged = OIDSET_INIT;
-       struct commit *commit;
-       int skip = queue->get_pending ? 1 : 0;
+       struct commit *c;
 
-       prio_queue_for_each(&queue->queue, commit) {
-               if (skip) {
-                       skip = 0;
-                       continue;
-               }
-               if (!(commit->object.flags & best->flag_within))
-                       oidset_insert(&unflagged, &commit->object.oid);
+       prio_queue_for_each(queue, c) {
+               if (!(c->object.flags & best->flag_within))
+                       oidset_insert(&unflagged, &c->object.oid);
        }
 
-       while (!lazy_queue_empty(queue)) {
-               struct commit *c = lazy_queue_get(queue);
+       while ((c = prio_queue_get(queue))) {
                struct commit_list *parents = c->parents;
                seen_commits++;
                if (c->object.flags & best->flag_within) {
@@ -321,7 +279,7 @@ static unsigned long finish_depth_computation(struct lazy_queue *queue,
                        repo_parse_commit(the_repository, p);
                        seen = p->object.flags & SEEN;
                        if (!seen)
-                               lazy_queue_put(queue, p);
+                               prio_queue_put(queue, p);
                        flag_before = p->object.flags & best->flag_within;
                        p->object.flags |= c->object.flags;
                        flag_after = p->object.flags & best->flag_within;
@@ -369,8 +327,8 @@ static void append_suffix(int depth, const struct object_id *oid, struct strbuf
 
 static void describe_commit(struct commit *cmit, struct strbuf *dst)
 {
-       struct commit *gave_up_on = NULL;
-       struct lazy_queue queue = LAZY_QUEUE_INIT;
+       struct commit *c, *gave_up_on = NULL;
+       struct prio_queue queue = { compare_commits_by_commit_date };
        struct commit_name *n;
        struct possible_tag all_matches[MAX_TAGS];
        unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
@@ -412,9 +370,8 @@ static void describe_commit(struct commit *cmit, struct strbuf *dst)
        }
 
        cmit->object.flags = SEEN;
-       lazy_queue_put(&queue, cmit);
-       while (!lazy_queue_empty(&queue)) {
-               struct commit *c = lazy_queue_get(&queue);
+       prio_queue_put(&queue, cmit);
+       while ((c = prio_queue_get(&queue))) {
                struct commit_list *parents = c->parents;
                struct commit_name **slot;
 
@@ -448,7 +405,7 @@ static void describe_commit(struct commit *cmit, struct strbuf *dst)
                                t->depth++;
                }
                /* Stop if last remaining path already covered by best candidate(s) */
-               if (annotated_cnt && lazy_queue_empty(&queue)) {
+               if (annotated_cnt && !prio_queue_size(&queue)) {
                        int best_depth = INT_MAX;
                        unsigned best_within = 0;
                        for (cur_match = 0; cur_match < match_cnt; cur_match++) {
@@ -471,7 +428,7 @@ static void describe_commit(struct commit *cmit, struct strbuf *dst)
                        struct commit *p = parents->item;
                        repo_parse_commit(the_repository, p);
                        if (!(p->object.flags & SEEN))
-                               lazy_queue_put(&queue, p);
+                               prio_queue_put(&queue, p);
                        p->object.flags |= c->object.flags;
                        parents = parents->next;
 
@@ -486,7 +443,7 @@ static void describe_commit(struct commit *cmit, struct strbuf *dst)
                        strbuf_add_unique_abbrev(dst, cmit_oid, abbrev);
                        if (suffix)
                                strbuf_addstr(dst, suffix);
-                       lazy_queue_clear(&queue);
+                       clear_prio_queue(&queue);
                        return;
                }
                if (unannotated_cnt)
@@ -502,11 +459,11 @@ static void describe_commit(struct commit *cmit, struct strbuf *dst)
        QSORT(all_matches, match_cnt, compare_pt);
 
        if (gave_up_on) {
-               lazy_queue_put(&queue, gave_up_on);
+               prio_queue_put(&queue, gave_up_on);
                seen_commits--;
        }
        seen_commits += finish_depth_computation(&queue, &all_matches[0]);
-       lazy_queue_clear(&queue);
+       clear_prio_queue(&queue);
 
        if (debug) {
                static int label_width = -1;
index 8846f2376fc2ff98e9cd776f585c01f36be21aea..2435e8aeda40ef8853970c768d579c407e805a86 100644 (file)
@@ -232,12 +232,13 @@ static void join_revs(struct prio_queue *queue,
        while ((commit = prio_queue_peek(queue))) {
                struct commit_list *parents;
                int still_interesting = !!interesting(queue);
-               bool get_pending = true;
                int flags = commit->object.flags & all_mask;
 
                if (!still_interesting && extra <= 0)
                        break;
 
+               prio_queue_get(queue);
+
                mark_seen(commit, seen_p);
                if ((flags & all_revs) == all_revs)
                        flags |= UNINTERESTING;
@@ -253,14 +254,8 @@ static void join_revs(struct prio_queue *queue,
                        if (mark_seen(p, seen_p) && !still_interesting)
                                extra--;
                        p->object.flags |= flags;
-                       if (get_pending)
-                               prio_queue_replace(queue, p);
-                       else
-                               prio_queue_put(queue, p);
-                       get_pending = false;
+                       prio_queue_put(queue, p);
                }
-               if (get_pending)
-                       prio_queue_get(queue);
        }
 
        /*
index fd8723502ed332958ed9f5ead62da4b4c6daa8ea..976bfc46186714bca5711fe59b2f0116cf471df1 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -795,24 +795,17 @@ void commit_list_sort_by_date(struct commit_list **list)
 struct commit *pop_most_recent_commit(struct prio_queue *queue,
                                      unsigned int mark)
 {
-       struct commit *ret = prio_queue_peek(queue);
-       int get_pending = 1;
+       struct commit *ret = prio_queue_get(queue);
        struct commit_list *parents = ret->parents;
 
        while (parents) {
                struct commit *commit = parents->item;
                if (!repo_parse_commit(the_repository, commit) && !(commit->object.flags & mark)) {
                        commit->object.flags |= mark;
-                       if (get_pending)
-                               prio_queue_replace(queue, commit);
-                       else
-                               prio_queue_put(queue, commit);
-                       get_pending = 0;
+                       prio_queue_put(queue, commit);
                }
                parents = parents->next;
        }
-       if (get_pending)
-               prio_queue_get(queue);
        return ret;
 }
 
index ead4faf4bbda4a9f0d54a00f536798037943c7b8..199775d5afd22a6a5a1fab180ee78f19816747b6 100644 (file)
@@ -34,12 +34,48 @@ void clear_prio_queue(struct prio_queue *queue)
        queue->nr_ = 0;
        queue->alloc = 0;
        queue->insertion_ctr = 0;
+       queue->get_pending = 0;
+}
+
+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);
+       }
+}
+
+static inline void flush_get(struct prio_queue *queue)
+{
+       if (!queue->get_pending)
+               return;
+       queue->get_pending = 0;
+       queue->array[0] = queue->array[--queue->nr_];
+       sift_down_root(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++;
+               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,61 +94,33 @@ 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;
-
-       if (!queue->nr_)
+       if (queue->nr_ <= queue->get_pending) {
+               queue->nr_ = 0;
+               queue->get_pending = 0;
                return NULL;
+       }
        if (!queue->compare)
                return queue->array[--queue->nr_].data; /* LIFO */
 
-       result = queue->array[0].data;
-       if (!--queue->nr_)
-               return result;
+       flush_get(queue);
 
-       queue->array[0] = queue->array[queue->nr_];
-       sift_down_root(queue);
-       return result;
+       queue->get_pending = 1;
+       return queue->array[0].data;
 }
 
 void *prio_queue_peek(struct prio_queue *queue)
 {
-       if (!queue->nr_)
+       if (queue->nr_ <= queue->get_pending) {
+               queue->nr_ = 0;
+               queue->get_pending = 0;
                return NULL;
+       }
        if (!queue->compare)
                return queue->array[queue->nr_ - 1].data;
-       return queue->array[0].data;
-}
 
-void prio_queue_replace(struct prio_queue *queue, void *thing)
-{
-       if (!queue->nr_) {
-               prio_queue_put(queue, thing);
-       } else if (!queue->compare) {
-               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);
-       }
+       flush_get(queue);
+
+       return queue->array[0].data;
 }
index 7f2aa986b1c7dbbceb59311680dfe763218574c4..570b48e6485b19863b9489b2b3d4367ba699f772 100644 (file)
@@ -30,8 +30,9 @@ struct prio_queue {
        prio_queue_compare_fn compare;
        size_t insertion_ctr;
        void *cb_data;
-       size_t alloc, nr_;
+       size_t alloc, nr_; /* use prio_queue_size() for logical count */
        struct prio_queue_entry *array;
+       unsigned get_pending;
 };
 
 /*
@@ -54,22 +55,14 @@ void *prio_queue_peek(struct prio_queue *);
 
 static inline size_t prio_queue_size(const struct prio_queue *queue)
 {
-       return queue->nr_;
+       return queue->nr_ - queue->get_pending;
 }
 
 #define prio_queue_for_each(queue, it) \
-       for (size_t pq_ix_ = 0; \
+       for (size_t pq_ix_ = (queue)->get_pending; \
             pq_ix_ < (queue)->nr_ && ((it) = (queue)->array[pq_ix_].data, 1); \
             pq_ix_++)
 
-/*
- * Replace the "thing" that compares the smallest with a new "thing",
- * like prio_queue_get()+prio_queue_put() would do, but in a more
- * efficient way.  Does the same as prio_queue_put() if the queue is
- * empty.
- */
-void prio_queue_replace(struct prio_queue *queue, void *thing);
-
 void clear_prio_queue(struct prio_queue *);
 
 /* Reverse the LIFO elements */
index 63e58114ae7dd52a60e0aa8f0aac6ef50517de61..af3e0b8598883238ef8e97a30093f27065b884a4 100644 (file)
@@ -53,13 +53,13 @@ static void test_prio_queue(int *input, size_t input_size,
                        prio_queue_reverse(&pq);
                        break;
                case REPLACE:
-                       peek = prio_queue_peek(&pq);
+                       get = prio_queue_get(&pq);
                        cl_assert(i + 1 < input_size);
                        cl_assert(input[i + 1] >= 0);
                        cl_assert(j < result_size);
-                       cl_assert_equal_i(result[j], show(peek));
+                       cl_assert_equal_i(result[j], show(get));
                        j++;
-                       prio_queue_replace(&pq, &input[++i]);
+                       prio_queue_put(&pq, &input[++i]);
                        break;
                default:
                        prio_queue_put(&pq, &input[i]);