Skip to content

Commit f7b5c26

Browse files
spkrkaspkrka
committed
commit-reach: early exit paint_down_to_common for single merge-base
Commits not in the commit-graph get GENERATION_NUMBER_INFINITY and sort to the top of the priority queue. After those, commits with finite generation numbers are popped in non-increasing order. When find_all is false the first doubly-painted commit with a finite generation is therefore a best merge-base: no commit still in the queue can be a descendant of 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 f7b5c26

5 files changed

Lines changed: 186 additions & 9 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: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ 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 };
@@ -97,6 +98,14 @@ static int paint_down_to_common(struct repository *r,
9798
if (!(commit->object.flags & RESULT)) {
9899
commit->object.flags |= RESULT;
99100
tail = commit_list_append(commit, tail);
101+
/*
102+
* The queue is generation-ordered; no
103+
* remaining common ancestor can be a
104+
* descendant of this one.
105+
*/
106+
if (!find_all &&
107+
generation < GENERATION_NUMBER_INFINITY)
108+
break;
100109
}
101110
/* Mark parents of a found merge stale */
102111
flags |= STALE;
@@ -136,6 +145,7 @@ static int paint_down_to_common(struct repository *r,
136145
static int merge_bases_many(struct repository *r,
137146
struct commit *one, int n,
138147
struct commit **twos,
148+
int find_all,
139149
struct commit_list **result)
140150
{
141151
struct commit_list *list = NULL, **tail = result;
@@ -165,7 +175,7 @@ static int merge_bases_many(struct repository *r,
165175
oid_to_hex(&twos[i]->object.oid));
166176
}
167177

168-
if (paint_down_to_common(r, one, n, twos, 0, 0, &list)) {
178+
if (paint_down_to_common(r, one, n, twos, 0, 0, find_all, &list)) {
169179
commit_list_free(list);
170180
return -1;
171181
}
@@ -246,7 +256,7 @@ static int remove_redundant_no_gen(struct repository *r,
246256
min_generation = curr_generation;
247257
}
248258
if (paint_down_to_common(r, array[i], filled,
249-
work, min_generation, 0, &common)) {
259+
work, min_generation, 0, 1, &common)) {
250260
clear_commit_marks(array[i], all_flags);
251261
clear_commit_marks_many(filled, work, all_flags);
252262
commit_list_free(common);
@@ -425,14 +435,15 @@ static int get_merge_bases_many_0(struct repository *r,
425435
size_t n,
426436
struct commit **twos,
427437
int cleanup,
438+
int find_all,
428439
struct commit_list **result)
429440
{
430441
struct commit_list *list, **tail = result;
431442
struct commit **rslt;
432443
size_t cnt, i;
433444
int ret;
434445

435-
if (merge_bases_many(r, one, n, twos, result) < 0)
446+
if (merge_bases_many(r, one, n, twos, find_all, result) < 0)
436447
return -1;
437448
for (i = 0; i < n; i++) {
438449
if (one == twos[i])
@@ -475,24 +486,25 @@ int repo_get_merge_bases_many(struct repository *r,
475486
struct commit **twos,
476487
struct commit_list **result)
477488
{
478-
return get_merge_bases_many_0(r, one, n, twos, 1, result);
489+
return get_merge_bases_many_0(r, one, n, twos, 1, 1, result);
479490
}
480491

481492
int repo_get_merge_bases_many_dirty(struct repository *r,
482493
struct commit *one,
483494
size_t n,
484495
struct commit **twos,
496+
int find_all,
485497
struct commit_list **result)
486498
{
487-
return get_merge_bases_many_0(r, one, n, twos, 0, result);
499+
return get_merge_bases_many_0(r, one, n, twos, 0, find_all, result);
488500
}
489501

490502
int repo_get_merge_bases(struct repository *r,
491503
struct commit *one,
492504
struct commit *two,
493505
struct commit_list **result)
494506
{
495-
return get_merge_bases_many_0(r, one, 1, &two, 1, result);
507+
return get_merge_bases_many_0(r, one, 1, &two, 1, 1, result);
496508
}
497509

498510
/*
@@ -555,7 +567,7 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
555567

556568
if (paint_down_to_common(r, commit,
557569
nr_reference, reference,
558-
generation, ignore_missing_commits, &bases))
570+
generation, ignore_missing_commits, 1, &bases))
559571
ret = -1;
560572
else if (commit->object.flags & PARENT2)
561573
ret = 1;

‎commit-reach.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,15 @@ 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+
/*
21+
* To be used only when object flags after this call no longer matter.
22+
* When find_all is false and generation numbers are available, returns
23+
* after finding the first merge-base, skipping the STALE drain.
24+
*/
2125
int repo_get_merge_bases_many_dirty(struct repository *r,
2226
struct commit *one, size_t n,
2327
struct commit **twos,
28+
int find_all,
2429
struct commit_list **result);
2530

2631
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

‎t/t6600-test-reach.sh‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -882,4 +882,44 @@ test_expect_success 'rev-list --maximal-only matches merge-base --independent' '
882882
test_cmp expect.sorted actual.sorted
883883
'
884884

885+
# The following tests verify the early-exit optimisation in
886+
# paint_down_to_common when merge-base is invoked without --all.
887+
# Each test checks all four commit-graph configurations.
888+
889+
merge_base_all_modes () {
890+
test_when_finished rm -rf .git/objects/info/commit-graph &&
891+
git merge-base "$@" >actual &&
892+
test_cmp expect actual &&
893+
cp commit-graph-full .git/objects/info/commit-graph &&
894+
git merge-base "$@" >actual &&
895+
test_cmp expect actual &&
896+
cp commit-graph-half .git/objects/info/commit-graph &&
897+
git merge-base "$@" >actual &&
898+
test_cmp expect actual &&
899+
cp commit-graph-no-gdat .git/objects/info/commit-graph &&
900+
git merge-base "$@" >actual &&
901+
test_cmp expect actual
902+
}
903+
904+
test_expect_success 'merge-base without --all (unique base)' '
905+
git rev-parse commit-5-3 >expect &&
906+
merge_base_all_modes commit-5-7 commit-8-3
907+
'
908+
909+
test_expect_success 'merge-base without --all is one of --all results' '
910+
test_when_finished rm -rf .git/objects/info/commit-graph &&
911+
912+
cp commit-graph-full .git/objects/info/commit-graph &&
913+
git merge-base --all commit-5-7 commit-4-8 commit-6-6 commit-8-3 >all &&
914+
git merge-base commit-5-7 commit-4-8 commit-6-6 commit-8-3 >single &&
915+
test_line_count = 1 single &&
916+
grep -F -f single all &&
917+
918+
cp commit-graph-half .git/objects/info/commit-graph &&
919+
git merge-base --all commit-5-7 commit-4-8 commit-6-6 commit-8-3 >all &&
920+
git merge-base commit-5-7 commit-4-8 commit-6-6 commit-8-3 >single &&
921+
test_line_count = 1 single &&
922+
grep -F -f single all
923+
'
924+
885925
test_done

0 commit comments

Comments
 (0)