Skip to content

Commit a43f270

Browse files
committed
commit-reach: skip STALE drain in paint_down_to_common when only one merge-base needed
When find_all is false and generation numbers are available, the priority queue pops in non-increasing generation order. The first doubly-painted commit is a valid best merge-base — no later commit can dominate it. Skip the expensive STALE drain in this case. Add find_all parameter to repo_get_merge_bases_many_dirty() and thread it through to paint_down_to_common(). git merge-base (without --all) passes show_all=0, triggering the early exit. On a 2.2M-commit merge-heavy monorepo with commit-graph: HEAD vs ~500: 5,229ms -> 24ms HEAD vs ~1000: 4,214ms -> 39ms HEAD vs ~5000: 3,799ms -> 46ms HEAD vs ~10000: 3,827ms -> 61ms Signed-off-by: Kristofer Karlsson <krka@spotify.com>
1 parent 94f0577 commit a43f270

4 files changed

Lines changed: 142 additions & 10 deletions

File tree

builtin/merge-base.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ static int show_merge_base(struct commit **rev, size_t rev_nr, int show_all)
1414
struct commit_list *result = NULL, *r;
1515

1616
if (repo_get_merge_bases_many_dirty(the_repository, rev[0],
17-
rev_nr - 1, rev + 1, &result) < 0) {
17+
rev_nr - 1, rev + 1,
18+
show_all, &result) < 0) {
1819
commit_list_free(result);
1920
return -1;
2021
}

