Skip to content

[RFC] commit-reach: skip STALE drain when only one merge-base needed #2109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
spkrka wants to merge 1 commit into
base: master
Choose a base branch
from
+183 −10
Open

[RFC] commit-reach: skip STALE drain when only one merge-base needed #2109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion builtin/merge-base.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ static int show_merge_base(struct commit **rev, size_t rev_nr, int show_all)
struct commit_list *result = NULL, *r;

if (repo_get_merge_bases_many_dirty(the_repository, rev[0],
rev_nr - 1, rev + 1, &result) < 0) {
rev_nr - 1, rev + 1,
show_all, &result) < 0) {
commit_list_free(result);
return -1;
}
Expand Down
26 changes: 18 additions & 8 deletions commit-reach.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,16 @@ static int paint_down_to_common(struct repository *r,
struct commit **twos,
timestamp_t min_generation,
int ignore_missing_commits,
int find_all,
struct commit_list **result)
{
struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
int i;
int has_gens = min_generation || corrected_commit_dates_enabled(r);
timestamp_t last_gen = GENERATION_NUMBER_INFINITY;
struct commit_list **tail = result;

if (!min_generation && !corrected_commit_dates_enabled(r))
if (!has_gens)
queue.compare = compare_commits_by_commit_date;

one->object.flags |= PARENT1;
Expand Down Expand Up @@ -97,6 +99,11 @@ static int paint_down_to_common(struct repository *r,
if (!(commit->object.flags & RESULT)) {
commit->object.flags |= RESULT;
tail = commit_list_append(commit, tail);
/* Generation-ordered queue: no later
* commit can dominate this one. */
if (!find_all && has_gens &&
generation < GENERATION_NUMBER_INFINITY)
break;
Comment thread
spkrka marked this conversation as resolved.
}
/* Mark parents of a found merge stale */
flags |= STALE;
Expand Down Expand Up @@ -136,6 +143,7 @@ static int paint_down_to_common(struct repository *r,
static int merge_bases_many(struct repository *r,
struct commit *one, int n,
struct commit **twos,
int find_all,
struct commit_list **result)
{
struct commit_list *list = NULL, **tail = result;
Expand Down Expand Up @@ -165,7 +173,7 @@ static int merge_bases_many(struct repository *r,
oid_to_hex(&twos[i]->object.oid));
}

if (paint_down_to_common(r, one, n, twos, 0, 0, &list)) {
if (paint_down_to_common(r, one, n, twos, 0, 0, find_all, &list)) {
commit_list_free(list);
return -1;
}
Expand Down Expand Up @@ -246,7 +254,7 @@ static int remove_redundant_no_gen(struct repository *r,
min_generation = curr_generation;
}
if (paint_down_to_common(r, array[i], filled,
work, min_generation, 0, &common)) {
work, min_generation, 0, 1, &common)) {
clear_commit_marks(array[i], all_flags);
clear_commit_marks_many(filled, work, all_flags);
commit_list_free(common);
Expand Down Expand Up @@ -425,14 +433,15 @@ static int get_merge_bases_many_0(struct repository *r,
size_t n,
struct commit **twos,
int cleanup,
int find_all,
struct commit_list **result)
{
struct commit_list *list, **tail = result;
struct commit **rslt;
size_t cnt, i;
int ret;

if (merge_bases_many(r, one, n, twos, result) < 0)
if (merge_bases_many(r, one, n, twos, find_all, result) < 0)
return -1;
for (i = 0; i < n; i++) {
if (one == twos[i])
Expand Down Expand Up @@ -475,24 +484,25 @@ int repo_get_merge_bases_many(struct repository *r,
struct commit **twos,
struct commit_list **result)
{
return get_merge_bases_many_0(r, one, n, twos, 1, result);
return get_merge_bases_many_0(r, one, n, twos, 1, 1, result);
}

int repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one,
size_t n,
struct commit **twos,
int find_all,
struct commit_list **result)
{
return get_merge_bases_many_0(r, one, n, twos, 0, result);
return get_merge_bases_many_0(r, one, n, twos, 0, find_all, result);
}

int repo_get_merge_bases(struct repository *r,
struct commit *one,
struct commit *two,
struct commit_list **result)
{
return get_merge_bases_many_0(r, one, 1, &two, 1, result);
return get_merge_bases_many_0(r, one, 1, &two, 1, 1, result);
}

/*
Expand Down Expand Up @@ -555,7 +565,7 @@ int repo_in_merge_bases_many(struct repository *r, struct commit *commit,

if (paint_down_to_common(r, commit,
nr_reference, reference,
generation, ignore_missing_commits, &bases))
generation, ignore_missing_commits, 1, &bases))
ret = -1;
else if (commit->object.flags & PARENT2)
ret = 1;
Expand Down
5 changes: 4 additions & 1 deletion commit-reach.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ int repo_get_merge_bases_many(struct repository *r,
struct commit *one, size_t n,
struct commit **twos,
struct commit_list **result);
/* To be used only when object flags after this call no longer matter */
/* To be used only when object flags after this call no longer matter.
* When find_all is false and generation numbers are available, returns
* after finding the first merge-base, skipping the STALE drain. */
int repo_get_merge_bases_many_dirty(struct repository *r,
struct commit *one, size_t n,
struct commit **twos,
int find_all,
struct commit_list **result);

int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result);
Expand Down
119 changes: 119 additions & 0 deletions t/t6010-merge-base.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,4 +305,123 @@ test_expect_success 'merge-base --octopus --all for complex tree' '
test_cmp expected actual
'

# The following tests verify that "git merge-base" (without --all)
# returns the same result with and without a commit-graph.
# This exercises the early-exit optimisation in paint_down_to_common
# that skips the STALE drain when generation numbers are available.
Comment thread
spkrka marked this conversation as resolved.

test_expect_success 'setup for commit-graph tests' '
git init graph-repo &&
(
cd graph-repo &&

# Build a forked DAG:
#
# L1---L2 (left)
# /
# S
# \
# R1---R2 (right)
#
test_commit GS &&
git checkout -b left &&
test_commit L1 &&
test_commit L2 &&
git checkout GS &&
git checkout -b right &&
test_commit GR1 &&
test_commit GR2
)
'

test_expect_success 'merge-base without commit-graph' '
(
cd graph-repo &&
rm -f .git/objects/info/commit-graph &&
git merge-base left right >actual &&
git rev-parse GS >expected &&
test_cmp expected actual
)
'

test_expect_success 'merge-base with commit-graph' '
(
cd graph-repo &&
git commit-graph write --reachable &&
git merge-base left right >actual &&
git rev-parse GS >expected &&
test_cmp expected actual
)
'

test_expect_success 'merge-base --all with commit-graph' '
(
cd graph-repo &&
git merge-base --all left right >actual &&
git rev-parse GS >expected &&
test_cmp expected actual
)
'

test_expect_success 'merge-base agrees with --all for single result' '
(
cd graph-repo &&
git commit-graph write --reachable &&
git merge-base left right >actual.single &&
git merge-base --all left right >actual.all &&
test_cmp actual.all actual.single
)
'

test_expect_success 'setup for deep chain commit-graph test' '
git init deep-repo &&
(
cd deep-repo &&

# Build a deep forked DAG:
#
# L1--L2--...--L20 (left)
# /
# S
# \
# R1--R2--...--R20 (right)
#
test_commit DS &&
git checkout -b left &&
for i in $(test_seq 1 20)
do
test_commit DL$i || return 1
done &&
git checkout DS &&
git checkout -b right &&
for i in $(test_seq 1 20)
do
test_commit DR$i || return 1
done
)
'

test_expect_success 'deep chain: merge-base matches with and without commit-graph' '
(
cd deep-repo &&
rm -f .git/objects/info/commit-graph &&
git merge-base left right >actual.no-graph &&
git rev-parse DS >expected &&
test_cmp expected actual.no-graph &&
git commit-graph write --reachable &&
git merge-base left right >actual.graph &&
test_cmp expected actual.graph
)
'

test_expect_success 'deep chain: --all and non---all agree with commit-graph' '
(
cd deep-repo &&
git commit-graph write --reachable &&
git merge-base left right >actual.single &&
git merge-base --all left right >actual.all &&
test_cmp actual.all actual.single
)
'

test_done
40 changes: 40 additions & 0 deletions t/t6600-test-reach.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,4 +882,44 @@ test_expect_success 'rev-list --maximal-only matches merge-base --independent' '
test_cmp expect.sorted actual.sorted
'

# The following tests verify the early-exit optimisation in
# paint_down_to_common when merge-base is invoked without --all.
# Each test checks all four commit-graph configurations.

merge_base_all_modes () {
test_when_finished rm -rf .git/objects/info/commit-graph &&
git merge-base "$@" >actual &&
test_cmp expect actual &&
cp commit-graph-full .git/objects/info/commit-graph &&
git merge-base "$@" >actual &&
test_cmp expect actual &&
cp commit-graph-half .git/objects/info/commit-graph &&
git merge-base "$@" >actual &&
test_cmp expect actual &&
cp commit-graph-no-gdat .git/objects/info/commit-graph &&
git merge-base "$@" >actual &&
test_cmp expect actual
}

test_expect_success 'merge-base without --all (unique base)' '
git rev-parse commit-5-3 >expect &&
merge_base_all_modes commit-5-7 commit-8-3
'

test_expect_success 'merge-base without --all is one of --all results' '
test_when_finished rm -rf .git/objects/info/commit-graph &&

cp commit-graph-full .git/objects/info/commit-graph &&
git merge-base --all commit-5-7 commit-4-8 commit-6-6 commit-8-3 >all &&
git merge-base commit-5-7 commit-4-8 commit-6-6 commit-8-3 >single &&
test_line_count = 1 single &&
grep -F -f single all &&

cp commit-graph-half .git/objects/info/commit-graph &&
git merge-base --all commit-5-7 commit-4-8 commit-6-6 commit-8-3 >all &&
git merge-base commit-5-7 commit-4-8 commit-6-6 commit-8-3 >single &&
test_line_count = 1 single &&
grep -F -f single all
'

test_done
Loading