diff --git a/Documentation/technical/paint-down-to-common.adoc b/Documentation/technical/paint-down-to-common.adoc index 7ae9b11529..f9978ff9f0 100644 --- a/Documentation/technical/paint-down-to-common.adoc +++ b/Documentation/technical/paint-down-to-common.adoc @@ -44,10 +44,6 @@ ancestor is necessarily redundant. 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 <>. - 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 @@ -82,10 +78,12 @@ traversal: children are always visited before their parents. This 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 @@ -149,43 +147,6 @@ descendant of this candidate (generation ordering guarantees 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 ----------------------- - -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 --------------------- diff --git a/commit-graph.c b/commit-graph.c index c6d9c5c740..abccdeacd7 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -793,17 +793,6 @@ int generation_numbers_enabled(struct repository *r) 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; diff --git a/commit-graph.h b/commit-graph.h index 13ca4ff010..d96147a07c 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -136,12 +136,6 @@ struct commit_graph *parse_commit_graph(struct repository *r, */ 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 { diff --git a/commit-reach.c b/commit-reach.c index ba0b767b64..db397048e1 100644 --- a/commit-reach.c +++ b/commit-reach.c @@ -89,7 +89,6 @@ struct paint_state { size_t parent1_count; size_t parent2_count; size_t mb_candidate_count; - int gen_ordered; timestamp_t min_generation; timestamp_t last_gen; }; @@ -166,7 +165,6 @@ static struct commit *paint_queue_get(struct paint_state *state) /* one side is exhausted */ if ((!state->parent1_count || !state->parent2_count) && - state->gen_ordered && generation < GENERATION_NUMBER_INFINITY) return NULL; } @@ -187,9 +185,13 @@ static int paint_down_to_common(struct repository *r, 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; @@ -198,10 +200,6 @@ static int paint_down_to_common(struct repository *r, 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) { @@ -229,7 +227,6 @@ static int paint_down_to_common(struct repository *r, * descendant of this one. */ if (!(mb_flags & MERGE_BASE_FIND_ALL) && - state.gen_ordered && state.last_gen < GENERATION_NUMBER_INFINITY) break; } diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh index 8b6fa5fe0c..cbdb4d3b40 100755 --- a/t/t6600-test-reach.sh +++ b/t/t6600-test-reach.sh @@ -381,7 +381,7 @@ test_expect_success 'get_merge_bases_many:infinity-both-sides' ' 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' ' @@ -414,31 +414,26 @@ test_expect_success 'merge-base --all commit-walk steps' ' >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' '