git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 08/13] merge-ort: add code to check for whether cached renames can be reused

From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Date
May 20, 2021, 06:09 UTC
Message-ID
<e4a97b1b16ca4e2fe6bdbee2630b3e189fc39a67.1621490982.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.859.v3.git.1621490982.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>

We need to know when renames detected in a previous merge operation can be reused in a later merge operation. Consider the following setup (from the git-rebase manpage):

                     A---B---C topic
                    /
               D---E---F---G master
After rebasing, this will appear as:
                             A'--B'--C' topic
                            /
               D---E---F---G master
Further, let's say that 'oldfile' was renamed to 'newfile' between E
and G.  The rebase or cherry-pick of A onto G will involve a three-way
merge between E (as the merge base) and G and A.  After detecting the
rename between E:oldfile and G:newfile, there will be a three-way
content merge of the following:
    E:oldfile
    G:newfile
    A:oldfile
and produce a new result:
    A':newfile
Now, when we want to pick B onto A', we will need to do a three-way
merge between A (as the merge-base) and A' and B.  This will involve
a three-way content merge of
    A:oldfile
    A':newfile
    B:oldfile
but only if we can detect that A:oldfile is similar enough to A':newfile
to be used together in a three-way content merge, i.e. only if we can
detect that A:oldfile and A':newfile are a rename.  But we already know
that A:oldfile and A':newfile are similar enough to be used in a
three-way content merge, because that is precisely where A':newfile came
from in the previous merge.

Note that A & A' both appear in both merges. That gives us the condition under which we can reuse renames.

