Skip to content

Commit 503176b

Browse files
committed
prio-queue: fold lazy_queue into prio_queue for automatic get+put fusion
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 08bb69d) 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>
1 parent d3ee883 commit 503176b

6 files changed

Lines changed: 78 additions & 132 deletions

File tree

builtin/describe.c

Lines changed: 16 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -251,61 +251,19 @@ static int compare_pt(const void *a_, const void *b_)
251251
return 0;
252252
}
253253

254-
struct lazy_queue {
255-
struct prio_queue queue;
256-
bool get_pending;
257-
};
258-
259-
#define LAZY_QUEUE_INIT { { compare_commits_by_commit_date }, false }
260-
261-
static void *lazy_queue_get(struct lazy_queue *queue)
262-
{
263-
if (queue->get_pending)
264-
prio_queue_get(&queue->queue);
265-
else
266-
queue->get_pending = true;
267-
return prio_queue_peek(&queue->queue);
268-
}
269-
270-
static void lazy_queue_put(struct lazy_queue *queue, void *thing)
271-
{
272-
if (queue->get_pending)
273-
prio_queue_replace(&queue->queue, thing);
274-
else
275-
prio_queue_put(&queue->queue, thing);
276-
queue->get_pending = false;
277-
}
278-
279-
static bool lazy_queue_empty(const struct lazy_queue *queue)
280-
{
281-
return prio_queue_size(&queue->queue) == (queue->get_pending ? 1 : 0);
282-
}
283-
284-
static void lazy_queue_clear(struct lazy_queue *queue)
285-
{
286-
clear_prio_queue(&queue->queue);
287-
queue->get_pending = false;
288-
}
289-
290-
static unsigned long finish_depth_computation(struct lazy_queue *queue,
254+
static unsigned long finish_depth_computation(struct prio_queue *queue,
291255
struct possible_tag *best)
292256
{
293257
unsigned long seen_commits = 0;
294258
struct oidset unflagged = OIDSET_INIT;
295-
struct commit *commit;
296-
int skip = queue->get_pending ? 1 : 0;
259+
struct commit *c;
297260

298-
prio_queue_for_each(&queue->queue, commit) {
299-
if (skip) {
300-
skip = 0;
301-
continue;
302-
}
303-
if (!(commit->object.flags & best->flag_within))
304-
oidset_insert(&unflagged, &commit->object.oid);
261+
prio_queue_for_each(queue, c) {
262+
if (!(c->object.flags & best->flag_within))
263+
oidset_insert(&unflagged, &c->object.oid);
305264
}
306265

307-
while (!lazy_queue_empty(queue)) {
308-
struct commit *c = lazy_queue_get(queue);
266+
while ((c = prio_queue_get(queue))) {
309267
struct commit_list *parents = c->parents;
310268
seen_commits++;
311269
if (c->object.flags & best->flag_within) {
@@ -321,7 +279,7 @@ static unsigned long finish_depth_computation(struct lazy_queue *queue,
321279
repo_parse_commit(the_repository, p);
322280
seen = p->object.flags & SEEN;
323281
if (!seen)
324-
lazy_queue_put(queue, p);
282+
prio_queue_put(queue, p);
325283
flag_before = p->object.flags & best->flag_within;
326284
p->object.flags |= c->object.flags;
327285
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
369327

370328
static void describe_commit(struct commit *cmit, struct strbuf *dst)
371329
{
372-
struct commit *gave_up_on = NULL;
373-
struct lazy_queue queue = LAZY_QUEUE_INIT;
330+
struct commit *c, *gave_up_on = NULL;
331+
struct prio_queue queue = { compare_commits_by_commit_date };
374332
struct commit_name *n;
375333
struct possible_tag all_matches[MAX_TAGS];
376334
unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
@@ -412,9 +370,8 @@ static void describe_commit(struct commit *cmit, struct strbuf *dst)
412370
}
413371

414372
cmit->object.flags = SEEN;
415-
lazy_queue_put(&queue, cmit);
416-
while (!lazy_queue_empty(&queue)) {
417-
struct commit *c = lazy_queue_get(&queue);
373+
prio_queue_put(&queue, cmit);
374+
while ((c = prio_queue_get(&queue))) {
418375
struct commit_list *parents = c->parents;
419376
struct commit_name **slot;
420377

@@ -448,7 +405,7 @@ static void describe_commit(struct commit *cmit, struct strbuf *dst)
448405
t->depth++;
449406
}
450407
/* Stop if last remaining path already covered by best candidate(s) */
451-
if (annotated_cnt && lazy_queue_empty(&queue)) {
408+
if (annotated_cnt && !prio_queue_size(&queue)) {
452409
int best_depth = INT_MAX;
453410
unsigned best_within = 0;
454411
for (cur_match = 0; cur_match < match_cnt; cur_match++) {
@@ -471,7 +428,7 @@ static void describe_commit(struct commit *cmit, struct strbuf *dst)
471428
struct commit *p = parents->item;
472429
repo_parse_commit(the_repository, p);
473430
if (!(p->object.flags & SEEN))
474-
lazy_queue_put(&queue, p);
431+
prio_queue_put(&queue, p);
475432
p->object.flags |= c->object.flags;
476433
parents = parents->next;
477434

@@ -486,7 +443,7 @@ static void describe_commit(struct commit *cmit, struct strbuf *dst)
486443
strbuf_add_unique_abbrev(dst, cmit_oid, abbrev);
487444
if (suffix)
488445
strbuf_addstr(dst, suffix);
489-
lazy_queue_clear(&queue);
446+
clear_prio_queue(&queue);
490447
return;
491448
}
492449
if (unannotated_cnt)
@@ -502,11 +459,11 @@ static void describe_commit(struct commit *cmit, struct strbuf *dst)
502459
QSORT(all_matches, match_cnt, compare_pt);
503460

504461
if (gave_up_on) {
505-
lazy_queue_put(&queue, gave_up_on);
462+
prio_queue_put(&queue, gave_up_on);
506463
seen_commits--;
507464
}
508465
seen_commits += finish_depth_computation(&queue, &all_matches[0]);
509-
lazy_queue_clear(&queue);
466+
clear_prio_queue(&queue);
510467

511468
if (debug) {
512469
static int label_width = -1;

builtin/show-branch.c

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -232,12 +232,13 @@ static void join_revs(struct prio_queue *queue,
232232
while ((commit = prio_queue_peek(queue))) {
233233
struct commit_list *parents;
234234
int still_interesting = !!interesting(queue);
235-
bool get_pending = true;
236235
int flags = commit->object.flags & all_mask;
237236

238237
if (!still_interesting && extra <= 0)
239238
break;
240239

240+
prio_queue_get(queue);
241+
241242
mark_seen(commit, seen_p);
242243
if ((flags & all_revs) == all_revs)
243244
flags |= UNINTERESTING;
@@ -253,14 +254,8 @@ static void join_revs(struct prio_queue *queue,
253254
if (mark_seen(p, seen_p) && !still_interesting)
254255
extra--;
255256
p->object.flags |= flags;
256-
if (get_pending)
257-
prio_queue_replace(queue, p);
258-
else
259-
prio_queue_put(queue, p);
260-
get_pending = false;
257+
prio_queue_put(queue, p);
261258
}
262-
if (get_pending)
263-
prio_queue_get(queue);
264259
}
265260

266261
/*

commit.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -795,24 +795,17 @@ void commit_list_sort_by_date(struct commit_list **list)
795795
struct commit *pop_most_recent_commit(struct prio_queue *queue,
796796
unsigned int mark)
797797
{
798-
struct commit *ret = prio_queue_peek(queue);
799-
int get_pending = 1;
798+
struct commit *ret = prio_queue_get(queue);
800799
struct commit_list *parents = ret->parents;
801800

802801
while (parents) {
803802
struct commit *commit = parents->item;
804803
if (!repo_parse_commit(the_repository, commit) && !(commit->object.flags & mark)) {
805804
commit->object.flags |= mark;
806-
if (get_pending)
807-
prio_queue_replace(queue, commit);
808-
else
809-
prio_queue_put(queue, commit);
810-
get_pending = 0;
805+
prio_queue_put(queue, commit);
811806
}
812807
parents = parents->next;
813808
}
814-
if (get_pending)
815-
prio_queue_get(queue);
816809
return ret;
817810
}
818811

prio-queue.c

Lines changed: 50 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,48 @@ void clear_prio_queue(struct prio_queue *queue)
3434
queue->nr_ = 0;
3535
queue->alloc = 0;
3636
queue->insertion_ctr = 0;
37+
queue->get_pending = 0;
38+
}
39+
40+
static void sift_down_root(struct prio_queue *queue)
41+
{
42+
size_t ix, child;
43+
44+
/* Push down the one at the root */
45+
for (ix = 0; ix * 2 + 1 < queue->nr_; ix = child) {
46+
child = ix * 2 + 1; /* left */
47+
if (child + 1 < queue->nr_ &&
48+
compare(queue, child, child + 1) >= 0)
49+
child++; /* use right child */
50+
51+
if (compare(queue, ix, child) <= 0)
52+
break;
53+
54+
swap(queue, child, ix);
55+
}
56+
}
57+
58+
static inline void flush_get(struct prio_queue *queue)
59+
{
60+
if (!queue->get_pending)
61+
return;
62+
queue->get_pending = 0;
63+
queue->array[0] = queue->array[--queue->nr_];
64+
sift_down_root(queue);
3765
}
3866

3967
void prio_queue_put(struct prio_queue *queue, void *thing)
4068
{
4169
size_t ix, parent;
4270

71+
if (queue->get_pending) {
72+
queue->get_pending = 0;
73+
queue->array[0].ctr = queue->insertion_ctr++;
74+
queue->array[0].data = thing;
75+
sift_down_root(queue);
76+
return;
77+
}
78+
4379
/* Append at the end */
4480
ALLOC_GROW(queue->array, queue->nr_ + 1, queue->alloc);
4581
queue->array[queue->nr_].ctr = queue->insertion_ctr++;
@@ -58,61 +94,33 @@ void prio_queue_put(struct prio_queue *queue, void *thing)
5894
}
5995
}
6096

61-
static void sift_down_root(struct prio_queue *queue)
62-
{
63-
size_t ix, child;
64-
65-
/* Push down the one at the root */
66-
for (ix = 0; ix * 2 + 1 < queue->nr_; ix = child) {
67-
child = ix * 2 + 1; /* left */
68-
if (child + 1 < queue->nr_ &&
69-
compare(queue, child, child + 1) >= 0)
70-
child++; /* use right child */
71-
72-
if (compare(queue, ix, child) <= 0)
73-
break;
74-
75-
swap(queue, child, ix);
76-
}
77-
}
78-
7997
void *prio_queue_get(struct prio_queue *queue)
8098
{
81-
void *result;
82-
83-
if (!queue->nr_)
99+
if (queue->nr_ <= queue->get_pending) {
100+
queue->nr_ = 0;
101+
queue->get_pending = 0;
84102
return NULL;
103+
}
85104
if (!queue->compare)
86105
return queue->array[--queue->nr_].data; /* LIFO */
87106

88-
result = queue->array[0].data;
89-
if (!--queue->nr_)
90-
return result;
107+
flush_get(queue);
91108

92-
queue->array[0] = queue->array[queue->nr_];
93-
sift_down_root(queue);
94-
return result;
109+
queue->get_pending = 1;
110+
return queue->array[0].data;
95111
}
96112

97113
void *prio_queue_peek(struct prio_queue *queue)
98114
{
99-
if (!queue->nr_)
115+
if (queue->nr_ <= queue->get_pending) {
116+
queue->nr_ = 0;
117+
queue->get_pending = 0;
100118
return NULL;
119+
}
101120
if (!queue->compare)
102121
return queue->array[queue->nr_ - 1].data;
103-
return queue->array[0].data;
104-
}
105122

106-
void prio_queue_replace(struct prio_queue *queue, void *thing)
107-
{
108-
if (!queue->nr_) {
109-
prio_queue_put(queue, thing);
110-
} else if (!queue->compare) {
111-
queue->array[queue->nr_ - 1].ctr = queue->insertion_ctr++;
112-
queue->array[queue->nr_ - 1].data = thing;
113-
} else {
114-
queue->array[0].ctr = queue->insertion_ctr++;
115-
queue->array[0].data = thing;
116-
sift_down_root(queue);
117-
}
123+
flush_get(queue);
124+
125+
return queue->array[0].data;
118126
}

prio-queue.h

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@ struct prio_queue {
3030
prio_queue_compare_fn compare;
3131
size_t insertion_ctr;
3232
void *cb_data;
33-
size_t alloc, nr_;
33+
size_t alloc, nr_; /* use prio_queue_size() for logical count */
3434
struct prio_queue_entry *array;
35+
unsigned get_pending;
3536
};
3637

3738
/*
@@ -54,22 +55,14 @@ void *prio_queue_peek(struct prio_queue *);
5455

5556
static inline size_t prio_queue_size(const struct prio_queue *queue)
5657
{
57-
return queue->nr_;
58+
return queue->nr_ - queue->get_pending;
5859
}
5960

6061
#define prio_queue_for_each(queue, it) \
61-
for (size_t pq_ix_ = 0; \
62+
for (size_t pq_ix_ = (queue)->get_pending; \
6263
pq_ix_ < (queue)->nr_ && ((it) = (queue)->array[pq_ix_].data, 1); \
6364
pq_ix_++)
6465

65-
/*
66-
* Replace the "thing" that compares the smallest with a new "thing",
67-
* like prio_queue_get()+prio_queue_put() would do, but in a more
68-
* efficient way. Does the same as prio_queue_put() if the queue is
69-
* empty.
70-
*/
71-
void prio_queue_replace(struct prio_queue *queue, void *thing);
72-
7366
void clear_prio_queue(struct prio_queue *);
7467

7568
/* Reverse the LIFO elements */

t/unit-tests/u-prio-queue.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@ static void test_prio_queue(int *input, size_t input_size,
5353
prio_queue_reverse(&pq);
5454
break;
5555
case REPLACE:
56-
peek = prio_queue_peek(&pq);
56+
get = prio_queue_get(&pq);
5757
cl_assert(i + 1 < input_size);
5858
cl_assert(input[i + 1] >= 0);
5959
cl_assert(j < result_size);
60-
cl_assert_equal_i(result[j], show(peek));
60+
cl_assert_equal_i(result[j], show(get));
6161
j++;
62-
prio_queue_replace(&pq, &input[++i]);
62+
prio_queue_put(&pq, &input[++i]);
6363
break;
6464
default:
6565
prio_queue_put(&pq, &input[i]);

0 commit comments

Comments
 (0)