From d0f22946610492695d42b6f98368157625b246a2 Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Mon, 8 Jun 2026 19:51:22 +0200 Subject: [PATCH 1/2] prio-queue: rename .nr to .nr_ and add accessor helpers Rename the .nr member to .nr_ so that callers outside prio-queue.c that directly reference .nr get a compilation error. This catches both existing misuse and future in-flight topics. Add prio_queue_size() for callers that need to know the element count and prio_queue_for_each() for callers that need to walk all elements. Convert all external .nr users: - Loop conditions: use prio_queue_size(), prio_queue_get(), or prio_queue_peek() as the loop condition - Array iterations: use prio_queue_for_each() Signed-off-by: Kristofer Karlsson --- builtin/describe.c | 11 ++++++++--- builtin/last-modified.c | 7 +++---- builtin/show-branch.c | 13 ++++++------- commit-reach.c | 14 +++++++------- fetch-pack.c | 4 ++-- negotiator/default.c | 4 +++- negotiator/skipping.c | 12 +++++++----- object-name.c | 2 +- pack-bitmap-write.c | 10 +++++----- path-walk.c | 8 ++++---- prio-queue.c | 38 +++++++++++++++++++------------------- prio-queue.h | 12 +++++++++++- revision.c | 16 +++++++--------- walker.c | 4 ++-- 14 files changed, 85 insertions(+), 70 deletions(-) diff --git a/builtin/describe.c b/builtin/describe.c index 1c47d7c0b7c38d..8e88bdeea6cf1c 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -278,7 +278,7 @@ static void lazy_queue_put(struct lazy_queue *queue, void *thing) static bool lazy_queue_empty(const struct lazy_queue *queue) { - return queue->queue.nr == (queue->get_pending ? 1 : 0); + return prio_queue_size(&queue->queue) == (queue->get_pending ? 1 : 0); } static void lazy_queue_clear(struct lazy_queue *queue) @@ -292,9 +292,14 @@ static unsigned long finish_depth_computation(struct lazy_queue *queue, { unsigned long seen_commits = 0; struct oidset unflagged = OIDSET_INIT; + struct commit *commit; + int skip = queue->get_pending ? 1 : 0; - for (size_t i = queue->get_pending ? 1 : 0; i < queue->queue.nr; i++) { - struct commit *commit = queue->queue.array[i].data; + 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); } diff --git a/builtin/last-modified.c b/builtin/last-modified.c index 8900ceece1cdf3..5478182f2e95c2 100644 --- a/builtin/last-modified.c +++ b/builtin/last-modified.c @@ -344,6 +344,7 @@ static void process_parent(struct last_modified *lm, static int last_modified_run(struct last_modified *lm) { int max_count, queue_popped = 0; + struct commit *c, *n; struct prio_queue queue = { compare_commits_by_gen_then_commit_date }; struct prio_queue not_queue = { compare_commits_by_gen_then_commit_date }; struct commit_list *list; @@ -389,10 +390,9 @@ static int last_modified_run(struct last_modified *lm) } } - while (queue.nr) { + while ((c = prio_queue_get(&queue))) { int parent_i; struct commit_list *p; - struct commit *c = prio_queue_get(&queue); struct bitmap *active_c = active_paths_for(lm, c); if ((0 <= max_count && max_count < ++queue_popped) || @@ -416,9 +416,8 @@ static int last_modified_run(struct last_modified *lm) */ repo_parse_commit(lm->rev.repo, c); - while (not_queue.nr) { + while ((n = prio_queue_get(¬_queue))) { struct commit_list *np; - struct commit *n = prio_queue_get(¬_queue); repo_parse_commit(lm->rev.repo, n); diff --git a/builtin/show-branch.c b/builtin/show-branch.c index f02831b08500c4..8846f2376fc2ff 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -62,11 +62,10 @@ static const char *get_color_reset_code(void) static struct commit *interesting(struct prio_queue *queue) { - for (size_t i = 0; i < queue->nr; i++) { - struct commit *commit = queue->array[i].data; - if (commit->object.flags & UNINTERESTING) - continue; - return commit; + struct commit *commit; + prio_queue_for_each(queue, commit) { + if (!(commit->object.flags & UNINTERESTING)) + return commit; } return NULL; } @@ -228,11 +227,11 @@ static void join_revs(struct prio_queue *queue, { int all_mask = ((1u << (REV_SHIFT + num_rev)) - 1); int all_revs = all_mask & ~((1u << REV_SHIFT) - 1); + struct commit *commit; - while (queue->nr) { + while ((commit = prio_queue_peek(queue))) { struct commit_list *parents; int still_interesting = !!interesting(queue); - struct commit *commit = prio_queue_peek(queue); bool get_pending = true; int flags = commit->object.flags & all_mask; diff --git a/commit-reach.c b/commit-reach.c index 9b3ea46d6f2824..a849de653ef544 100644 --- a/commit-reach.c +++ b/commit-reach.c @@ -41,8 +41,8 @@ static int compare_commits_by_gen(const void *_a, const void *_b) static int queue_has_nonstale(struct prio_queue *queue) { - for (size_t i = 0; i < queue->nr; i++) { - struct commit *commit = queue->array[i].data; + struct commit *commit; + prio_queue_for_each(queue, commit) { if (!(commit->object.flags & STALE)) return 1; } @@ -1070,6 +1070,7 @@ void ahead_behind(struct repository *r, struct ahead_behind_count *counts, size_t counts_nr) { struct prio_queue queue = { .compare = compare_commits_by_gen_then_commit_date }; + void *entry; size_t width = DIV_ROUND_UP(commits_nr, BITS_IN_EWORD); if (!commits_nr || !counts_nr) @@ -1135,8 +1136,8 @@ void ahead_behind(struct repository *r, /* STALE is used here, PARENT2 is used by insert_no_dup(). */ repo_clear_commit_marks(r, PARENT2 | STALE); - for (size_t i = 0; i < queue.nr; i++) - free_bit_array(queue.array[i].data); + prio_queue_for_each(&queue, entry) + free_bit_array(entry); clear_bit_arrays(&bit_arrays); clear_prio_queue(&queue); } @@ -1269,7 +1270,7 @@ int get_branch_base_for_tip(struct repository *r, size_t bases_nr) { int best_index = -1; - struct commit *branch_point = NULL; + struct commit *c, *branch_point = NULL; struct prio_queue queue = { compare_commits_by_gen_then_commit_date }; int found_missing_gen = 0; @@ -1322,8 +1323,7 @@ int get_branch_base_for_tip(struct repository *r, prio_queue_put(&queue, c); } - while (queue.nr) { - struct commit *c = prio_queue_get(&queue); + while ((c = prio_queue_get(&queue))) { int best_for_c = get_best(c); int best_for_p, positive; struct commit *parent; diff --git a/fetch-pack.c b/fetch-pack.c index 120e01f3cf2674..29c41132ee0495 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -662,8 +662,8 @@ static int mark_complete_oid(const struct reference *ref, void *cb_data UNUSED) static void mark_recent_complete_commits(struct fetch_pack_args *args, timestamp_t cutoff) { - while (complete.nr) { - struct commit *item = prio_queue_peek(&complete); + struct commit *item; + while ((item = prio_queue_peek(&complete))) { if (item->date < cutoff) break; print_verbose(args, _("Marking %s as complete"), diff --git a/negotiator/default.c b/negotiator/default.c index 78d58d57cebbfb..19cdf3808cf266 100644 --- a/negotiator/default.c +++ b/negotiator/default.c @@ -113,10 +113,12 @@ static const struct object_id *get_rev(struct negotiation_state *ns) unsigned int mark; struct commit_list *parents; - if (ns->rev_list.nr == 0 || ns->non_common_revs == 0) + if (ns->non_common_revs == 0) return NULL; commit = prio_queue_get(&ns->rev_list); + if (!commit) + return NULL; repo_parse_commit(the_repository, commit); parents = commit->parents; diff --git a/negotiator/skipping.c b/negotiator/skipping.c index 68c9b3b997dc80..db90fa77b5325b 100644 --- a/negotiator/skipping.c +++ b/negotiator/skipping.c @@ -143,8 +143,7 @@ static int push_parent(struct data *data, struct entry *entry, /* * Find the existing entry and use it. */ - for (size_t i = 0; i < data->rev_list.nr; i++) { - parent_entry = data->rev_list.array[i].data; + prio_queue_for_each(&data->rev_list, parent_entry) { if (parent_entry->commit == to_push) goto parent_found; } @@ -181,10 +180,12 @@ static const struct object_id *get_rev(struct data *data) struct commit_list *p; int parent_pushed = 0; - if (data->rev_list.nr == 0 || data->non_common_revs == 0) + if (data->non_common_revs == 0) return NULL; entry = prio_queue_get(&data->rev_list); + if (!entry) + return NULL; commit = entry->commit; commit->object.flags |= POPPED; if (!(commit->object.flags & COMMON)) @@ -253,8 +254,9 @@ static void have_sent(struct fetch_negotiator *n, struct commit *c) static void release(struct fetch_negotiator *n) { struct data *data = n->data; - for (size_t i = 0; i < data->rev_list.nr; i++) - free(data->rev_list.array[i].data); + void *entry; + prio_queue_for_each(&data->rev_list, entry) + free(entry); clear_prio_queue(&data->rev_list); FREE_AND_NULL(data); } diff --git a/object-name.c b/object-name.c index 9ac86f19c77bbd..2fedfe1761016e 100644 --- a/object-name.c +++ b/object-name.c @@ -1208,7 +1208,7 @@ static int get_oid_oneline(struct repository *r, l->item->object.flags |= ONELINE_SEEN; prio_queue_put(©, l->item); } - while (copy.nr) { + while (prio_queue_size(©)) { const char *p, *buf; struct commit *commit; int matches; diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c index 1c8070f99c03ca..ed9714b135cf52 100644 --- a/pack-bitmap-write.c +++ b/pack-bitmap-write.c @@ -513,6 +513,8 @@ static int fill_bitmap_commit(struct bitmap_writer *writer, struct bitmap_index *old_bitmap, const uint32_t *mapping) { + struct commit *c; + struct tree *tree; int found; uint32_t pos; if (!ent->bitmap) @@ -520,9 +522,8 @@ static int fill_bitmap_commit(struct bitmap_writer *writer, prio_queue_put(queue, commit); - while (queue->nr) { + while ((c = prio_queue_get(queue))) { struct commit_list *p; - struct commit *c = prio_queue_get(queue); if (old_bitmap && mapping) { struct ewah_bitmap *old; @@ -574,9 +575,8 @@ static int fill_bitmap_commit(struct bitmap_writer *writer, } } - while (tree_queue->nr) { - if (fill_bitmap_tree(writer, ent->bitmap, - prio_queue_get(tree_queue)) < 0) + while ((tree = prio_queue_get(tree_queue))) { + if (fill_bitmap_tree(writer, ent->bitmap, tree) < 0) return -1; } return 0; diff --git a/path-walk.c b/path-walk.c index 94ff90bd1566b6..cf3b2d0765961b 100644 --- a/path-walk.c +++ b/path-walk.c @@ -699,6 +699,7 @@ int walk_objects_by_path(struct path_walk_info *info) int ret; size_t commits_nr = 0, paths_nr = 0; struct commit *c; + char *path; struct type_and_oid_list *root_tree_list; struct type_and_oid_list *commit_list; struct path_walk_context ctx = { @@ -808,8 +809,7 @@ int walk_objects_by_path(struct path_walk_info *info) free(commit_list); trace2_region_enter("path-walk", "path-walk", info->revs->repo); - while (!ret && ctx.path_stack.nr) { - char *path = prio_queue_get(&ctx.path_stack); + while (!ret && (path = prio_queue_get(&ctx.path_stack))) { paths_nr++; ret = walk_path(&ctx, path); @@ -821,12 +821,12 @@ int walk_objects_by_path(struct path_walk_info *info) if (!strmap_empty(&ctx.paths_to_lists)) { struct hashmap_iter iter; struct strmap_entry *entry; + char *path; strmap_for_each_entry(&ctx.paths_to_lists, &iter, entry) push_to_stack(&ctx, entry->key); - while (!ret && ctx.path_stack.nr) { - char *path = prio_queue_get(&ctx.path_stack); + while (!ret && (path = prio_queue_get(&ctx.path_stack))) { paths_nr++; ret = walk_path(&ctx, path); diff --git a/prio-queue.c b/prio-queue.c index 9748528ce6ecd6..ead4faf4bbda4a 100644 --- a/prio-queue.c +++ b/prio-queue.c @@ -22,16 +22,16 @@ void prio_queue_reverse(struct prio_queue *queue) if (queue->compare) BUG("prio_queue_reverse() on non-LIFO queue"); - if (!queue->nr) + if (!queue->nr_) return; - for (i = 0; i < (j = (queue->nr - 1) - i); i++) + for (i = 0; i < (j = (queue->nr_ - 1) - i); i++) swap(queue, i, j); } void clear_prio_queue(struct prio_queue *queue) { FREE_AND_NULL(queue->array); - queue->nr = 0; + queue->nr_ = 0; queue->alloc = 0; queue->insertion_ctr = 0; } @@ -41,15 +41,15 @@ void prio_queue_put(struct prio_queue *queue, void *thing) size_t ix, parent; /* Append at the end */ - ALLOC_GROW(queue->array, queue->nr + 1, queue->alloc); - queue->array[queue->nr].ctr = queue->insertion_ctr++; - queue->array[queue->nr].data = thing; - queue->nr++; + ALLOC_GROW(queue->array, queue->nr_ + 1, queue->alloc); + queue->array[queue->nr_].ctr = queue->insertion_ctr++; + queue->array[queue->nr_].data = thing; + queue->nr_++; if (!queue->compare) return; /* LIFO */ /* Bubble up the new one */ - for (ix = queue->nr - 1; ix; ix = parent) { + for (ix = queue->nr_ - 1; ix; ix = parent) { parent = (ix - 1) / 2; if (compare(queue, parent, ix) <= 0) break; @@ -63,9 +63,9 @@ 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) { + for (ix = 0; ix * 2 + 1 < queue->nr_; ix = child) { child = ix * 2 + 1; /* left */ - if (child + 1 < queue->nr && + if (child + 1 < queue->nr_ && compare(queue, child, child + 1) >= 0) child++; /* use right child */ @@ -80,36 +80,36 @@ void *prio_queue_get(struct prio_queue *queue) { void *result; - if (!queue->nr) + if (!queue->nr_) return NULL; if (!queue->compare) - return queue->array[--queue->nr].data; /* LIFO */ + return queue->array[--queue->nr_].data; /* LIFO */ result = queue->array[0].data; - if (!--queue->nr) + if (!--queue->nr_) return result; - queue->array[0] = queue->array[queue->nr]; + queue->array[0] = queue->array[queue->nr_]; sift_down_root(queue); return result; } void *prio_queue_peek(struct prio_queue *queue) { - if (!queue->nr) + if (!queue->nr_) return NULL; if (!queue->compare) - return queue->array[queue->nr - 1].data; + 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) { + 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; + 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; diff --git a/prio-queue.h b/prio-queue.h index da7fad2f1f408f..7f2aa986b1c7db 100644 --- a/prio-queue.h +++ b/prio-queue.h @@ -30,7 +30,7 @@ struct prio_queue { prio_queue_compare_fn compare; size_t insertion_ctr; void *cb_data; - size_t alloc, nr; + size_t alloc, nr_; struct prio_queue_entry *array; }; @@ -52,6 +52,16 @@ void *prio_queue_get(struct prio_queue *); */ void *prio_queue_peek(struct prio_queue *); +static inline size_t prio_queue_size(const struct prio_queue *queue) +{ + return queue->nr_; +} + +#define prio_queue_for_each(queue, it) \ + for (size_t pq_ix_ = 0; \ + 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 diff --git a/revision.c b/revision.c index 5693618be4ec81..34e2d146f4e09d 100644 --- a/revision.c +++ b/revision.c @@ -476,16 +476,15 @@ static struct commit *handle_commit(struct rev_info *revs, static int everybody_uninteresting(struct prio_queue *orig, struct commit **interesting_cache) { - size_t i; + struct commit *commit; if (*interesting_cache) { - struct commit *commit = *interesting_cache; + commit = *interesting_cache; if (!(commit->object.flags & UNINTERESTING)) return 0; } - for (i = 0; i < orig->nr; i++) { - struct commit *commit = orig->array[i].data; + prio_queue_for_each(orig, commit) { if (commit->object.flags & UNINTERESTING) continue; @@ -1446,7 +1445,7 @@ static int limit_list(struct rev_info *revs) struct commit_list *original_list = revs->commits; struct commit_list *newlist = NULL; struct commit_list **p = &newlist; - struct commit *interesting_cache = NULL; + struct commit *commit, *interesting_cache = NULL; struct prio_queue queue = { .compare = compare_commits_by_commit_date }; if (revs->ancestry_path_implicit_bottoms) { @@ -1461,8 +1460,7 @@ static int limit_list(struct rev_info *revs) prio_queue_put(&queue, commit); } - while (queue.nr) { - struct commit *commit = prio_queue_get(&queue); + while ((commit = prio_queue_get(&queue))) { struct object *obj = &commit->object; if (commit == interesting_cache) @@ -4028,8 +4026,8 @@ static enum rewrite_result rewrite_one_1(struct rev_info *revs, static void merge_queue_into_list(struct prio_queue *q, struct commit_list **list) { - while (q->nr) { - struct commit *item = prio_queue_peek(q); + struct commit *item; + while ((item = prio_queue_peek(q))) { struct commit_list *p = *list; if (p && p->item->date >= item->date) diff --git a/walker.c b/walker.c index e98eb6da53692e..e3de77f0925082 100644 --- a/walker.c +++ b/walker.c @@ -84,12 +84,12 @@ static struct prio_queue complete = { compare_commits_by_commit_date }; static int process_commit(struct walker *walker, struct commit *commit) { struct commit_list *parents; + struct commit *item; if (repo_parse_commit(the_repository, commit)) return -1; - while (complete.nr) { - struct commit *item = prio_queue_peek(&complete); + while ((item = prio_queue_peek(&complete))) { if (item->date < commit->date) break; pop_most_recent_commit(&complete, COMPLETE); From a3f4cb57f2f8f90a8b93123b1b10b17a5be8bbb8 Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Mon, 8 Jun 2026 20:20:08 +0200 Subject: [PATCH 2/2] prio-queue: fold lazy_queue into prio_queue for automatic get+put fusion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Kristofer Karlsson --- builtin/describe.c | 75 +++++++----------------------- builtin/show-branch.c | 11 ++--- commit.c | 11 +---- prio-queue.c | 92 ++++++++++++++++++++----------------- prio-queue.h | 15 ++---- t/unit-tests/u-prio-queue.c | 6 +-- 6 files changed, 78 insertions(+), 132 deletions(-) diff --git a/builtin/describe.c b/builtin/describe.c index 8e88bdeea6cf1c..64424543ef229c 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -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; diff --git a/builtin/show-branch.c b/builtin/show-branch.c index 8846f2376fc2ff..2435e8aeda40ef 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -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); } /* diff --git a/commit.c b/commit.c index fd8723502ed332..976bfc46186714 100644 --- 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; } diff --git a/prio-queue.c b/prio-queue.c index ead4faf4bbda4a..199775d5afd22a 100644 --- a/prio-queue.c +++ b/prio-queue.c @@ -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; } diff --git a/prio-queue.h b/prio-queue.h index 7f2aa986b1c7db..570b48e6485b19 100644 --- a/prio-queue.h +++ b/prio-queue.h @@ -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 */ diff --git a/t/unit-tests/u-prio-queue.c b/t/unit-tests/u-prio-queue.c index 63e58114ae7dd5..af3e0b85988832 100644 --- a/t/unit-tests/u-prio-queue.c +++ b/t/unit-tests/u-prio-queue.c @@ -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]);