There are a couple important points about this optimization:
  - If the rebase or cherry-pick halts for user conflicts, these caches
    are NOT saved anywhere.  Thus, resuming a halted rebase or
    cherry-pick will result in no reused renames for the next commit.
    This is intentional, as user resolution can change files
    significantly and in ways that violate the similarity assumptions
    here.
  - Technically, in a *very* narrow case this might give slightly
    different results for rename detection.  Using the example above,
    if:
      * E:oldfile had 20 lines
      * G:newfile added 10 new lines at the beginning of the file
      * A:oldfile deleted all but the first three lines of the file
    then
      => A':newfile would have 13 lines, 3 of which matches those
         in A:oldfile.
    Consider the two cases:
      * Without this optimization:
        - the next step of the rebase operation (moving B to B')
          would not detect the rename betwen A:oldfile and A':newfile
        - we'd thus get a modify/delete conflict with the rebase
          operation halting for the user to resolve, and have both
          A':newfile and B:oldfile sitting in the working tree.
      * With this optimization:
        - the rename between A:oldfile and A':newfile would be detected
          via the cache of renames
        - a three-way merge between A:oldfile, A':newfile, and B:oldfile
          would commence and be written to A':newfile
    Now, is the difference in behavior a bug...or a bugfix?  I can't
    tell.  Given that A:oldfile and A':newfile are not very similar,
    when we three-way merge with B:oldfile it seems likely we'll hit a
    conflict for the user to resolve.  And it shouldn't be too hard for
    users to see why we did that three-way merge; oldfile and newfile
    *were* renames somewhere in the sequence.  So, most of these corner
    cases will still behave similarly -- namely, a conflict given to the
    user to resolve.  Also, consider the interesting case when commit B
    is a clean revert of commit A.  Without this optimization, a rebase
    could not both apply a weird patch like A and then immediately
    revert it; users would be forced to resolve merge conflicts.  With
    this optimization, it would successfully apply the clean revert.
    So, there is certainly at least one case that behaves better.  Even
    if it's considered a "difference in behavior", I think both behaviors
    are reasonable, and the time savings provided by this optimization
    justify using the slightly altered rename heuristics.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-ort.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 2 deletions(-)
diff --git a/merge-ort.c b/merge-ort.c
index da9026524aa5..e08b60a26d20 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -140,6 +140,30 @@ struct rename_info {
 	int callback_data_nr, callback_data_alloc;
 	char *callback_data_traverse_path;
 
+	/*
+	 * merge_trees: trees passed to the merge algorithm for the merge
+	 *
+	 * merge_trees records the trees passed to the merge algorithm.  But,
+	 * this data also is stored in merge_result->priv.  If a sequence of
+	 * merges are being done (such as when cherry-picking or rebasing),
+	 * the next merge can look at this and re-use information from
+	 * previous merges under certain circumstances.
+	 *
+	 * See also all the cached_* variables.
+	 */
+	struct tree *merge_trees[3];
+
+	/*
+	 * cached_pairs_valid_side: which side's cached info can be reused
+	 *
+	 * See the description for merge_trees.  For repeated merges, at most
+	 * only one side's cached information can be used.  Valid values:
+	 *   MERGE_SIDE2: cached data from side2 can be reused
+	 *   MERGE_SIDE1: cached data from side1 can be reused
+	 *   0:           no cached data can be reused
+	 */
+	int cached_pairs_valid_side;
+
 	/*
 	 * cached_pairs: Caching of renames and deletions.
 	 *
@@ -473,6 +497,8 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti,
 		strmap_func(&renames->cached_pairs[i], 1);
 		strset_func(&renames->cached_irrelevant[i]);
 	}
+	renames->cached_pairs_valid_side = 0;
+	renames->dir_rename_mask = 0;
 
 	if (!reinitialize) {
 		struct hashmap_iter iter;
@@ -495,8 +521,6 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti,
 		strmap_clear(&opti->output, 0);
 	}
 
-	renames->dir_rename_mask = 0;
-
 	/* Clean out callback_data as well. */
 	FREE_AND_NULL(renames->callback_data);
 	renames->callback_data_nr = renames->callback_data_alloc = 0;
@@ -3816,6 +3840,35 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
 	trace2_region_leave("merge", "allocate/init", opt->repo);
 }
 
+static void merge_check_renames_reusable(struct merge_options *opt,
+					 struct merge_result *result,
+					 struct tree *merge_base,
+					 struct tree *side1,
+					 struct tree *side2)
+{
+	struct rename_info *renames;
+	struct tree **merge_trees;
+	struct merge_options_internal *opti = result->priv;
+
+	if (!opti)
+		return;
+
+	renames = &opti->renames;
+	merge_trees = renames->merge_trees;
+	/* merge_trees[0..2] will only be NULL if opti is */
+	assert(merge_trees[0] && merge_trees[1] && merge_trees[2]);
+
+	/* Check if we meet a condition for re-using cached_pairs */
+	if (oideq(&merge_base->object.oid, &merge_trees[2]->object.oid) &&
+	    oideq(&side1->object.oid, &result->tree->object.oid))
+		renames->cached_pairs_valid_side = MERGE_SIDE1;
+	else if (oideq(&merge_base->object.oid, &merge_trees[1]->object.oid) &&
+		 oideq(&side2->object.oid, &result->tree->object.oid))
+		renames->cached_pairs_valid_side = MERGE_SIDE2;
+	else
+		renames->cached_pairs_valid_side = 0; /* neither side valid */
+}
+
 /*** Function Grouping: merge_incore_*() and their internal variants ***/
 
 /*
@@ -3963,7 +4016,16 @@ void merge_incore_nonrecursive(struct merge_options *opt,
 
 	trace2_region_enter("merge", "merge_start", opt->repo);
 	assert(opt->ancestor != NULL);
+	merge_check_renames_reusable(opt, result, merge_base, side1, side2);
 	merge_start(opt, result);
+	/*
+	 * Record the trees used in this merge, so if there's a next merge in
+	 * a cherry-pick or rebase sequence it might be able to take advantage
+	 * of the cached_pairs in that next merge.
+	 */
+	opt->priv->renames.merge_trees[0] = merge_base;
+	opt->priv->renames.merge_trees[1] = side1;
+	opt->priv->renames.merge_trees[2] = side2;
 	trace2_region_leave("merge", "merge_start", opt->repo);
 
 	merge_ort_nonrecursive_internal(opt, merge_base, side1, side2, result);
-- 
gitgitgadget
Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren via GitGitGadget
Message 53 of 61 in “Optimization batch 11: avoid repeatedly detecting same renames”
  1. 0/7 Optimization batch 11: avoid repeatedly detecting same renamesElijah Newren via GitGitGadget, Mar 24, 2021
  2. 2/7 merge-ort: populate caches of rename detection resultsElijah Newren via GitGitGadget, Mar 24, 2021
  3. 1/7 merge-ort: add data structures for in-memory caching of rename detectionElijah Newren via GitGitGadget, Mar 24, 2021
  4. 4/7 merge-ort: avoid accidental API mis-useElijah Newren via GitGitGadget, Mar 24, 2021
  5. 3/7 merge-ort: add code to check for whether cached renames can be reusedElijah Newren via GitGitGadget, Mar 24, 2021
  6. 5/7 merge-ort: preserve cached renames for the appropriate sideElijah Newren via GitGitGadget, Mar 24, 2021
  7. 6/7 merge-ort: add helper functions for using cached renamesElijah Newren via GitGitGadget, Mar 24, 2021
  8. 7/7 merge-ort, diffcore-rename: employ cached renames when possibleElijah Newren via GitGitGadget, Mar 24, 2021
  9. Junio C HamanoMar 24, 2021
  10. Elijah NewrenMar 24, 2021
  11. Junio C HamanoMar 25, 2021
  12. Elijah NewrenMar 29, 2021
  13. Derrick StoleeMar 30, 2021
  14. 00/13 Optimization batch 11: avoid repeatedly detecting same renamesElijah Newren via GitGitGadget, May 4, 2021
  15. 01/13 t6423: rename file within directory that other side renamedElijah Newren via GitGitGadget, May 4, 2021
  16. 03/13 fast-rebase: change assert() to BUG()Elijah Newren via GitGitGadget, May 4, 2021
  17. 04/13 fast-rebase: write conflict state to working tree, index, and HEADElijah Newren via GitGitGadget, May 4, 2021
  18. Derrick StoleeMay 17, 2021
  19. Elijah NewrenMay 18, 2021
  20. Derrick StoleeMay 18, 2021
  21. 02/13 Documentation/technical: describe remembering renames optimizationElijah Newren via GitGitGadget, May 4, 2021
  22. 07/13 merge-ort: populate caches of rename detection resultsElijah Newren via GitGitGadget, May 4, 2021
  23. Derrick StoleeMay 17, 2021
  24. Elijah NewrenMay 20, 2021
  25. 06/13 merge-ort: add data structures for in-memory caching of rename detectionElijah Newren via GitGitGadget, May 4, 2021
  26. Derrick StoleeMay 17, 2021
  27. Elijah NewrenMay 18, 2021
  28. Derrick StoleeMay 18, 2021
  29. 05/13 t6429: testcases for remembering renamesElijah Newren via GitGitGadget, May 4, 2021
  30. 08/13 merge-ort: add code to check for whether cached renames can be reusedElijah Newren via GitGitGadget, May 4, 2021
  31. Derrick StoleeMay 17, 2021
  32. 10/13 merge-ort: preserve cached renames for the appropriate sideElijah Newren via GitGitGadget, May 4, 2021
  33. 11/13 merge-ort: add helper functions for using cached renamesElijah Newren via GitGitGadget, May 4, 2021
  34. 12/13 merge-ort: handle interactions of caching and rename/rename(1to1) casesElijah Newren via GitGitGadget, May 4, 2021
  35. Derrick StoleeMay 17, 2021
  36. 13/13 merge-ort, diffcore-rename: employ cached renames when possibleElijah Newren via GitGitGadget, May 4, 2021
  37. Derrick StoleeMay 17, 2021
  38. Elijah NewrenMay 20, 2021
  39. Derrick StoleeMay 22, 2021
  40. 09/13 merge-ort: avoid accidental API mis-useElijah Newren via GitGitGadget, May 4, 2021
  41. Derrick StoleeMay 17, 2021
  42. Elijah NewrenMay 14, 2021
  43. Derrick StoleeMay 14, 2021
  44. 00/13 Optimization batch 11: avoid repeatedly detecting same renamesElijah Newren via GitGitGadget, May 20, 2021
  45. 01/13 t6423: rename file within directory that other side renamedElijah Newren via GitGitGadget, May 20, 2021
  46. 02/13 Documentation/technical: describe remembering renames optimizationElijah Newren via GitGitGadget, May 20, 2021
  47. Bagas SanjayaMay 20, 2021
  48. Kerry, RichardMay 20, 2021
  49. Elijah NewrenMay 20, 2021
  50. 03/13 fast-rebase: change assert() to BUG()Elijah Newren via GitGitGadget, May 20, 2021
  51. 04/13 fast-rebase: write conflict state to working tree, index, and HEADElijah Newren via GitGitGadget, May 20, 2021
  52. 05/13 t6429: testcases for remembering renamesElijah Newren via GitGitGadget, May 20, 2021
  53. 08/13 merge-ort: add code to check for whether cached renames can be reusedElijah Newren via GitGitGadget, May 20, 2021
  54. 07/13 merge-ort: populate caches of rename detection resultsElijah Newren via GitGitGadget, May 20, 2021
  55. 06/13 merge-ort: add data structures for in-memory caching of rename detectionElijah Newren via GitGitGadget, May 20, 2021
  56. 09/13 merge-ort: avoid accidental API mis-useElijah Newren via GitGitGadget, May 20, 2021
  57. 10/13 merge-ort: preserve cached renames for the appropriate sideElijah Newren via GitGitGadget, May 20, 2021
  58. 12/13 merge-ort: handle interactions of caching and rename/rename(1to1) casesElijah Newren via GitGitGadget, May 20, 2021
  59. 11/13 merge-ort: add helper functions for using cached renamesElijah Newren via GitGitGadget, May 20, 2021
  60. 13/13 merge-ort, diffcore-rename: employ cached renames when possibleElijah Newren via GitGitGadget, May 20, 2021
  61. Derrick StoleeMay 22, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.