commit-reach.c

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,16 @@ static int paint_down_to_common(struct repository *r,
5555
struct commit **twos,
5656
timestamp_t min_generation,
5757
int ignore_missing_commits,
58+
int find_all,
5859
struct commit_list **result)
5960
{
6061
struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
6162
int i;
63+
int has_gens = min_generation || corrected_commit_dates_enabled(r);
6264
timestamp_t last_gen = GENERATION_NUMBER_INFINITY;
6365
struct commit_list **tail = result;
6466

65-
if (!min_generation && !corrected_commit_dates_enabled(r))
67+
if (!has_gens)
6668
queue.compare = compare_commits_by_commit_date;
6769

6870
one->object.flags |= PARENT1;
@@ -97,6 +99,10 @@ static int paint_down_to_common(struct repository *r,
9799
if (!(commit->object.flags & RESULT)) {
98100
commit->object.flags |= RESULT;
99101
tail = commit_list_append(commit, tail);
102+
/* Generation-ordered queue: no later
103+
* commit can dominate this one. */
104+
if (!find_all && has_gens)
105+
break;
100106
}
101107
/* Mark parents of a found merge stale */
102108
flags |= STALE;
@@ -136,6 +142,7 @@ static int paint_down_to_common(struct repository *r,
136142
static int merge_bases_many(struct repository *r,
137143
struct commit *one, int n,
138144
struct commit **twos,
145+
int find_all,
139146
struct commit_list **result)
140147
{
141148
struct commit_list *list = NULL, **tail = result;
@@ -165,7 +172,7 @@ static int merge_bases_many(struct repository *r,
165172
oid_to_hex(&twos[i]->object.oid));
166173
}
167174

168-
if (paint_down_to_common(r, one, n, twos, 0, 0, &list)) {
175+
if (paint_down_to_common(r, one, n, twos, 0, 0, find_all, &list)) {
169176
commit_list_free(list);
170177
return -1;
171178
}
@@ -246,7 +253,7 @@ static int remove_redundant_no_gen(struct repository *r,
246253
min_generation = curr_generation;
247254
}
248255
if (paint_down_to_common(r, array[i], filled,
249-
work, min_generation, 0, &common)) {
256+
work, min_generation, 0, 1, &common)) {
250257
clear_commit_marks(array[i], all_flags);
251258
clear_commit_marks_many(filled, work, all_flags);
252259
commit_list_free(common);
@@ -425,14 +432,15 @@ static int get_merge_bases_many_0(struct repository *r,
425432
size_t n,
426433
struct commit **twos,
427434
int cleanup,
435+
int find_all,
428436
struct commit_list **result)
429437
{
430438
struct commit_list *list, **tail = result;
431439
struct commit **rslt;
432440
size_t cnt, i;
433441
int ret;
434442

435-
if (merge_bases_many(r, one, n, twos, result) < 0)
443+
if (merge_bases_many(r, one, n, twos, find_all, result) < 0)
436444
return -1;
437445
for (i = 0; i < n; i++) {
438446
if (one == twos[i])
@@ -475,24 +483,25 @@ int repo_get_merge_bases_many(struct repository *r,
475483
struct commit **twos,
476484
struct commit_list **result)
477485
{
478-
return get_merge_bases_many_0(r, one, n, twos, 1, result);
486+
return get_merge_bases_many_0(r, one, n, twos, 1, 1, result);
479487
}
480488

481489
int repo_get_merge_bases_many_dirty(struct repository *r,
482490
struct commit *one,
483491
size_t n,
484492
struct commit **twos,
493+
int find_all,
485494
struct commit_list **result)
486495
{
487-
return get_merge_bases_many_0(r, one, n, twos, 0, result);
496+
return get_merge_bases_many_0(r, one, n, twos, 0, find_all, result);
488497
}
489498

490499
int repo_get_merge_bases(struct repository *r,
491500
struct commit *one,
492501
struct commit *two,
493502
struct commit_list **result)
494503
{
495-
return get_merge_bases_many_0(r, one, 1, &two, 1, result);
504+
return get_merge_bases_many_0(r, one, 1, &two, 1, 1, result);
496505
}
497506

498507
/*
@@ -555,7 +564,7 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
555564

556565
if (paint_down_to_common(r, commit,
557566
nr_reference, reference,
558-
generation, ignore_missing_commits, &bases))
567+
generation, ignore_missing_commits, 1, &bases))
559568
ret = -1;
560569
else if (commit->object.flags & PARENT2)
561570
ret = 1;

commit-reach.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,13 @@ int repo_get_merge_bases_many(struct repository *r,
1717
struct commit *one, size_t n,
1818
struct commit **twos,
1919
struct commit_list **result);
20-
/* To be used only when object flags after this call no longer matter */
20+
/* To be used only when object flags after this call no longer matter.
21+
* When find_all is false and generation numbers are available, returns
22+
* after finding the first merge-base, skipping the STALE drain. */
2123
int repo_get_merge_bases_many_dirty(struct repository *r,
2224
struct commit *one, size_t n,
2325
struct commit **twos,
26+
int find_all,
2427
struct commit_list **result);
2528

2629
int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result);

t/t6010-merge-base.sh

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,4 +305,123 @@ test_expect_success 'merge-base --octopus --all for complex tree' '
305305
test_cmp expected actual
306306
'
307307

308+
# The following tests verify that "git merge-base" (without --all)
309+
# returns the same result with and without a commit-graph.
310+
# This exercises the early-exit optimisation in paint_down_to_common
311+
# that skips the STALE drain when generation numbers are available.
312+
313+
test_expect_success 'setup for commit-graph tests' '
314+
git init graph-repo &&
315+
(
316+
cd graph-repo &&
317+
318+
# Build a forked DAG:
319+
#
320+
# L1---L2 (left)
321+
# /
322+
# S
323+
# \
324+
# R1---R2 (right)
325+
#
326+
test_commit GS &&
327+
git checkout -b left &&
328+
test_commit L1 &&
329+
test_commit L2 &&
330+
git checkout GS &&
331+
git checkout -b right &&
332+
test_commit GR1 &&
333+
test_commit GR2
334+
)
335+
'
336+
337+
test_expect_success 'merge-base without commit-graph' '
338+
(
339+
cd graph-repo &&
340+
rm -f .git/objects/info/commit-graph &&
341+
git merge-base left right >actual &&
342+
git rev-parse GS >expected &&
343+
test_cmp expected actual
344+
)
345+
'
346+
347+
test_expect_success 'merge-base with commit-graph' '
348+
(
349+
cd graph-repo &&
350+
git commit-graph write --reachable &&
351+
git merge-base left right >actual &&
352+
git rev-parse GS >expected &&
353+
test_cmp expected actual
354+
)
355+
'
356+
357+
test_expect_success 'merge-base --all with commit-graph' '
358+
(
359+
cd graph-repo &&
360+
git merge-base --all left right >actual &&
361+
git rev-parse GS >expected &&
362+
test_cmp expected actual
363+
)
364+
'
365+
366+
test_expect_success 'merge-base agrees with --all for single result' '
367+
(
368+
cd graph-repo &&
369+
git commit-graph write --reachable &&
370+
git merge-base left right >actual.single &&
371+
git merge-base --all left right >actual.all &&
372+
test_cmp actual.all actual.single
373+
)
374+
'
375+
376+
test_expect_success 'setup for deep chain commit-graph test' '
377+
git init deep-repo &&
378+
(
379+
cd deep-repo &&
380+
381+
# Build a deep forked DAG:
382+
#
383+
# L1--L2--...--L20 (left)
384+
# /
385+
# S
386+
# \
387+
# R1--R2--...--R20 (right)
388+
#
389+
test_commit DS &&
390+
git checkout -b left &&
391+
for i in $(test_seq 1 20)
392+
do
393+
test_commit DL$i || return 1
394+
done &&
395+
git checkout DS &&
396+
git checkout -b right &&
397+
for i in $(test_seq 1 20)
398+
do
399+
test_commit DR$i || return 1
400+
done
401+
)
402+
'
403+
404+
test_expect_success 'deep chain: merge-base matches with and without commit-graph' '
405+
(
406+
cd deep-repo &&
407+
rm -f .git/objects/info/commit-graph &&
408+
git merge-base left right >actual.no-graph &&
409+
git rev-parse DS >expected &&
410+
test_cmp expected actual.no-graph &&
411+
git commit-graph write --reachable &&
412+
git merge-base left right >actual.graph &&
413+
test_cmp expected actual.graph
414+
)
415+
'
416+
417+
test_expect_success 'deep chain: --all and non---all agree with commit-graph' '
418+
(
419+
cd deep-repo &&
420+
git commit-graph write --reachable &&
421+
git merge-base left right >actual.single &&
422+
git merge-base --all left right >actual.all &&
423+
test_cmp actual.all actual.single
424+
)
425+
'
426+
308427
test_done

0 commit comments

Comments
 (0)