commit-reach: use corrected commit dates in paint_down_to_common()

091f4cf (commit: don't use generation numbers if not needed,
2018-08-30) changed paint_down_to_common() to use commit dates instead
of generation numbers v1 (topological levels) as the performance
regressed on certain topologies. With generation number v2 (corrected
commit dates) implemented, we no longer have to rely on commit dates and
can use generation numbers.

For example, the command `git merge-base v4.8 v4.9` on the Linux
repository walks 167468 commits, taking 0.135s for committer date and
167496 commits, taking 0.157s for corrected committer date respectively.

While using corrected commit dates, Git walks nearly the same number of
commits as commit date, the process is slower as for each comparision we
have to access a commit-slab (for corrected committer date) instead of
accessing struct member (for committer date).

This change incidentally broke the fragile t6404-recursive-merge test.
t6404-recursive-merge sets up a unique repository where all commits have
the same committer date without a well-defined merge-base.

While running tests with GIT_TEST_COMMIT_GRAPH unset, we use committer
date as a heuristic in paint_down_to_common(). 6404.1 'combined merge
conflicts' merges commits in the order:
- Merge C with B to form an intermediate commit.
- Merge the intermediate commit with A.

With GIT_TEST_COMMIT_GRAPH=1, we write a commit-graph and subsequently
use the corrected committer date, which changes the order in which
commits are merged:
- Merge A with B to form an intermediate commit.
- Merge the intermediate commit with C.

While resulting repositories are equivalent, 6404.4 'virtual trees were
processed' fails with GIT_TEST_COMMIT_GRAPH=1 as we are selecting
different merge-bases and thus have different object ids for the
intermediate commits.

As this has already causes problems (as noted in 859fdc0 (commit-graph:
define GIT_TEST_COMMIT_GRAPH, 2018-08-29)), we disable commit graph
within t6404-recursive-merge.

Signed-off-by: Abhishek Kumar <abhishekkumar8222@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/commit-graph.c b/commit-graph.c
index 7034a14e5c93551132441c53e05acb40f4b7a8a7..f3bde2ad95a16ad81c657296d999e7f7c417ff4b 100644 (file)
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -714,6 +714,20 @@ int generation_numbers_enabled(struct repository *r)
        return !!first_generation;
 }
 
+int corrected_commit_dates_enabled(struct repository *r)
+{
+       struct commit_graph *g;
+       if (!prepare_commit_graph(r))
+               return 0;
+
+       g = r->objects->commit_graph;
+
+       if (!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 = r->objects->commit_graph;

diff --git a/commit-graph.h b/commit-graph.h
index ad52130883b9e7fbeff8e7a60f298d8d53297ff3..97f3497c2790c5bca0f9c73dd069291c95456895 100644 (file)
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -95,6 +95,12 @@ 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 9b24b0378d51f307a131696e007524911e7aad4d..e38771ca5a1f63c84b7ad3e6d3bf6013c923042a 100644 (file)
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -39,7 +39,7 @@ static struct commit_list *paint_down_to_common(struct repository *r,
        int i;
        timestamp_t last_gen = GENERATION_NUMBER_INFINITY;
 
-       if (!min_generation)
+       if (!min_generation && !corrected_commit_dates_enabled(r))
                queue.compare = compare_commits_by_commit_date;
 
        one->object.flags |= PARENT1;

diff --git a/t/t6404-recursive-merge.sh b/t/t6404-recursive-merge.sh
index b1c3d4dda49fc3c7d08e472217614469c76e9b9b..86f74ae584797ec128b7a2c24796571a12069d65 100755 (executable)
--- a/t/t6404-recursive-merge.sh
+++ b/t/t6404-recursive-merge.sh
@@ -15,6 +15,8 @@ GIT_COMMITTER_DATE="2006-12-12 23:28:00 +0100"
 export GIT_COMMITTER_DATE
 
 test_expect_success 'setup tests' '
+       GIT_TEST_COMMIT_GRAPH=0 &&
+       export GIT_TEST_COMMIT_GRAPH &&
        echo 1 >a1 &&
        git add a1 &&
        GIT_AUTHOR_DATE="2006-12-12 23:00:00" git commit -m 1 a1 &&
@@ -66,7 +68,7 @@ test_expect_success 'setup tests' '
 '
 
 test_expect_success 'combined merge conflicts' '
-       test_must_fail env GIT_TEST_COMMIT_GRAPH=0 git merge -m final G
+       test_must_fail git merge -m final G
 '
 
 test_expect_success 'result contains a conflict' '
@@ -82,6 +84,7 @@ test_expect_success 'result contains a conflict' '
 '
 
 test_expect_success 'virtual trees were processed' '
+       # TODO: fragile test, relies on ambigious merge-base resolution
        git ls-files --stage >out &&
 
        cat >expect <<-EOF &&