Remove the fallback that switched paint_down_to_common() from
generation ordering to commit-date ordering when the commit-graph
lacks corrected commit dates (v1 graph with topo levels only).
The fallback was added in
091f4cf3 (commit: don't use generation
numbers if not needed, 2018-08-30) to avoid a performance
regression on the Linux kernel repo where v1 topo levels caused
"git merge-base v4.8 v4.9" to walk 636k commits instead of 167k.
A side branch with a low topo level stayed in the queue behind a
long chain, preventing early STALE propagation.
Side-exhaustion (added in the previous commits) solves this
differently by terminating the walk as soon as one paint side
empties from the queue, preventing the deep walk regardless of
queue ordering. Benchmarks of "git merge-base --all v4.8 v4.9"
on the Linux kernel repo show that side-exhaustion reduces the
step count far below what the date-ordering fallback achieved:
steps time
no graph, baseline: 167,413 3.25 s
v1 graph, baseline: 167,413 0.25 s
v2 graph, baseline: 167,441 0.29 s
v1 graph, this series: 5,725 0.02 s
v2 graph, this series: 3,887 0.01 s
With generation ordering always active, the existing min_generation
check in paint_queue_get() correctly identifies when the walk has
reached the finite generation region. The date ordering fallback
broke this invariant: a commit could have a finite topo level
while the queue was date-ordered, causing the early exit to fire
before all merge bases were found.
Also remove corrected_commit_dates_enabled() from commit-graph.c
which has no remaining callers.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
INFINITY and finite generation regions
--------------------------------------
-The properties in this section assume generation-number ordering (the
-default comparator). They do NOT hold when the date-ordering fallback
-is active -- see <<date-ordering-fallback>>.
-
The commit-graph stores a generation number for each commit.
Commits not in the commit-graph have generation
`GENERATION_NUMBER_INFINITY`. The graph is closed under
means that paint on already-visited commits is final -- no future
traversal step can add paint to them.
-In the INFINITY region, commit-date ordering can violate this: a
-parent with a later date can be visited before a child with an earlier
-date. Paint flags are therefore NOT final at visit time, and a
-commit visited with only one side's paint may later gain the other.
+In the INFINITY region, all commits share the same generation
+value, so the queue breaks ties by commit date. This can violate
+topological ordering: a parent with a later date can be visited
+before a child with an earlier date. Paint flags are therefore
+NOT final at visit time, and a commit visited with only one
+side's paint may later gain the other.
Paint flags are only added, never removed. Since each flag can be set
at most once per commit, the number of times a commit can be
children are visited first), so it cannot be redundant and the walk
can stop immediately.
-This optimization is NOT safe when the date-ordering fallback is
-active, because commit-date order can visit a deeper ancestor
-before a shallower one -- see <<date-ordering-fallback>>.
-
-[[date-ordering-fallback]]
-Date-ordering fallback
-----------------------
-
-When the commit-graph has generation numbers v1 and no
-generation floor is specified, topological ordering
-(via generation numbers) is disabled. Topological levels are
-correct but unbalanced -- ordering by such generation numbers
-can sometimes cause the walk to detour too far before finding
-merge bases. Commit-date ordering typically reaches them in
-fewer steps -- see this change for more details:
-
- 091f4cf3 (commit: don't use generation numbers if not needed,
- 2018-08-30)
-
-With generation number v2 (corrected commit dates) we have the best
-of both worlds and do not need this fallback.
-
-For v1, `paint_down_to_common()` falls back to pure commit-date
-ordering via `compare_commits_by_commit_date`. Because commit
-dates are not monotonic (clock skew, rebases, etc.), the queue
-may visit commits out of topological order.
-
-This disables the optimizations that depend on generation ordering:
-
- - *Single result*: the first merge-base candidate found may not
- be the shallowest, because a deeper ancestor with a higher
- commit date can be dequeued first.
-
- - *Side exhaustion*: one paint side can appear to drain from the
- queue while commits from that side are still waiting with lower
- dates, causing premature termination.
-
Related documentation
---------------------
return !!first_generation;
}
-int corrected_commit_dates_enabled(struct repository *r)
-{
- struct commit_graph *g;
-
- g = prepare_commit_graph(r);
- if (!g || !g->num_commits)
- return 0;
-
- return g->read_generation_data;
-}
-
struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r)
{
struct commit_graph *g;
*/
int generation_numbers_enabled(struct repository *r);
-/*
- * Return 1 if and only if the repository has a commit-graph
- * file and generation data chunk has been written for the file.
- */
-int corrected_commit_dates_enabled(struct repository *r);
-
struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r);
enum commit_graph_write_flags {
size_t parent1_count;
size_t parent2_count;
size_t mb_candidate_count;
- int gen_ordered;
timestamp_t min_generation;
timestamp_t last_gen;
};
/* one side is exhausted */
if ((!state->parent1_count || !state->parent2_count) &&
- state->gen_ordered &&
generation < GENERATION_NUMBER_INFINITY)
return NULL;
}
enum merge_base_flags mb_flags,
struct commit_list **result)
{
+ /*
+ * Generation ordering is required for the side-exhaustion and
+ * single-result early exits, which rely on topological traversal
+ * order (children visited before parents) in the finite region.
+ */
struct paint_state state = {
- .queue = { compare_commits_by_gen_then_commit_date },
- .gen_ordered = 1,
+ .queue = { compare_commits_by_gen_then_commit_date }
};
struct commit *commit;
int i;
state.min_generation = min_generation;
state.last_gen = GENERATION_NUMBER_INFINITY;
- if (!min_generation && !corrected_commit_dates_enabled(r)) {
- state.queue.compare = compare_commits_by_commit_date;
- state.gen_ordered = 0;
- }
one->object.flags |= PARENT1;
if (!n) {
* descendant of this one.
*/
if (!(mb_flags & MERGE_BASE_FIND_ALL) &&
- state.gen_ordered &&
state.last_gen < GENERATION_NUMBER_INFINITY)
break;
}
git rev-parse pi-B
} >expect &&
test_all_modes get_merge_bases_many &&
- test_paint_down_steps 5 4 5 5
+ test_paint_down_steps 5 4 5 4
'
test_expect_success 'setup mixed finite/INFINITY topology' '
>input &&
git rev-parse commit-9-1 >expect &&
run_all_modes git merge-base --all commit-9-9 commit-9-1 &&
- test_paint_down_steps 81 9 57 81
+ test_paint_down_steps 81 9 57 37
'
test_expect_success 'merge-base --all with clock skew (side-exhaustion)' '
- # Verify correct merge base under clock skew. se-D (the
- # merge base) has a higher date than its child se-C.
- # Generation ordering ensures se-C is visited before se-D,
- # so P1 paint propagates correctly and se-D is found.
+ # Verify that the merge base is computed correctly even
+ # when commits have non-monotonic commit dates.
>input &&
git rev-parse se-D >expect &&
run_all_modes git merge-base --all se-A se-B &&
- test_paint_down_steps 6 4 6 6
+ test_paint_down_steps 6 4 6 4
'
test_expect_success 'merge-base --all with clock skew and redundant ancestor (side-exhaustion)' '
- # Verify correct merge base when clock skew could cause a
- # too-deep result. MB1 is the correct merge base; MB2 is
- # its ancestor. A reaches MB2 via E (high date) and MB1
- # via C (low date). Generation ordering ensures C is
- # visited before side-exhaustion fires, so MB1 is found
- # and remove_redundant correctly discards MB2.
+ # Verify that the correct merge base is found even when
+ # non-monotonic commit dates could cause a redundant
+ # ancestor to be visited first.
>input &&
git rev-parse se2-MB1 >expect &&
run_all_modes git merge-base --all se2-A se2-B &&
- test_paint_down_steps 8 6 8 8
+ test_paint_down_steps 8 6 8 6
'
test_expect_success 'reduce_heads' '
projects / thirdparty / git.git / commitdiff
