Skip to content

Commit 54cdf9b

Browse files
spkrkaspkrka
committed
commit-reach: early exit paint_down_to_common for single merge-base
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. The early exit is guarded by three conditions: find_all must be false, the commit-graph must provide generation numbers, and the merge-base commit itself must have a finite generation (not GENERATION_NUMBER_INFINITY from being outside the commit-graph). 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 54cdf9b

5 files changed

Lines changed: 183 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: 18 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,11 @@ 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+
generation < GENERATION_NUMBER_INFINITY)
106+
break;
100107
}
101108
/* Mark parents of a found merge stale */
102109
flags |= STALE;
@@ -136,6 +143,7 @@ static int paint_down_to_common(struct repository *r,
136143
static int merge_bases_many(struct repository *r,
137144
struct commit *one, int n,
138145
struct commit **twos,
146+
int find_all,
139147
struct commit_list **result)
140148
{
141149
struct commit_list *list = NULL, **tail = result;
@@ -165,7 +173,7 @@ static int merge_bases_many(struct repository *r,
165173
oid_to_hex(&twos[i]->object.oid));
166174
}
167175

168-
if (paint_down_to_common(r, one, n, twos, 0, 0, &list)) {
176+
if (paint_down_to_common(r, one, n, twos, 0, 0, find_all, &list)) {
169177
commit_list_free(list);
170178
return -1;
171179
}
@@ -246,7 +254,7 @@ static int remove_redundant_no_gen(struct repository *r,
246254
min_generation = curr_generation;
247255
}
248256
if (paint_down_to_common(r, array[i], filled,
249-
work, min_generation, 0, &common)) {
257+
work, min_generation, 0, 1, &common)) {
250258
clear_commit_marks(array[i], all_flags);
251259
clear_commit_marks_many(filled, work, all_flags);
252260
commit_list_free(common);
@@ -425,14 +433,15 @@ static int get_merge_bases_many_0(struct repository *r,
425433
size_t n,
426434
struct commit **twos,
427435
int cleanup,
436+
int find_all,
428437
struct commit_list **result)
429438
{
430439
struct commit_list *list, **tail = result;
431440
struct commit **rslt;
432441
size_t cnt, i;
433442
int ret;
434443

435-
if (merge_bases_many(r, one, n, twos, result) < 0)
444+
if (merge_bases_many(r, one, n, twos, find_all, result) < 0)
436445
return -1;
437446
for (i = 0; i < n; i++) {
438447
if (one == twos[i])
@@ -475,24 +484,25 @@ int repo_get_merge_bases_many(struct repository *r,
475484
struct commit **twos,
476485
struct commit_list **result)
477486
{
478-
return get_merge_bases_many_0(r, one, n, twos, 1, result);
487+
return get_merge_bases_many_0(r, one, n, twos, 1, 1, result);
479488
}
480489

481490
int repo_get_merge_bases_many_dirty(struct repository *r,
482491
struct commit *one,
483492
size_t n,
484493
struct commit **twos,
494+
int find_all,
485495
struct commit_list **result)
486496
{
487-
return get_merge_bases_many_0(r, one, n, twos, 0, result);
497+
return get_merge_bases_many_0(r, one, n, twos, 0, find_all, result);
488498
}
489499

490500
int repo_get_merge_bases(struct repository *r,
491501
struct commit *one,
492502
struct commit *two,
493503
struct commit_list **result)
494504
{
495-
return get_merge_bases_many_0(r, one, 1, &two, 1, result);
505+
return get_merge_bases_many_0(r, one, 1, &two, 1, 1, result);
496506
}
497507

498508
/*
@@ -555,7 +565,7 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,
555565

556566
if (paint_down_to_common(r, commit,
557567
nr_reference, reference,
558-
generation, ignore_missing_commits, &bases))
568+
generation, ignore_missing_commits, 1, &bases))
559569
ret = -1;
560570
else if (commit->object.flags & PARENT2)
561571
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

‎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)