Performance regression in `git log --follow`
Hi,
a user on IRC reported a performance regression in linux.git:
$ git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git $ git -C linux log --stat --follow -- include/linux/rsc_table.h
I was able to reproduce the performance regression: while this command takes roughly 33 seconds with Git 2.55, it takes almost 8 minutes with Git 2.56.
I've bisected the regression to 304812ed33 (log: improve --follow following renames for non-linear history, 2026-06-22). I don't have time right now to debug any further, so I decided to just forward the report to the mailing list. I've also Cc'd the original author of the above commit.
Thanks!
Patrick
Quoted 11 lines · Patrick Steinhardt
Hi,
a user on IRC reported a performance regression in linux.git:
$ git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git $ git -C linux log --stat --follow -- include/linux/rsc_table.hI was able to reproduce the performance regression: while this command takes roughly 33 seconds with Git 2.55, it takes almost 8 minutes with Git 2.56.
Quoted 5 lines
I've bisected the regression to
304812ed33(log: improve --follow following renames for non-linear history, 2026-06-22). I don't have time right now to debug any further, so I decided to just forward the report to the mailing list. I've also Cc'd the original author of the above commit.
The performance regression is clearly noticable in the git repository, though the slowdown is not as pronounced as in linux.git:
Benchmark 1: ./304812ed33^/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh
Time (mean ± σ): 3.275 s ± 0.022 s [User: 3.157 s, System: 0.116 s]
Range (min … max): 3.240 s … 3.313 s 10 runs
Benchmark 2: ./304812ed33/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh
Time (mean ± σ): 8.825 s ± 0.029 s [User: 8.554 s, System: 0.264 s]
Range (min … max): 8.769 s … 8.864 s 10 runs
Summary
'./304812ed33^/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh' ran
2.69 ± 0.02 times faster than './304812ed33/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh'That's 5.5 seconds of runtime difference, and with a bit of trace performance hack[1] it turns out that most of it is spent in a call to propagate_follow_pathspec_to_parent() to compare the current commit with its second...N-th parent:
$ GIT_TRACE_PERFORMANCE=1 ./304812ed33/bin/git --no-pager log --follow --oneline v2.56.0 t/valgrind/valgrind.sh >/dev/null 21:59:30.465592 read-cache.c:2364 performance: 0.000626524 s: read cache .git/index 21:59:39.246697 log-tree.c:1158 performance: 1.374980154 s: first parent propagate_follow_pathspec_to_parent() 21:59:39.247164 log-tree.c:1159 performance: 4.448692273 s: N-th parents propagate_follow_pathspec_to_parent() 21:59:39.247192 trace.c:416 performance: 8.799236172 s: git command: ./304812ed33/bin/git --no-pager log --follow --oneline v2.56.0 t/valgrind/valgrind.sh
Interestingly, although line-level log performs its own rename tracking, it's still notably faster than "--follow" even before this perf regression (that "-L1,40" means "track the whole file"):
Benchmark 1: ./304812ed33^/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh
Time (mean ± σ): 3.260 s ± 0.019 s [User: 3.140 s, System: 0.118 s]
Range (min … max): 3.219 s … 3.284 s 10 runs
Benchmark 2: ./304812ed33^/bin/git --no-pager log --oneline --no-patch -L1,40:t/valgrind/valgrind.sh v2.56.0
Time (mean ± σ): 1.454 s ± 0.014 s [User: 1.369 s, System: 0.084 s]
Range (min … max): 1.441 s … 1.481 s 10 runs
Summary
'./304812ed33^/bin/git --no-pager log --oneline --no-patch -L1,40:t/valgrind/valgrind.sh v2.56.0' ran
2.24 ± 0.02 times faster than './304812ed33^/bin/git --no-pager log --oneline --no-patch --follow v2.56.0 t/valgrind/valgrind.sh'The reason for this might be:
- If line-level log finds that the interesting path is not modified between a merge commit and its first parent, then it doesn't perform a diff between that merge commit and its second..N-th parent.
"--follow" seems to performs a diff between a merge commit and all its parents.
- Line-level log checks for renames only when the path is not present in the parent commit.
"--follow" seems to always look for renames.
[1] It really is just a hack:
log-tree.c +16 -1
@@ -34,6 +34,7 @@#include "wildmatch.h" #include "write-or-die.h" #include "pager.h" +#include "trace.h" static struct decoration name_decoration = {"object names" };static int decoration_loaded;
@@ -1144,6 +1145,8 @@ static void free_follow_pathspec_slot(char **slot)FREE_AND_NULL (*slot); } +static uint64_t t_first =0 , t_nth =0 ; +void release_follow_pathspec_slab (struct rev_info *opt) {if (!opt->follow_pathspec_slab)
@@ -1151,6 +1154,9 @@ void release_follow_pathspec_slab(struct rev_info *opt)deep_clear_follow_pathspec_slab (opt->follow_pathspec_slab, free_follow_pathspec_slot);FREE_AND_NULL (opt->follow_pathspec_slab); + +trace_performance (t_first,"first parent propagate_follow_pathspec_to_parent()" ); +trace_performance (t_nth,"N-th parents propagate_follow_pathspec_to_parent()" ); }/* Compute a path to follow in parent, if there is one */
@@ -1304,9 +1310,18 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)struct commit_list *parents =get_saved_parents (opt, commit);if (parents && parents->next) {struct commit_list *p; -for (p = parents; p; p = p->next) +for (p = parents; p; p = p->next) { +if (p == parents) + t_first -=getnanotime (); +else + t_nth -=getnanotime ();propagate_follow_pathspec_to_parent (opt, commit, p->item); +if (p == parents) + t_first +=getnanotime (); +else + t_nth +=getnanotime (); + } }else if (parents) {remember_follow_pathspec (opt, parents->item,pathspec_single_path (&opt->diffopt.pathspec));
PATCH 1/1 log: speed up --follow following renames for non-linear history
Test the performance of 'git log --stat --follow -- include/linux/rsc_table.h' on linux.git, it used to take about 28 seconds, and since commit 304812ed33 (log: improve --follow following renames for non-linear history, 2026-06-22), it takes 6m43s.
What happens is that propagate_follow_pathspec_to_parent() does a somewhat expensive rename detection of the current commit vs all of its parents, and this slows down 'git log --follow'.
Fix the problem by introducing two shortcuts:
1) Only do that when the path is added relative to a parent and
2) If the path is the same as in one of the parents, then just follow that history
These shortcuts keep all the uses-cases in t4219-log-follow-merge.sh working, but now the above use case takes 29s, close to the original cost.
---
Hi Gabor, Patrick,
Here is my suggested speed-up patch. I think it's correct to take these two shortcuts since the original use cases continue to work, and then we're almost back to the original performance.
Let me know what you think; if you don't see problems with it, then a bit later I would send the patch to Junio.
Thanks,
Miklos
log-tree.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 4 deletions(-)
log-tree.c +70 -4
@@ -1180,6 +1180,74 @@ static void propagate_follow_pathspec_to_parent(struct rev_info *opt,diff_free (&diff_opts); } +enum follow_path_change { +FOLLOW_PATH_SAME , +FOLLOW_PATH_ADDED , +FOLLOW_PATH_OTHER , +}; + +/* How does the followed path change from parent to commit? */ +static enum follow_path_change get_follow_path_change (struct rev_info *opt, +struct commit *commit, +struct commit *parent) +{ +struct diff_options diff_opts; +enum follow_path_change ret =FOLLOW_PATH_OTHER ; + +parse_commit_or_die (parent); +repo_diff_setup (opt->diffopt.repo, &diff_opts); +copy_pathspec (&diff_opts.pathspec, &opt->diffopt.pathspec); + diff_opts.flags.recursive =1 ; + diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; +diff_setup_done (&diff_opts); +diff_tree_oid (get_commit_tree_oid (parent), +get_commit_tree_oid (commit), +"" , &diff_opts); + +if (!diff_queued_diff.nr) + ret =FOLLOW_PATH_SAME ; +else if (diff_queued_diff.nr ==1 && + !DIFF_FILE_VALID (diff_queued_diff.queue[0 ]->one)) + ret =FOLLOW_PATH_ADDED ; + +diff_queue_clear (&diff_queued_diff); +diff_free (&diff_opts); +return ret; +} + +/* Compute a path to follow in each parent of a merge */ +static void propagate_follow_pathspec_to_parents (struct rev_info *opt, +struct commit *commit, +struct commit_list *parents) +{ +const char *path =pathspec_single_path (&opt->diffopt.pathspec); +struct commit_list *p; +enum follow_path_change *changes; +int nr =commit_list_count (parents), i; +int has_same =0 ; + +if (!path) +return ; + +ALLOC_ARRAY (changes, nr); +for (p = parents, i =0 ; p; p = p->next, i++) { + changes[i] =get_follow_path_change (opt, commit, p->item); +if (changes[i] ==FOLLOW_PATH_SAME ) { + has_same =1 ; +break ; + } + } + +for (p = parents, i =0 ; p; p = p->next, i++) { +if (!has_same && changes[i] ==FOLLOW_PATH_ADDED ) +/* Never the same, added here: expensive rename detection */ +propagate_follow_pathspec_to_parent (opt, commit, p->item); +else +remember_follow_pathspec (opt, p->item, path); + } +free (changes); +} +/* * Show the diff of a commit. *
@@ -1303,10 +1371,8 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)if (opt->diffopt.flags.follow_renames) {struct commit_list *parents =get_saved_parents (opt, commit);if (parents && parents->next) { -struct commit_list *p; -for (p = parents; p; p = p->next) -propagate_follow_pathspec_to_parent (opt, commit, - p->item); +propagate_follow_pathspec_to_parents (opt, commit, + parents); }else if (parents) {remember_follow_pathspec (opt, parents->item,pathspec_single_path (&opt->diffopt.pathspec));
Signature · 1 line
2.56.0
Re: [PATCH] log: speed up --follow following renames for non-linear history
Quoted 9 lines · Miklos Vajna
Test the performance of 'git log --stat --follow --
include/linux/rsc_table.h' on linux.git, it used to take about 28 seconds, and since commit304812ed33(log: improve --follow following renames for non-linear history, 2026-06-22), it takes 6m43s.What happens is that
propagate_follow_pathspec_to_parent()does a somewhat expensive rename detection of the current commit vs all of its parents, and this slows down 'git log --follow'.
Quoted 8 lines
Fix the problem by introducing two shortcuts:
1) Only do that when the path is added relative to a parent and
2) If the path is the same as in one of the parents, then just follow that historyThese shortcuts keep all the uses-cases in
t4219-log-follow-merge.shworking, but now the above use case takes 29s, close to the original cost.
The original follow-renames implementation kicked in only when the path being followed did not exist in the parent. log_tree_commit() calls log_tree_diff() to show the diff for the current commit, which calls diff_tree_oid(), which checks with diff_might_be_rename() to see if the path we have been following is missing in the parent and only then runs try_to_follow_renames(), which is expensive, to find out where it came from. It avoided the expensive second diff when we know the path we have been following existed in the parent.
But reading log_tree_commit() in today's code, it seems that propagate_follow_pathspec_to_parent() is called, after we have ALREADY spent cycles to run the log_tree_diff() and flushed its output. The precious clue that tells you if the path you have been following is shared with the parent has already been flushed at that point.
Perhaps inside log_tree_diff_flush() while we still have the list of filepairs in diff_queue is where you should be able to see if any of the paths you have been following appeared anew relative to the parent(s) and to decide if you must adjust the list of paths you follow for renames, instead of calling the diff machinery all the time from propagate_follow_pathspec_to_parent() unconditionally.
I am not sure if it may be cleaner to first rip out all the change introduced in 304812ed33 (log: improve --follow following renames for non-linear history, 2026-06-22) and then rethink the approach, instead of piling more code with additional patches. I dunno.
Thanks.
Quoted 95 lines
log-tree.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 4 deletions(-)log-tree.c +70
-4@@ -1180,6 +1180,74 @@ static void propagate_follow_pathspec_to_parent(struct rev_info *opt,diff_free (&diff_opts); } +enum follow_path_change { +FOLLOW_PATH_SAME , +FOLLOW_PATH_ADDED , +FOLLOW_PATH_OTHER , +}; + +/* How does the followed path change from parent to commit? */ +static enum follow_path_change get_follow_path_change (struct rev_info *opt, +struct commit *commit, +struct commit *parent) +{ +struct diff_options diff_opts; +enum follow_path_change ret =FOLLOW_PATH_OTHER ; + +parse_commit_or_die (parent); +repo_diff_setup (opt->diffopt.repo, &diff_opts); +copy_pathspec (&diff_opts.pathspec, &opt->diffopt.pathspec); + diff_opts.flags.recursive =1 ; + diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; +diff_setup_done (&diff_opts); +diff_tree_oid (get_commit_tree_oid (parent), +get_commit_tree_oid (commit), +"" , &diff_opts); + +if (!diff_queued_diff.nr) + ret =FOLLOW_PATH_SAME ; +else if (diff_queued_diff.nr ==1 && + !DIFF_FILE_VALID (diff_queued_diff.queue[0 ]->one)) + ret =FOLLOW_PATH_ADDED ; + +diff_queue_clear (&diff_queued_diff); +diff_free (&diff_opts); +return ret; +} + +/* Compute a path to follow in each parent of a merge */ +static void propagate_follow_pathspec_to_parents (struct rev_info *opt, +struct commit *commit, +struct commit_list *parents) +{ +const char *path =pathspec_single_path (&opt->diffopt.pathspec); +struct commit_list *p; +enum follow_path_change *changes; +int nr =commit_list_count (parents), i; +int has_same =0 ; + +if (!path) +return ; + +ALLOC_ARRAY (changes, nr); +for (p = parents, i =0 ; p; p = p->next, i++) { + changes[i] =get_follow_path_change (opt, commit, p->item); +if (changes[i] ==FOLLOW_PATH_SAME ) { + has_same =1 ; +break ; + } + } + +for (p = parents, i =0 ; p; p = p->next, i++) { +if (!has_same && changes[i] ==FOLLOW_PATH_ADDED ) +/* Never the same, added here: expensive rename detection */ +propagate_follow_pathspec_to_parent (opt, commit, p->item); +else +remember_follow_pathspec (opt, p->item, path); + } +free (changes); +} +/* * Show the diff of a commit. * @@ -1303,10 +1371,8 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)if (opt->diffopt.flags.follow_renames) {struct commit_list *parents =get_saved_parents (opt, commit);if (parents && parents->next) { -struct commit_list *p; -for (p = parents; p; p = p->next) -propagate_follow_pathspec_to_parent (opt, commit, - p->item); +propagate_follow_pathspec_to_parents (opt, commit, + parents); }else if (parents) {remember_follow_pathspec (opt, parents->item,pathspec_single_path (&opt->diffopt.pathspec));
PATCH v2 1/1 log: speed up --follow following renames for non-linear history
Test the performance of 'git log --stat --follow -- include/linux/rsc_table.h' on linux.git, it used to take about 28 seconds, and since commit 304812ed33 (log: improve --follow following renames for non-linear history, 2026-06-22), it takes 6m43s.
What happens is that propagate_follow_pathspec_to_parent() diffs the current commit against all of its parents with rename following, and this slows down 'git log --follow'.
Fix the problem by removing propagate_follow_pathspec_to_parent() and instead recording the path to follow for each parent in the parent loop of log_tree_diff(), which already calls diff_tree_oid() with rename following, so for non-merge commits the diff that we show is reused. Merges are not diffed by default, so in that case run the same loop, but only to find the paths to follow, without showing anything. With -c, --cc or --remerge-diff, merges are diffed, but not in this loop, so run the loop for them as well and only show their merge diff after that, using the path of the merge itself. Also, do this in two passes: first look for a parent without rename following, and if the path is unchanged against a parent, then just follow that history in all parents. Only do the expensive rename following against each parent if the path changed relative to all parents. Given that this way a parent may be asked to follow a path that it doesn't have, also adjust remember_follow_pathspec(): if children disagree on what should be the path for a commit, then prefer the path that exists in that commit.
Following the same path in all parents is not enough when the other side renamed the path, though: if the merge kept F from one parent, but the other parent renamed F to G and then modified G, then the commits modifying G would be lost. Handle this by still doing the rename following against a parent if that parent doesn't have the path, the merge has it and a merge base of the two parents also has it, i.e. the path was renamed or removed on that side. The merge base is only computed in this rare case, and back-merges where the path was created after the other side forked still skip the expensive rename detection. Now the above use case takes 37s, somewhat close to the original cost.
Also add new testcases: the -c/--cc/--remerge-diff options and the --no-ff merging case were working, but were untested, so the optimization could have broken them. And cover following a rename on the side the merge did not take as well.
---
Hi Junio,
Quoted 4 lines · Junio C Hamano
I am not sure if it may be cleaner to first rip out all the change introduced in
304812ed33(log: improve --follow following renames for non-linear history, 2026-06-22) and then rethink the approach, instead of piling more code with additional patches. I dunno.
Here is my attempt at the redo approach: instead of adding more diffing on top of 304812ed33, I removed its propagate_follow_pathspec_to_parent() and I do the path recording in the existing parent loop of log_tree_diff(). For merges, there is no diff to reuse by default, so the same loop runs for them without showing anything.
Most of the performance benefit from v1 is still there.
I also found a few cases which had no test coverage, I added tests to t4219-log-follow-merge.sh.
Quoted 9 lines · Junio C Hamano
Hmph, so if you have two branches A and B that forked from a common ancestor that had file F,
- A updates F in place
- B renames F to G in one commit
- B then updates the renamed G on in another commit
- A merges B but decides to keep the original name F and original content, deleting G.
Quoted 6 lines
then because
has_samewould be the same, you do not callpropagate_follow_pathspec_to_parent()to prepare the traversal into branch B's history when you cross the merge, even when we are not simplifying the history?That somehow sounds broken to me. Am I missing something?
I didn't consider that case. I've now added a follow_needs_renames() to handle this.
This leads to a larger patch: should I extract the "rethink the approach" and the new tests to separate patches or it doesn't matter?
To sum up, compared to v1, now I 1) redid the approach to avoid propagate_follow_pathspec_to_parent() 2) added a few more tests and 3) the above "following a rename on the side the merge did not take" case now works as well.
Thanks,
Miklos
log-tree.c | 201 +++++++++++++++++++++++++----------- t/t4219-log-follow-merge.sh | 112 ++++++++++++++++++++ 2 files changed, 255 insertions(+), 58 deletions(-)
log-tree.c +143 -58
@@ -31,6 +31,7 @@#include "range-diff.h" #include "strmap.h" #include "tree.h" +#include "tree-walk.h" #include "wildmatch.h" #include "write-or-die.h" #include "pager.h"
@@ -1110,6 +1111,17 @@ static void set_pathspec_to_single_path(struct pathspec *ps, const char *path) PATHSPEC_LITERAL_PATH, "", paths); } +static int commit_has_path (struct rev_info *opt,struct commit *c, +const char *path) +{ +struct object_id oid; +unsigned short mode; + +parse_commit_or_die (c); +return !get_tree_entry (opt->diffopt.repo,get_commit_tree_oid (c), path, + &oid, &mode); +} +static void remember_follow_pathspec (struct rev_info *opt,struct commit *c,const char *path) {
@@ -1122,8 +1134,12 @@ static void remember_follow_pathspec(struct rev_info *opt,init_follow_pathspec_slab (opt->follow_pathspec_slab); } slot =follow_pathspec_slab_at (opt->follow_pathspec_slab, c); -if (*slot && !strcmp (*slot, path)) -return ; +/* If children of 'c' disagree on the path, prefer the one 'c' has. */ +if (*slot) { +if (!strcmp (*slot, path) ||commit_has_path (opt, c, *slot) || + !commit_has_path (opt, c, path)) +return ; + }free (*slot); *slot =xstrdup (path); }
@@ -1153,31 +1169,47 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } -/* Compute a path to follow in parent, if there is one */ -static void propagate_follow_pathspec_to_parent (struct rev_info *opt, -struct commit *commit, -struct commit *parent) +/* +* When the path in a merge is the same as in 'same', does the path still need +* renames to be followed in 'parent'? Only if the merge has the path, 'parent' +* doesn't have it, but its fork point with 'same' had it, e.g. 'parent' +* renamed the path. +*/ +static int follow_needs_renames (struct rev_info *opt,struct commit *parent, +struct commit *same,const char *path) +{ +struct commit_list *bases =NULL , *base; +int ret =0 ; + +if (commit_has_path (opt, parent, path) || + !commit_has_path (opt, same, path)) +return 0 ; +if (repo_get_merge_bases (opt->diffopt.repo, parent, same, &bases) <0 ) +exit (128 ); +for (base = bases; base && !ret; base = base->next) + ret =commit_has_path (opt, base->item, path); +commit_list_free (bases); +return ret; +} + +/* Show the remerge or combined diff of a merge commit */ +static int log_tree_merge_diff (struct rev_info *opt,struct commit *commit, +struct commit_list *parents, +struct object_id *oid) { -struct diff_options diff_opts; -const char *path; - -parse_commit_or_die (parent); -repo_diff_setup (opt->diffopt.repo, &diff_opts); -copy_pathspec (&diff_opts.pathspec, &opt->diffopt.pathspec); - diff_opts.flags.recursive =1 ; - diff_opts.flags.follow_renames =1 ; - diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; -diff_setup_done (&diff_opts); -diff_tree_oid (get_commit_tree_oid (parent), -get_commit_tree_oid (commit), -"" , &diff_opts); - - path =pathspec_single_path (&diff_opts.pathspec); -if (path) -remember_follow_pathspec (opt, parent, path); - -diff_queue_clear (&diff_queued_diff); -diff_free (&diff_opts); +int octopus = (parents->next->next !=NULL ); + +if (opt->remerge_diff) { +if (octopus) { +show_log (opt); +fprintf (opt->diffopt.file, +"diff: warning: Skipping remerge-diff " +"for octopus merges.\n" ); +return 1 ; + } +return do_remerge_diff (opt, parents, oid); + } +return do_diff_combined (opt, commit); }/*
@@ -1188,10 +1220,12 @@ static void propagate_follow_pathspec_to_parent(struct rev_info *opt,static int log_tree_diff (struct rev_info *opt,struct commit *commit,struct log_info *log) {int showed_log; -struct commit_list *parents; +struct commit_list *parents, *all_parents, *rename_parents =NULL ;struct object_id *oid;int is_merge;int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status; +int follow_only =0 ; +char *path =NULL ;if (!all_need_diff && !opt->merges_need_diff)return 0 ;
@@ -1220,47 +1254,113 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log }if (is_merge) { -int octopus = (parents->next->next !=NULL ); - -if (opt->remerge_diff) { -if (octopus) { -show_log (opt); -fprintf (opt->diffopt.file, -"diff: warning: Skipping remerge-diff " -"for octopus merges.\n" ); -return 1 ; - } -return do_remerge_diff (opt, parents, oid); - } -if (opt->combine_merges) -return do_diff_combined (opt, commit); -if (opt->separate_merges) { +int merge_diff = opt->remerge_diff || opt->combine_merges; + +if (merge_diff && !opt->diffopt.flags.follow_renames) +return log_tree_merge_diff (opt, commit, parents, oid); +if (opt->separate_merges && !merge_diff) {if (!opt->first_parent_merges) {/* Show parent info for multiple diffs */ log->parent = parents->item; } + }else if (opt->diffopt.flags.follow_renames) { +/* Diff only to find the path to follow in parents */ + follow_only =1 ; }else return 0 ; } +if (opt->diffopt.flags.follow_renames) + path =xstrdup_or_null ( +pathspec_single_path (&opt->diffopt.pathspec)); + +if (follow_only) { +/* +* First look for a parent where the path is unchanged, without +* the expensive rename detection. +*/ + opt->diffopt.flags.follow_renames =0 ; + } + showed_log =0 ; + all_parents = parents;for (;;) {struct commit *parent = parents->item; +int changed =1 ;parse_commit_or_die (parent);diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &opt->diffopt); -log_tree_diff_flush (opt); +if (follow_only) { + changed = diff_queued_diff.nr; +diff_queue_clear (&diff_queued_diff); + opt->diffopt.found_follow =0 ; + }else +log_tree_diff_flush (opt); showed_log |= !opt->loginfo; +if (!changed) { +/* +* Same as in this parent, follow that history, but +* still look for renames in parents that had the path +* at the fork point. +*/ +struct commit_list **tail = &rename_parents; + +for (parents = all_parents; parents; + parents = parents->next) { +if (parents->item != parent && +follow_needs_renames (opt, parents->item, + parent, path)) + tail = &commit_list_insert (parents->item, + tail)->next; +else +remember_follow_pathspec (opt, + parents->item, + path); + } +if (!rename_parents) +break ; + opt->diffopt.flags.follow_renames =1 ; + parents = rename_parents; +continue ; + } + +/* The diff updated the pathspec in case of a rename */ +if (path && opt->diffopt.flags.follow_renames) +remember_follow_pathspec (opt, parent, +pathspec_single_path (&opt->diffopt.pathspec)); +/* Set up the log info for the next parent, if any.. */ parents = parents->next; +if (!parents && follow_only && + !opt->diffopt.flags.follow_renames) { +/* The path changed in all parents, look for renames. */ + opt->diffopt.flags.follow_renames =1 ; + parents = all_parents; + }if (!parents || opt->first_parent_merges)break ; +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path); +if (follow_only) +continue ; log->parent = parents->item; opt->loginfo = log; } +if (follow_only) + opt->diffopt.flags.follow_renames =1 ; +if (follow_only && (opt->remerge_diff || opt->combine_merges)) { +/* Show the merge diff with the path of this commit */ +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path); + showed_log =log_tree_merge_diff (opt, commit, all_parents, oid); + } +commit_list_free (rename_parents); +free (path);return showed_log; }
@@ -1298,21 +1398,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)fprintf (opt->diffopt.file,"\n%s\n" , opt->break_bar);if (shown)show_diff_of_diff (opt); - -/* Record what path each parent of this commit should use */ -if (opt->diffopt.flags.follow_renames) { -struct commit_list *parents =get_saved_parents (opt, commit); -if (parents && parents->next) { -struct commit_list *p; -for (p = parents; p; p = p->next) -propagate_follow_pathspec_to_parent (opt, commit, - p->item); - }else if (parents) { -remember_follow_pathspec (opt, parents->item, -pathspec_single_path (&opt->diffopt.pathspec)); - } - } - opt->loginfo =NULL ;maybe_flush_or_die (opt->diffopt.file,"stdout" ); opt->diffopt.no_free = no_free;
t/t4219-log-follow-merge.sh +112 -0
116 lines not on this page · raw message
Signature · 1 line
2.56.0
v3 cover log: speed up --follow following renames for non-linear history
Hi Junio,
Quoted 2 lines · Miklos Vajna
This leads to a larger patch: should I extract the "rethink the approach" and the new tests to separate patches or it doesn't matter?
Here is a series where I split the changes into 4 patches: add more tests, a refactor, then your suggestion to start over (with one more test) and finally the speedup.
Thanks,
Miklos
Miklos Vajna (4):
t4219: test more --follow cases across merges
log: extract showing the remerge/combined diff of a merge commit
log: simplify --follow following renames for non-linear history
log: speed up --follow following renames for non-linear history
log-tree.c | 201 +++++++++++++++++++++++++----------- t/t4219-log-follow-merge.sh | 128 +++++++++++++++++++++++ 2 files changed, 271 insertions(+), 58 deletions(-)
Interdiff against v2:
t/t4219-log-follow-merge.sh +16 -0
@@ -144,6 +144,22 @@ do ' done +test_expect_success '--follow follows renames through a fork with -m' ' + git -C diamond log --follow -m --pretty=tformat:%s path >log && + grep "^[A-Z]: " log | sort >actual && + cat >expect <<-\EOF && + A: add path0 + B: modify path0 on upper + C: modify path0 on lower + M: merge with rename to path + M: merge with rename to path + X: rename path0 to path1 + Y: rename path0 to path2 + Z: modify path + EOF + test_cmp expect actual +' + # Build a merge that takes the renamed path from the second parent, while the # first parent only has the old path. $2 decides which side is committed first, # which decides the walk order.
Signature · 1 line
2.56.0
PATCH v3 1/4 t4219: test more --follow cases across merges
A later commit in this series will speed up 'git log --follow' for merge commits. Add tests for cases which already work, but had no test coverage, so it's easy to break them while optimizing:
- --follow through a fork with renames on both sides, with -c, --cc and --remerge-diff
- a --no-ff merge taking a rename from the second parent, in both commit orders, since the result could depend on the walk order
- a merge keeping the path, while the other side renamed it
--- t/t4219-log-follow-merge.sh | 112 ++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+)
t/t4219-log-follow-merge.sh +112 -0
@@ -126,4 +126,116 @@ test_expect_success '--follow follows renames through a fork in a single history test_cmp expect actual.sorted ' +for opt in -c --cc --remerge-diff +do + test_expect_success "--follow follows renames through a fork with $opt" ' + git -C diamond log --follow $opt --pretty=tformat:%s path >log && + grep "^[A-Z]: " log | sort >actual && + cat >expect <<-\EOF && + A: add path0 + B: modify path0 on upper + C: modify path0 on lower + M: merge with rename to path + X: rename path0 to path1 + Y: rename path0 to path2 + Z: modify path + EOF + test_cmp expect actual + ' +done + +# Build a merge that takes the renamed path from the second parent, while the +# first parent only has the old path. $2 decides which side is committed first, +# which decides the walk order. +setup_rename_on_second_parent () { + git init $1 && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >$1/old && + git -C $1 add old && + test_tick && + git -C $1 commit -m "A: add old" && + printf "line 1\nline 2\nline 3 modified\nline 4\nline 5\n" >$1/old && + test_tick && + git -C $1 commit -am "B: modify old" && + git -C $1 branch topic && + + if test "$2" = topic-first + then + git -C $1 checkout topic && + git -C $1 mv old new && + test_tick && + git -C $1 commit -m "R: rename old to new" && + git -C $1 checkout master + fi && + + echo other >$1/other && + git -C $1 add other && + test_tick && + git -C $1 commit -m "M: unrelated change on master" && + + if test "$2" = master-first + then + git -C $1 checkout topic && + git -C $1 mv old new && + test_tick && + git -C $1 commit -m "R: rename old to new" && + git -C $1 checkout master + fi && + + test_tick && + git -C $1 merge --no-ff -m "merge topic" topic +} + +test_expect_success 'setup merge taking a rename from the second parent' ' + setup_rename_on_second_parent rename-master-first master-first && + setup_rename_on_second_parent rename-topic-first topic-first +' + +for order in master-first topic-first +do + test_expect_success "--follow follows a rename from the second parent ($order)" ' + git -C rename-$order log --follow --pretty=tformat:%s new >actual && + cat >expect <<-\EOF && + R: rename old to new + B: modify old + A: add old + EOF + test_cmp expect actual + ' +done + +test_expect_success 'setup merge keeping the path, other side renamed it' ' + git init keep && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep/F && + git -C keep add F && + test_tick && + git -C keep commit -m "O: add F" && + git -C keep branch B && + printf "line 1\nline 2\nline 3 A\nline 4\nline 5\n" >keep/F && + test_tick && + git -C keep commit -am "A1: update F in place" && + + git -C keep checkout B && + git -C keep mv F G && + test_tick && + git -C keep commit -m "B1: rename F to G" && + printf "line 1\nline 2\nline 3\nline 4 B\nline 5\n" >keep/G && + test_tick && + git -C keep commit -am "B2: update G" && + + git -C keep checkout master && + test_tick && + git -C keep merge -s ours -m "M: merge B, keep F, drop G" B +' + +test_expect_success '--follow follows a rename on the side the merge did not take' ' + git -C keep log --follow --pretty=tformat:%s F >actual && + cat >expect <<-\EOF && + B2: update G + B1: rename F to G + A1: update F in place + O: add F + EOF + test_cmp expect actual +' + test_done
Signature · 1 line
2.56.0
PATCH v3 2/4 log: extract showing the remerge/combined diff of a merge commit
In reply to Miklos VajnaNo functional changes intended. A later commit in this series will also call log_tree_merge_diff() after the parent loop of log_tree_diff(), so extract that code to a function to avoid duplicating it.
--- log-tree.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-)
log-tree.c +22 -14
@@ -1180,6 +1180,26 @@ static void propagate_follow_pathspec_to_parent(struct rev_info *opt,diff_free (&diff_opts); } +/* Show the remerge or combined diff of a merge commit */ +static int log_tree_merge_diff (struct rev_info *opt,struct commit *commit, +struct commit_list *parents, +struct object_id *oid) +{ +int octopus = (parents->next->next !=NULL ); + +if (opt->remerge_diff) { +if (octopus) { +show_log (opt); +fprintf (opt->diffopt.file, +"diff: warning: Skipping remerge-diff " +"for octopus merges.\n" ); +return 1 ; + } +return do_remerge_diff (opt, parents, oid); + } +return do_diff_combined (opt, commit); +} +/* * Show the diff of a commit. *
@@ -1220,20 +1240,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log }if (is_merge) { -int octopus = (parents->next->next !=NULL ); - -if (opt->remerge_diff) { -if (octopus) { -show_log (opt); -fprintf (opt->diffopt.file, -"diff: warning: Skipping remerge-diff " -"for octopus merges.\n" ); -return 1 ; - } -return do_remerge_diff (opt, parents, oid); - } -if (opt->combine_merges) -return do_diff_combined (opt, commit); +if (opt->remerge_diff || opt->combine_merges) +return log_tree_merge_diff (opt, commit, parents, oid);if (opt->separate_merges) {if (!opt->first_parent_merges) {/* Show parent info for multiple diffs */
Signature · 1 line
2.56.0
PATCH v3 3/4 log: simplify --follow following renames for non-linear history
In reply to Miklos VajnaCode added in 304812ed33 (log: improve --follow following renames for non-linear history, 2026-06-22) implemented the feature by introducing a new propagate_follow_pathspec_to_parent() function which adds a new call to diff_tree_oid(). Also it introduces a new loop to record what path each parent of a merge commit should use when following a path.
This was working, but fixing its performance would require adding even more custom code, instead of just extending the existing code in log_tree_diff().
Remove propagate_follow_pathspec_to_parent() and the loop calling it for each parent. Instead, record the path to follow for each parent in the parent loop of log_tree_diff(), right after diff_tree_oid() updated the pathspec in case of a rename. Merges are not diffed by default, so with --follow, run the same loop for them as well, but without showing anything. With -c, --cc or --remerge-diff, show the merge diff after the loop. This gets rid of the additional call to diff_tree_oid(). The rework has no positive performance impact yet, it just simplifies the code.
This also fixes 'git log -m --follow': -m diffs the merge against each parent with rename following, so a rename found against the first parent updated the followed path, and the second parent was then diffed with the already renamed path. Also, propagate_follow_pathspec_to_parent() started from that same updated path, so history was lost on the side of the second parent. Now the path of the merge is restored before diffing each parent. Add a test for this, which shows the full history of the diamond case with -m.
--- log-tree.c | 84 +++++++++++++++++-------------------- t/t4219-log-follow-merge.sh | 16 +++++++ 2 files changed, 54 insertions(+), 46 deletions(-)
log-tree.c +38 -46
@@ -1153,33 +1153,6 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } -/* Compute a path to follow in parent, if there is one */ -static void propagate_follow_pathspec_to_parent (struct rev_info *opt, -struct commit *commit, -struct commit *parent) -{ -struct diff_options diff_opts; -const char *path; - -parse_commit_or_die (parent); -repo_diff_setup (opt->diffopt.repo, &diff_opts); -copy_pathspec (&diff_opts.pathspec, &opt->diffopt.pathspec); - diff_opts.flags.recursive =1 ; - diff_opts.flags.follow_renames =1 ; - diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; -diff_setup_done (&diff_opts); -diff_tree_oid (get_commit_tree_oid (parent), -get_commit_tree_oid (commit), -"" , &diff_opts); - - path =pathspec_single_path (&diff_opts.pathspec); -if (path) -remember_follow_pathspec (opt, parent, path); - -diff_queue_clear (&diff_queued_diff); -diff_free (&diff_opts); -} -/* Show the remerge or combined diff of a merge commit */ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit, struct commit_list *parents,
@@ -1208,10 +1181,12 @@ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,static int log_tree_diff (struct rev_info *opt,struct commit *commit,struct log_info *log) {int showed_log; -struct commit_list *parents; +struct commit_list *parents, *all_parents;struct object_id *oid;int is_merge;int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status; +int follow_only =0 ; +char *path =NULL ;if (!all_need_diff && !opt->merges_need_diff)return 0 ;
@@ -1240,35 +1215,67 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log }if (is_merge) { -if (opt->remerge_diff || opt->combine_merges) +int merge_diff = opt->remerge_diff || opt->combine_merges; + +if (merge_diff && !opt->diffopt.flags.follow_renames)return log_tree_merge_diff (opt, commit, parents, oid); -if (opt->separate_merges) { +if (opt->separate_merges && !merge_diff) {if (!opt->first_parent_merges) {/* Show parent info for multiple diffs */ log->parent = parents->item; } + }else if (opt->diffopt.flags.follow_renames) { +/* Diff only to find the path to follow in parents */ + follow_only =1 ; }else return 0 ; } +if (opt->diffopt.flags.follow_renames) + path =xstrdup_or_null ( +pathspec_single_path (&opt->diffopt.pathspec)); + showed_log =0 ; + all_parents = parents;for (;;) {struct commit *parent = parents->item;parse_commit_or_die (parent);diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &opt->diffopt); -log_tree_diff_flush (opt); +if (follow_only) { +diff_queue_clear (&diff_queued_diff); + opt->diffopt.found_follow =0 ; + }else +log_tree_diff_flush (opt); showed_log |= !opt->loginfo; +/* The diff updated the pathspec in case of a rename */ +if (path) +remember_follow_pathspec (opt, parent, +pathspec_single_path (&opt->diffopt.pathspec)); +/* Set up the log info for the next parent, if any.. */ parents = parents->next;if (!parents || opt->first_parent_merges)break ; +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path); +if (follow_only) +continue ; log->parent = parents->item; opt->loginfo = log; } +if (follow_only && (opt->remerge_diff || opt->combine_merges)) { +/* Show the merge diff with the path of this commit */ +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path); + showed_log =log_tree_merge_diff (opt, commit, all_parents, oid); + } +free (path);return showed_log; }
@@ -1306,21 +1313,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)fprintf (opt->diffopt.file,"\n%s\n" , opt->break_bar);if (shown)show_diff_of_diff (opt); - -/* Record what path each parent of this commit should use */ -if (opt->diffopt.flags.follow_renames) { -struct commit_list *parents =get_saved_parents (opt, commit); -if (parents && parents->next) { -struct commit_list *p; -for (p = parents; p; p = p->next) -propagate_follow_pathspec_to_parent (opt, commit, - p->item); - }else if (parents) { -remember_follow_pathspec (opt, parents->item, -pathspec_single_path (&opt->diffopt.pathspec)); - } - } - opt->loginfo =NULL ;maybe_flush_or_die (opt->diffopt.file,"stdout" ); opt->diffopt.no_free = no_free;
t/t4219-log-follow-merge.sh +16 -0
@@ -144,6 +144,22 @@ do ' done +test_expect_success '--follow follows renames through a fork with -m' ' + git -C diamond log --follow -m --pretty=tformat:%s path >log && + grep "^[A-Z]: " log | sort >actual && + cat >expect <<-\EOF && + A: add path0 + B: modify path0 on upper + C: modify path0 on lower + M: merge with rename to path + M: merge with rename to path + X: rename path0 to path1 + Y: rename path0 to path2 + Z: modify path + EOF + test_cmp expect actual +' + # Build a merge that takes the renamed path from the second parent, while the # first parent only has the old path. $2 decides which side is committed first, # which decides the walk order.
Signature · 1 line
2.56.0
Re: [PATCH v3 3/4] log: simplify --follow following renames for non-linear history
Quoted 7 lines · Miklos Vajna
Code added in
304812ed33(log: improve --follow following renames for non-linear history, 2026-06-22) implemented the feature by introducing a newpropagate_follow_pathspec_to_parent()function which adds a new call todiff_tree_oid(). Also it introduces a new loop to record what path each parent of a merge commit should use when following a path.This was working, but fixing its performance would require adding even
It's somewhat curious that you start talking about performance here without even mentioning that performance is what breaks the current approach. You mention contributing factors to the performance regression without really calling it that.
Quoted 13 lines
more custom code, instead of just extending the existing code in
log_tree_diff().Remove
propagate_follow_pathspec_to_parent()and the loop calling it for each parent. Instead, record the path to follow for each parent in the parent loop oflog_tree_diff(), right afterdiff_tree_oid()updated the pathspec in case of a rename. Merges are not diffed by default, so with --follow, run the same loop for them as well, but without showing anything. With -c, --cc or --remerge-diff, show the merge diff after the loop. This gets rid of the additional call todiff_tree_oid(). The rework has no positive performance impact yet, it just simplifies the code.
Quoted 4 lines
This also fixes 'git log -m --follow': -m diffs the merge against each parent with rename following, so a rename found against the first parent updated the followed path, and the second parent was then diffed with the already renamed path.
So next to a performance regression there's also a bug? Interesting. I feel like giving a bit more time to introduce and explain the bug would have helped.
Quoted 5 lines
Also,
propagate_follow_pathspec_to_parent()
started from that same updated path, so history was lost on the side of the second parent. Now the path of the merge is restored before diffing each parent. Add a test for this, which shows the full history of the diamond case with -m.
And another bug?
So in summary we have two bugs and one performance regression? The two bugs are fixed by this change, and it also prepares us for the fix to the performance regression? It might be easier to reason through all of that if we were able to split this so that we fix one issue at a time, but that may of course not be possible (or just very awkward).
Quoted 34 lines
log-tree.c +12
-2@@ -1240,35 +1215,67 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log }if (is_merge) { -if (opt->remerge_diff || opt->combine_merges) +int merge_diff = opt->remerge_diff || opt->combine_merges; + +if (merge_diff && !opt->diffopt.flags.follow_renames)return log_tree_merge_diff (opt, commit, parents, oid); -if (opt->separate_merges) { +if (opt->separate_merges && !merge_diff) {if (!opt->first_parent_merges) {/* Show parent info for multiple diffs */ log->parent = parents->item; } + }else if (opt->diffopt.flags.follow_renames) { +/* Diff only to find the path to follow in parents */ + follow_only =1 ; }else return 0 ; } +if (opt->diffopt.flags.follow_renames) + path =xstrdup_or_null ( +pathspec_single_path (&opt->diffopt.pathspec)); + showed_log =0 ; + all_parents = parents;for (;;) {struct commit *parent = parents->item;
log-tree.c static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log · line 1250
Quoted 10 lines
log-tree.c +5
-1parse_commit_or_die (parent);diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &opt->diffopt); -log_tree_diff_flush (opt); +if (follow_only) { +diff_queue_clear (&diff_queued_diff); + opt->diffopt.found_follow =0 ; + }else +log_tree_diff_flush (opt);
Nit: style, if once condition body requires curly braces all of them should have it. Sorry, I cannot really provide more valuable feedback on this patch than this.
Patrick
PATCH v3 4/4 log: speed up --follow following renames for non-linear history
In reply to Miklos VajnaTest the performance of 'git log --stat --follow -- include/linux/rsc_table.h' on linux.git, it used to take about 28 seconds, and since commit 304812ed33 (log: improve --follow following renames for non-linear history, 2026-06-22), it takes 6m43s.
What happens is that log_tree_diff() diffs each merge against all of its parents with rename following. diff_tree_oid() only does the expensive rename detection when the path is added relative to a parent, but that is common for merges, e.g. when a branch forked before the path was created merges mainline, and this slows down 'git log --follow'.
Fix the problem by diffing the merge against its parents in two passes:
- First pass: diff against the parents without rename following. If the path is unchanged against a parent, then just follow that history in all parents.
- Except when an other parent doesn't have the path, the merge has it and a merge base of the two parents also has it, i.e. the path was renamed or removed on that side: still do the rename following against such a parent, otherwise the history on that side would be lost. The merge base is only computed in this rare case, and back-merges where the path was created after the other side forked still skip the expensive rename detection.
- Second pass: only do the rename following against each parent if the path changed relative to all parents.
Given that this way a parent may be asked to follow a path that it doesn't have, also adjust remember_follow_pathspec(): if children disagree on what should be the path for a commit, then prefer the path that exists in that commit.
Now the above use case takes 36s, somewhat close to the original cost.
--- log-tree.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 89 insertions(+), 4 deletions(-)
log-tree.c +89 -4
@@ -31,6 +31,7 @@#include "range-diff.h" #include "strmap.h" #include "tree.h" +#include "tree-walk.h" #include "wildmatch.h" #include "write-or-die.h" #include "pager.h"
@@ -1110,6 +1111,17 @@ static void set_pathspec_to_single_path(struct pathspec *ps, const char *path) PATHSPEC_LITERAL_PATH, "", paths); } +static int commit_has_path (struct rev_info *opt,struct commit *c, +const char *path) +{ +struct object_id oid; +unsigned short mode; + +parse_commit_or_die (c); +return !get_tree_entry (opt->diffopt.repo,get_commit_tree_oid (c), path, + &oid, &mode); +} +static void remember_follow_pathspec (struct rev_info *opt,struct commit *c,const char *path) {
@@ -1122,8 +1134,12 @@ static void remember_follow_pathspec(struct rev_info *opt,init_follow_pathspec_slab (opt->follow_pathspec_slab); } slot =follow_pathspec_slab_at (opt->follow_pathspec_slab, c); -if (*slot && !strcmp (*slot, path)) -return ; +/* If children of 'c' disagree on the path, prefer the one 'c' has. */ +if (*slot) { +if (!strcmp (*slot, path) ||commit_has_path (opt, c, *slot) || + !commit_has_path (opt, c, path)) +return ; + }free (*slot); *slot =xstrdup (path); }
@@ -1153,6 +1169,29 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } +/* +* When the path in a merge is the same as in 'same', does the path still need +* renames to be followed in 'parent'? Only if the merge has the path, 'parent' +* doesn't have it, but its fork point with 'same' had it, e.g. 'parent' +* renamed the path. +*/ +static int follow_needs_renames (struct rev_info *opt,struct commit *parent, +struct commit *same,const char *path) +{ +struct commit_list *bases =NULL , *base; +int ret =0 ; + +if (commit_has_path (opt, parent, path) || + !commit_has_path (opt, same, path)) +return 0 ; +if (repo_get_merge_bases (opt->diffopt.repo, parent, same, &bases) <0 ) +exit (128 ); +for (base = bases; base && !ret; base = base->next) + ret =commit_has_path (opt, base->item, path); +commit_list_free (bases); +return ret; +} +/* Show the remerge or combined diff of a merge commit */ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit, struct commit_list *parents,
@@ -1181,7 +1220,7 @@ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,static int log_tree_diff (struct rev_info *opt,struct commit *commit,struct log_info *log) {int showed_log; -struct commit_list *parents, *all_parents; +struct commit_list *parents, *all_parents, *rename_parents =NULL ;struct object_id *oid;int is_merge;int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;
@@ -1235,15 +1274,25 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log path =xstrdup_or_null (pathspec_single_path (&opt->diffopt.pathspec)); +if (follow_only) { +/* +* First look for a parent where the path is unchanged, without +* the expensive rename detection. +*/ + opt->diffopt.flags.follow_renames =0 ; + } + showed_log =0 ; all_parents = parents;for (;;) {struct commit *parent = parents->item; +int changed =1 ;parse_commit_or_die (parent);diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &opt->diffopt);if (follow_only) { + changed = diff_queued_diff.nr;diff_queue_clear (&diff_queued_diff); opt->diffopt.found_follow =0 ; }else
@@ -1251,13 +1300,46 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log showed_log |= !opt->loginfo; +if (!changed) { +/* +* Same as in this parent, follow that history, but +* still look for renames in parents that had the path +* at the fork point. +*/ +struct commit_list **tail = &rename_parents; + +for (parents = all_parents; parents; + parents = parents->next) { +if (parents->item != parent && +follow_needs_renames (opt, parents->item, + parent, path)) + tail = &commit_list_insert (parents->item, + tail)->next; +else +remember_follow_pathspec (opt, + parents->item, + path); + } +if (!rename_parents) +break ; + opt->diffopt.flags.follow_renames =1 ; + parents = rename_parents; +continue ; + } +/* The diff updated the pathspec in case of a rename */ -if (path) +if (path && opt->diffopt.flags.follow_renames)remember_follow_pathspec (opt, parent,pathspec_single_path (&opt->diffopt.pathspec));/* Set up the log info for the next parent, if any.. */ parents = parents->next; +if (!parents && follow_only && + !opt->diffopt.flags.follow_renames) { +/* The path changed in all parents, look for renames. */ + opt->diffopt.flags.follow_renames =1 ; + parents = all_parents; + }if (!parents || opt->first_parent_merges)break ;if (path)
@@ -1268,6 +1350,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log log->parent = parents->item; opt->loginfo = log; } +if (follow_only) + opt->diffopt.flags.follow_renames =1 ;if (follow_only && (opt->remerge_diff || opt->combine_merges)) {/* Show the merge diff with the path of this commit */ if (path)
@@ -1275,6 +1359,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log path); showed_log =log_tree_merge_diff (opt, commit, all_parents, oid); } +commit_list_free (rename_parents);free (path);return showed_log; }
Signature · 1 line
2.56.0
Re: [PATCH v3 4/4] log: speed up --follow following renames for non-linear history
Quoted 13 lines · Miklos Vajna
log-tree.c +5
-0@@ -1153,6 +1169,29 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } +/* +* When the path in a merge is the same as in 'same', does the path still need +* renames to be followed in 'parent'? Only if the merge has the path, 'parent' +* doesn't have it, but its fork point with 'same' had it, e.g. 'parent' +* renamed the path.
log-tree.c void release_follow_pathspec_slab(struct rev_info *opt) · line 1186
Quoted 10 lines
log-tree.c +10
-0+*/ +static int follow_needs_renames (struct rev_info *opt,struct commit *parent, +struct commit *same,const char *path) +{ +struct commit_list *bases =NULL , *base; +int ret =0 ; + +if (commit_has_path (opt, parent, path) || + !commit_has_path (opt, same, path)) +return 0 ;
Doesn't this miss the reverse case where we have the path in "parent", but not in "same"? So basically this test case, which is very similar to the one you already added to 4219:
test_expect_success 'setup merge keeping the renamed path, other side edited the old one' '
git init keep-renamed && printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-renamed/F && git -C keep-renamed add F && test_tick && git -C keep-renamed commit -m "O: add F" && git -C keep-renamed branch B && git -C keep-renamed mv F G && test_tick && git -C keep-renamed commit -m "A1: rename F to G on master" && git -C keep-renamed checkout B && printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-renamed/F && test_tick && git -C keep-renamed commit -am "B2: update F on B" && git -C keep-renamed checkout master && test_tick && git -C keep-renamed merge -s ours -m "M: merge B, keep G, drop F" B
'
test_expect_success '--follow follows an edit on the side the merge did not take' '
git -C keep-renamed log --follow --pretty=tformat:%s G >actual && cat >expect <<-\EOF &&
B2: update F on B
A1: rename F to G on master
O: add F
EOF
test_cmp expect actual
'
That test passes with HEAD~, but fails with HEAD.
Patrick
Re: [PATCH v3 0/4] log: speed up --follow following renames for non-linear history
In reply to Miklos VajnaQuoted 9 lines · Miklos Vajna
Hi Junio,
Quoted 2 lines · Miklos Vajna
This leads to a larger patch: should I extract the "rethink the approach" and the new tests to separate patches or it doesn't matter?
Here is a series where I split the changes into 4 patches: add more tests, a refactor, then your suggestion to start over (with one more test) and finally the speedup.
Thanks for your work.
I cannot really provide much of value as I'm way out of my depth here, but will try to do so anyway as the regression has now caused an incident for GitLab.com. Hope to get this resolved sooner rather than later so that I can ideally avoid backporting patches in favor of getting this series landed.
Patrick
v4 cover log: speed up --follow following renames for non-linear history
In reply to Miklos VajnaHi Patrick,
Thanks for the reviews of v3. Changes in v4:
- The -m --follow fix is now a separate patch (3/6), so the simplify patch (4/6) no longer changes behavior. The two bugs you pointed out in the old commit message have one cause: the followed path is not restored between the parents of a merge. So 3/6 fixes both. The commit message of 4/6 now says upfront that 304812ed33 (log: improve --follow following renames for non-linear history, 2026-06-22) caused a performance regression.
- Fixed the brace style nit in the simplify patch.
- Patch 6/6 fixes the case you found, where the speedup lost history:
one side of the merge renamed the path, and the other side, whose version the merge dropped, edited the old name. It uses your test. I kept it separate from the speedup patch, which is complicated enough already. With the extra check, the linux.git case still takes about 37s.
- I also noticed that the simplify patch lost history with --diff-merges=first-parent, which diffs merges only against their first parent, but still walks all parents. Fixed that and added a test for it to 1/6.
Thanks,
Miklos
Miklos Vajna (6):
t4219: test more --follow cases across merges
log: extract showing the remerge/combined diff of a merge commit
log: fix -m --follow losing the history of later parents
log: simplify --follow following renames for non-linear history
log: speed up --follow following renames for non-linear history
log: fix --follow following an edit on a side the merge did not take
log-tree.c | 265 +++++++++++++++++++++++++++++------- t/t4219-log-follow-merge.sh | 171 +++++++++++++++++++++++ 2 files changed, 388 insertions(+), 48 deletions(-)
Interdiff against v3:
log-tree.c +96 -12
@@ -1111,15 +1111,20 @@ static void set_pathspec_to_single_path(struct pathspec *ps, const char *path) PATHSPEC_LITERAL_PATH, "", paths); } -static int commit_has_path (struct rev_info *opt,struct commit *c, -const char *path) +static int tree_has_path (struct rev_info *opt,struct object_id *tree, +const char *path) {struct object_id oid;unsigned short mode; +return !get_tree_entry (opt->diffopt.repo, tree, path, &oid, &mode); +} + +static int commit_has_path (struct rev_info *opt,struct commit *c, +const char *path) +{parse_commit_or_die (c); -return !get_tree_entry (opt->diffopt.repo,get_commit_tree_oid (c), path, - &oid, &mode); +return tree_has_path (opt,get_commit_tree_oid (c), path); } static void remember_follow_pathspec(struct rev_info *opt,
@@ -1169,26 +1174,97 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } +/* +* Did 'parent' change a file since 'base' that the merge (with tree 'oid') no +* longer has? Only such a file can be the source of a rename on the other side +* of the merge that still needs to be followed in 'parent'. +*/ +static int follow_parent_changed_dropped_file (struct rev_info *opt, +struct commit *base, +struct commit *parent, +struct object_id *oid) +{ +struct diff_options diff_opts; +int i, ret =0 ; + +repo_diff_setup (opt->diffopt.repo, &diff_opts); + diff_opts.flags.recursive =1 ; + diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; +diff_setup_done (&diff_opts); +diff_tree_oid (get_commit_tree_oid (base),get_commit_tree_oid (parent), +"" , &diff_opts); +for (i =0 ; i < diff_queued_diff.nr && !ret; i++) { +struct diff_filepair *p = diff_queued_diff.queue[i]; + + ret =DIFF_FILE_VALID (p->two) && + !tree_has_path (opt, oid, p->two->path); + } +diff_queue_clear (&diff_queued_diff); +diff_free (&diff_opts); +return ret; +} + +/* +* Is 'path' in the merge (with tree 'oid') renamed from a file of 'parent'? +* Only look at the files that the merge deleted, not at all files of 'parent' +* like --follow does. +*/ +static int follow_has_rename_source (struct rev_info *opt, +struct commit *parent, +struct object_id *oid,const char *path) +{ +struct diff_options diff_opts; +int i, ret =0 ; + +repo_diff_setup (opt->diffopt.repo, &diff_opts); + diff_opts.flags.recursive =1 ; + diff_opts.detect_rename =DIFF_DETECT_RENAME ; + diff_opts.rename_score = opt->diffopt.rename_score; + diff_opts.single_follow = path; + diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; +diff_setup_done (&diff_opts); +diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &diff_opts); +diffcore_std (&diff_opts); +for (i =0 ; i < diff_queued_diff.nr && !ret; i++) { +struct diff_filepair *p = diff_queued_diff.queue[i]; + + ret = p->status ==DIFF_STATUS_RENAMED && + !strcmp (p->two->path, path); + } +diff_queue_clear (&diff_queued_diff); +diff_free (&diff_opts); +return ret; +} +/* * When the path in a merge is the same as in 'same', does the path still need * renames to be followed in 'parent'? Only if the merge has the path, 'parent' -* doesn't have it, but its fork point with 'same' had it, e.g. 'parent' -* renamed the path. +* doesn't have it, and either: +* +* - its fork point with 'same' had it, e.g. 'parent' renamed the path, or +* - 'same' renamed the path from a file that 'parent' changed. */ static int follow_needs_renames (struct rev_info *opt,struct commit *parent, -struct commit *same,const char *path) +struct commit *same,struct object_id *oid, +const char *path) {struct commit_list *bases =NULL , *base; -int ret =0 ; +int ret =0 , changed_dropped =0 ;if (commit_has_path (opt, parent, path) || !commit_has_path (opt, same, path))return 0 ;if (repo_get_merge_bases (opt->diffopt.repo, parent, same, &bases) <0 )exit (128 ); -for (base = bases; base && !ret; base = base->next) +for (base = bases; base && !ret; base = base->next) { ret =commit_has_path (opt, base->item, path); +if (!ret && !changed_dropped) + changed_dropped =follow_parent_changed_dropped_file ( + opt, base->item, parent, oid); + }commit_list_free (bases); +if (!ret && changed_dropped) + ret =follow_has_rename_source (opt, parent, oid, path);return ret; }
@@ -1295,8 +1371,9 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log changed = diff_queued_diff.nr;diff_queue_clear (&diff_queued_diff); opt->diffopt.found_follow =0 ; - }else + }else {log_tree_diff_flush (opt); + } showed_log |= !opt->loginfo;
@@ -1312,7 +1389,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log parents = parents->next) {if (parents->item != parent &&follow_needs_renames (opt, parents->item, - parent, path)) + parent, oid, path)) tail = &commit_list_insert (parents->item, tail)->next;else
@@ -1340,8 +1417,15 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log opt->diffopt.flags.follow_renames =1 ; parents = all_parents; } -if (!parents || opt->first_parent_merges) +if (!parents)break ; +if (opt->first_parent_merges && !follow_only) { +if (!path || opt->first_parent_only) +break ; +/* Only find the path to follow in the other parents */ + follow_only =1 ; + opt->diffopt.flags.follow_renames =0 ; + }if (path)set_pathspec_to_single_path (&opt->diffopt.pathspec, path);
t/t4219-log-follow-merge.sh +43 -0
47 lines not on this page · raw message
Signature · 1 line
2.56.0
PATCH v4 1/6 t4219: test more --follow cases across merges
A later commit in this series will speed up 'git log --follow' for merge commits. Add tests for cases which already work, but had no test coverage, so it's easy to break them while optimizing:
- --follow through a fork with renames on both sides, with -c, --cc and --remerge-diff
- a --no-ff merge taking a rename from the second parent, in both commit orders, since the result could depend on the walk order
- a merge keeping the path, while the other side renamed it
- --diff-merges=first-parent, which diffs merges only against their first parent, but still walks all parents
--- t/t4219-log-follow-merge.sh | 124 ++++++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+)
t/t4219-log-follow-merge.sh +124 -0
@@ -126,4 +126,128 @@ test_expect_success '--follow follows renames through a fork in a single history test_cmp expect actual.sorted ' +for opt in -c --cc --remerge-diff +do + test_expect_success "--follow follows renames through a fork with $opt" ' + git -C diamond log --follow $opt --pretty=tformat:%s path >log && + grep "^[A-Z]: " log | sort >actual && + cat >expect <<-\EOF && + A: add path0 + B: modify path0 on upper + C: modify path0 on lower + M: merge with rename to path + X: rename path0 to path1 + Y: rename path0 to path2 + Z: modify path + EOF + test_cmp expect actual + ' +done + +# Build a merge that takes the renamed path from the second parent, while the +# first parent only has the old path. $2 decides which side is committed first, +# which decides the walk order. +setup_rename_on_second_parent () { + git init $1 && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >$1/old && + git -C $1 add old && + test_tick && + git -C $1 commit -m "A: add old" && + printf "line 1\nline 2\nline 3 modified\nline 4\nline 5\n" >$1/old && + test_tick && + git -C $1 commit -am "B: modify old" && + git -C $1 branch topic && + + if test "$2" = topic-first + then + git -C $1 checkout topic && + git -C $1 mv old new && + test_tick && + git -C $1 commit -m "R: rename old to new" && + git -C $1 checkout master + fi && + + echo other >$1/other && + git -C $1 add other && + test_tick && + git -C $1 commit -m "M: unrelated change on master" && + + if test "$2" = master-first + then + git -C $1 checkout topic && + git -C $1 mv old new && + test_tick && + git -C $1 commit -m "R: rename old to new" && + git -C $1 checkout master + fi && + + test_tick && + git -C $1 merge --no-ff -m "merge topic" topic +} + +test_expect_success 'setup merge taking a rename from the second parent' ' + setup_rename_on_second_parent rename-master-first master-first && + setup_rename_on_second_parent rename-topic-first topic-first +' + +for order in master-first topic-first +do + test_expect_success "--follow follows a rename from the second parent ($order)" ' + git -C rename-$order log --follow --pretty=tformat:%s new >actual && + cat >expect <<-\EOF && + R: rename old to new + B: modify old + A: add old + EOF + test_cmp expect actual + ' +done + +test_expect_success 'setup merge keeping the path, other side renamed it' ' + git init keep && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep/F && + git -C keep add F && + test_tick && + git -C keep commit -m "O: add F" && + git -C keep branch B && + printf "line 1\nline 2\nline 3 A\nline 4\nline 5\n" >keep/F && + test_tick && + git -C keep commit -am "A1: update F in place" && + + git -C keep checkout B && + git -C keep mv F G && + test_tick && + git -C keep commit -m "B1: rename F to G" && + printf "line 1\nline 2\nline 3\nline 4 B\nline 5\n" >keep/G && + test_tick && + git -C keep commit -am "B2: update G" && + + git -C keep checkout master && + test_tick && + git -C keep merge -s ours -m "M: merge B, keep F, drop G" B +' + +test_expect_success '--follow follows a rename on the side the merge did not take' ' + git -C keep log --follow --pretty=tformat:%s F >actual && + cat >expect <<-\EOF && + B2: update G + B1: rename F to G + A1: update F in place + O: add F + EOF + test_cmp expect actual +' + +test_expect_success '--follow with --diff-merges=first-parent follows all parents' ' + git -C keep log --follow --diff-merges=first-parent --no-patch \ + --pretty=tformat:%s F >actual && + cat >expect <<-\EOF && + B2: update G + B1: rename F to G + A1: update F in place + O: add F + EOF + test_cmp expect actual +' + test_done
Signature · 1 line
2.56.0
PATCH v4 2/6 log: extract showing the remerge/combined diff of a merge commit
In reply to Miklos VajnaNo functional changes intended. A later commit in this series will also call log_tree_merge_diff() after the parent loop of log_tree_diff(), so extract that code to a function to avoid duplicating it.
--- log-tree.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-)
log-tree.c +22 -14
@@ -1180,6 +1180,26 @@ static void propagate_follow_pathspec_to_parent(struct rev_info *opt,diff_free (&diff_opts); } +/* Show the remerge or combined diff of a merge commit */ +static int log_tree_merge_diff (struct rev_info *opt,struct commit *commit, +struct commit_list *parents, +struct object_id *oid) +{ +int octopus = (parents->next->next !=NULL ); + +if (opt->remerge_diff) { +if (octopus) { +show_log (opt); +fprintf (opt->diffopt.file, +"diff: warning: Skipping remerge-diff " +"for octopus merges.\n" ); +return 1 ; + } +return do_remerge_diff (opt, parents, oid); + } +return do_diff_combined (opt, commit); +} +/* * Show the diff of a commit. *
@@ -1220,20 +1240,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log }if (is_merge) { -int octopus = (parents->next->next !=NULL ); - -if (opt->remerge_diff) { -if (octopus) { -show_log (opt); -fprintf (opt->diffopt.file, -"diff: warning: Skipping remerge-diff " -"for octopus merges.\n" ); -return 1 ; - } -return do_remerge_diff (opt, parents, oid); - } -if (opt->combine_merges) -return do_diff_combined (opt, commit); +if (opt->remerge_diff || opt->combine_merges) +return log_tree_merge_diff (opt, commit, parents, oid);if (opt->separate_merges) {if (!opt->first_parent_merges) {/* Show parent info for multiple diffs */
Signature · 1 line
2.56.0
PATCH v4 3/6 log: fix -m --follow losing the history of later parents
In reply to Miklos VajnaWith 'git log -m --follow', a merge is diffed against each of its parents, with rename following. Take the diamond case of t4219: the two sides of a fork rename path0 to path1 and to path2, and the merge renames the file to path. Here 'git log -m --follow path' doesn't show "A: add path0" and "C: modify path0 on lower".
The problem is that the parent loop in log_tree_diff() doesn't restore the followed path between parents. When the diff against the first parent finds a rename, the followed path is updated, and this has two effects. First, the second parent is diffed with the already renamed path, not with the path of the merge. Second, the path that propagate_follow_pathspec_to_parent() records for each parent also starts from that updated path, so history is lost on the side of the second parent. Without -m, merges are not diffed, so the followed path is not updated there.
Fix the problem by saving the path of the merge before the parent loop, and restoring it before diffing each parent and after the loop. Do this only for merges: for a commit with a single parent, the renamed path is what --follow needs to continue with. Add a test for this, which shows the full history of the diamond case with -m.
--- log-tree.c | 13 +++++++++++++ t/t4219-log-follow-merge.sh | 16 ++++++++++++++++ 2 files changed, 29 insertions(+)
log-tree.c +13 -0
@@ -1212,6 +1212,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct logstruct object_id *oid;int is_merge;int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status; +char *path =NULL ;if (!all_need_diff && !opt->merges_need_diff)return 0 ;
@@ -1251,10 +1252,18 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct logreturn 0 ; } +if (is_merge && opt->diffopt.flags.follow_renames) + path =xstrdup_or_null ( +pathspec_single_path (&opt->diffopt.pathspec)); + showed_log =0 ;for (;;) {struct commit *parent = parents->item; +/* Diff each parent with the path of the merge */ +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path);parse_commit_or_die (parent);diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &opt->diffopt);
@@ -1269,6 +1278,10 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log log->parent = parents->item; opt->loginfo = log; } +/* Propagate to parents starting from the path of the merge, too */ +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, path); +free (path);return showed_log; }
t/t4219-log-follow-merge.sh +16 -0
@@ -144,6 +144,22 @@ do ' done +test_expect_success '--follow follows renames through a fork with -m' ' + git -C diamond log --follow -m --pretty=tformat:%s path >log && + grep "^[A-Z]: " log | sort >actual && + cat >expect <<-\EOF && + A: add path0 + B: modify path0 on upper + C: modify path0 on lower + M: merge with rename to path + M: merge with rename to path + X: rename path0 to path1 + Y: rename path0 to path2 + Z: modify path + EOF + test_cmp expect actual +' + # Build a merge that takes the renamed path from the second parent, while the # first parent only has the old path. $2 decides which side is committed first, # which decides the walk order.
Signature · 1 line
2.56.0
PATCH v4 4/6 log: simplify --follow following renames for non-linear history
In reply to Miklos VajnaCode added in 304812ed33 (log: improve --follow following renames for non-linear history, 2026-06-22) implemented the feature by introducing a new propagate_follow_pathspec_to_parent() function which adds a new call to diff_tree_oid(). Also it introduces a new loop to record what path each parent of a merge commit should use when following a path.
This was working, but it caused a performance regression: e.g. 'git log --stat --follow -- include/linux/rsc_table.h' in linux.git went from 28s to 6m43s. The new code diffs each merge against each of its parents with rename detection, which is expensive when a parent doesn't have the followed path yet. Fixing this would require adding even more custom code, instead of just extending the existing code in log_tree_diff(). So first simplify the code here, and fix the performance regression in a follow-up commit.
Remove propagate_follow_pathspec_to_parent() and the loop calling it for each parent. Instead, record the path to follow for each parent in the parent loop of log_tree_diff(), right after diff_tree_oid() updated the pathspec in case of a rename. Merges are not diffed by default, so with --follow, run the same loop for them as well, but without showing anything. With -c, --cc or --remerge-diff, show the merge diff after the loop. With --diff-merges=first-parent, keep diffing the other parents silently, unless the walk only follows first parents. This gets rid of the additional call to diff_tree_oid(). The rework has no positive performance impact yet, it just simplifies the code.
The previous commit restores the path of the merge before diffing each parent, and also after the loop, for propagate_follow_pathspec_to_parent(). Keep restoring it between parents, but not after the loop, since there is no later propagation anymore.
--- log-tree.c | 96 +++++++++++++++++++++++------------------------------- 1 file changed, 41 insertions(+), 55 deletions(-)
log-tree.c +41 -55
@@ -1153,33 +1153,6 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } -/* Compute a path to follow in parent, if there is one */ -static void propagate_follow_pathspec_to_parent (struct rev_info *opt, -struct commit *commit, -struct commit *parent) -{ -struct diff_options diff_opts; -const char *path; - -parse_commit_or_die (parent); -repo_diff_setup (opt->diffopt.repo, &diff_opts); -copy_pathspec (&diff_opts.pathspec, &opt->diffopt.pathspec); - diff_opts.flags.recursive =1 ; - diff_opts.flags.follow_renames =1 ; - diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; -diff_setup_done (&diff_opts); -diff_tree_oid (get_commit_tree_oid (parent), -get_commit_tree_oid (commit), -"" , &diff_opts); - - path =pathspec_single_path (&diff_opts.pathspec); -if (path) -remember_follow_pathspec (opt, parent, path); - -diff_queue_clear (&diff_queued_diff); -diff_free (&diff_opts); -} -/* Show the remerge or combined diff of a merge commit */ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit, struct commit_list *parents,
@@ -1208,10 +1181,11 @@ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,static int log_tree_diff (struct rev_info *opt,struct commit *commit,struct log_info *log) {int showed_log; -struct commit_list *parents; +struct commit_list *parents, *all_parents;struct object_id *oid;int is_merge;int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status; +int follow_only =0 ;char *path =NULL ;if (!all_need_diff && !opt->merges_need_diff)
@@ -1241,46 +1215,73 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log }if (is_merge) { -if (opt->remerge_diff || opt->combine_merges) +int merge_diff = opt->remerge_diff || opt->combine_merges; + +if (merge_diff && !opt->diffopt.flags.follow_renames)return log_tree_merge_diff (opt, commit, parents, oid); -if (opt->separate_merges) { +if (opt->separate_merges && !merge_diff) {if (!opt->first_parent_merges) {/* Show parent info for multiple diffs */ log->parent = parents->item; } + }else if (opt->diffopt.flags.follow_renames) { +/* Diff only to find the path to follow in parents */ + follow_only =1 ; }else return 0 ; } -if (is_merge && opt->diffopt.flags.follow_renames) +if (opt->diffopt.flags.follow_renames) path =xstrdup_or_null (pathspec_single_path (&opt->diffopt.pathspec)); showed_log =0 ; + all_parents = parents;for (;;) {struct commit *parent = parents->item; -/* Diff each parent with the path of the merge */ -if (path) -set_pathspec_to_single_path (&opt->diffopt.pathspec, - path);parse_commit_or_die (parent);diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &opt->diffopt); -log_tree_diff_flush (opt); +if (follow_only) { +diff_queue_clear (&diff_queued_diff); + opt->diffopt.found_follow =0 ; + }else { +log_tree_diff_flush (opt); + } showed_log |= !opt->loginfo; +/* The diff updated the pathspec in case of a rename */ +if (path) +remember_follow_pathspec (opt, parent, +pathspec_single_path (&opt->diffopt.pathspec)); +/* Set up the log info for the next parent, if any.. */ parents = parents->next; -if (!parents || opt->first_parent_merges) +if (!parents)break ; +if (opt->first_parent_merges) { +if (!path || opt->first_parent_only) +break ; +/* Only find the path to follow in the other parents */ + follow_only =1 ; + } +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path); +if (follow_only) +continue ; log->parent = parents->item; opt->loginfo = log; } -/* Propagate to parents starting from the path of the merge, too */ -if (path) -set_pathspec_to_single_path (&opt->diffopt.pathspec, path); +if (follow_only && (opt->remerge_diff || opt->combine_merges)) { +/* Show the merge diff with the path of this commit */ +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path); + showed_log =log_tree_merge_diff (opt, commit, all_parents, oid); + }free (path);return showed_log; }
@@ -1319,21 +1320,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)fprintf (opt->diffopt.file,"\n%s\n" , opt->break_bar);if (shown)show_diff_of_diff (opt); - -/* Record what path each parent of this commit should use */ -if (opt->diffopt.flags.follow_renames) { -struct commit_list *parents =get_saved_parents (opt, commit); -if (parents && parents->next) { -struct commit_list *p; -for (p = parents; p; p = p->next) -propagate_follow_pathspec_to_parent (opt, commit, - p->item); - }else if (parents) { -remember_follow_pathspec (opt, parents->item, -pathspec_single_path (&opt->diffopt.pathspec)); - } - } - opt->loginfo =NULL ;maybe_flush_or_die (opt->diffopt.file,"stdout" ); opt->diffopt.no_free = no_free;
Signature · 1 line
2.56.0
PATCH v4 5/6 log: speed up --follow following renames for non-linear history
In reply to Miklos VajnaTest the performance of 'git log --stat --follow -- include/linux/rsc_table.h' on linux.git, it used to take about 28 seconds, and since commit 304812ed33 (log: improve --follow following renames for non-linear history, 2026-06-22), it takes 6m43s.
What happens is that log_tree_diff() diffs each merge against all of its parents with rename following. diff_tree_oid() only does the expensive rename detection when the path is added relative to a parent, but that is common for merges, e.g. when a branch forked before the path was created merges mainline, and this slows down 'git log --follow'.
Fix the problem by diffing the merge against its parents in two passes:
- First pass: diff against the parents without rename following. If the path is unchanged against a parent, then just follow that history in all parents.
- Except when an other parent doesn't have the path, the merge has it and a merge base of the two parents also has it, i.e. the path was renamed or removed on that side: still do the rename following against such a parent, otherwise the history on that side would be lost. The merge base is only computed in this rare case, and back-merges where the path was created after the other side forked still skip the expensive rename detection.
- Second pass: only do the rename following against each parent if the path changed relative to all parents.
Given that this way a parent may be asked to follow a path that it doesn't have, also adjust remember_follow_pathspec(): if children disagree on what should be the path for a commit, then prefer the path that exists in that commit.
Now the above use case takes 36s, somewhat close to the original cost.
--- log-tree.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 91 insertions(+), 5 deletions(-)
log-tree.c +91 -5
@@ -31,6 +31,7 @@#include "range-diff.h" #include "strmap.h" #include "tree.h" +#include "tree-walk.h" #include "wildmatch.h" #include "write-or-die.h" #include "pager.h"
@@ -1110,6 +1111,17 @@ static void set_pathspec_to_single_path(struct pathspec *ps, const char *path) PATHSPEC_LITERAL_PATH, "", paths); } +static int commit_has_path (struct rev_info *opt,struct commit *c, +const char *path) +{ +struct object_id oid; +unsigned short mode; + +parse_commit_or_die (c); +return !get_tree_entry (opt->diffopt.repo,get_commit_tree_oid (c), path, + &oid, &mode); +} +static void remember_follow_pathspec (struct rev_info *opt,struct commit *c,const char *path) {
@@ -1122,8 +1134,12 @@ static void remember_follow_pathspec(struct rev_info *opt,init_follow_pathspec_slab (opt->follow_pathspec_slab); } slot =follow_pathspec_slab_at (opt->follow_pathspec_slab, c); -if (*slot && !strcmp (*slot, path)) -return ; +/* If children of 'c' disagree on the path, prefer the one 'c' has. */ +if (*slot) { +if (!strcmp (*slot, path) ||commit_has_path (opt, c, *slot) || + !commit_has_path (opt, c, path)) +return ; + }free (*slot); *slot =xstrdup (path); }
@@ -1153,6 +1169,29 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } +/* +* When the path in a merge is the same as in 'same', does the path still need +* renames to be followed in 'parent'? Only if the merge has the path, 'parent' +* doesn't have it, but its fork point with 'same' had it, e.g. 'parent' +* renamed the path. +*/ +static int follow_needs_renames (struct rev_info *opt,struct commit *parent, +struct commit *same,const char *path) +{ +struct commit_list *bases =NULL , *base; +int ret =0 ; + +if (commit_has_path (opt, parent, path) || + !commit_has_path (opt, same, path)) +return 0 ; +if (repo_get_merge_bases (opt->diffopt.repo, parent, same, &bases) <0 ) +exit (128 ); +for (base = bases; base && !ret; base = base->next) + ret =commit_has_path (opt, base->item, path); +commit_list_free (bases); +return ret; +} +/* Show the remerge or combined diff of a merge commit */ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit, struct commit_list *parents,
@@ -1181,7 +1220,7 @@ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,static int log_tree_diff (struct rev_info *opt,struct commit *commit,struct log_info *log) {int showed_log; -struct commit_list *parents, *all_parents; +struct commit_list *parents, *all_parents, *rename_parents =NULL ;struct object_id *oid;int is_merge;int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;
@@ -1235,15 +1274,25 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log path =xstrdup_or_null (pathspec_single_path (&opt->diffopt.pathspec)); +if (follow_only) { +/* +* First look for a parent where the path is unchanged, without +* the expensive rename detection. +*/ + opt->diffopt.flags.follow_renames =0 ; + } + showed_log =0 ; all_parents = parents;for (;;) {struct commit *parent = parents->item; +int changed =1 ;parse_commit_or_die (parent);diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &opt->diffopt);if (follow_only) { + changed = diff_queued_diff.nr;diff_queue_clear (&diff_queued_diff); opt->diffopt.found_follow =0 ; }else {
@@ -1252,20 +1301,54 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log showed_log |= !opt->loginfo; +if (!changed) { +/* +* Same as in this parent, follow that history, but +* still look for renames in parents that had the path +* at the fork point. +*/ +struct commit_list **tail = &rename_parents; + +for (parents = all_parents; parents; + parents = parents->next) { +if (parents->item != parent && +follow_needs_renames (opt, parents->item, + parent, path)) + tail = &commit_list_insert (parents->item, + tail)->next; +else +remember_follow_pathspec (opt, + parents->item, + path); + } +if (!rename_parents) +break ; + opt->diffopt.flags.follow_renames =1 ; + parents = rename_parents; +continue ; + } +/* The diff updated the pathspec in case of a rename */ -if (path) +if (path && opt->diffopt.flags.follow_renames)remember_follow_pathspec (opt, parent,pathspec_single_path (&opt->diffopt.pathspec));/* Set up the log info for the next parent, if any.. */ parents = parents->next; +if (!parents && follow_only && + !opt->diffopt.flags.follow_renames) { +/* The path changed in all parents, look for renames. */ + opt->diffopt.flags.follow_renames =1 ; + parents = all_parents; + }if (!parents)break ; -if (opt->first_parent_merges) { +if (opt->first_parent_merges && !follow_only) {if (!path || opt->first_parent_only)break ;/* Only find the path to follow in the other parents */ follow_only =1 ; + opt->diffopt.flags.follow_renames =0 ; }if (path) set_pathspec_to_single_path(&opt->diffopt.pathspec,
@@ -1275,6 +1358,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log log->parent = parents->item; opt->loginfo = log; } +if (follow_only) + opt->diffopt.flags.follow_renames =1 ;if (follow_only && (opt->remerge_diff || opt->combine_merges)) {/* Show the merge diff with the path of this commit */ if (path)
@@ -1282,6 +1367,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log path); showed_log =log_tree_merge_diff (opt, commit, all_parents, oid); } +commit_list_free (rename_parents);free (path);return showed_log; }
Signature · 1 line
2.56.0
Re: [PATCH v4 5/6] log: speed up --follow following renames for non-linear history
Quoted 20 lines · Miklos Vajna
log-tree.c +12
-0@@ -1153,6 +1169,29 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } +/* +* When the path in a merge is the same as in 'same', does the path still need +* renames to be followed in 'parent'? Only if the merge has the path, 'parent' +* doesn't have it, but its fork point with 'same' had it, e.g. 'parent' +* renamed the path. +*/ +static int follow_needs_renames (struct rev_info *opt,struct commit *parent, +struct commit *same,const char *path) +{ +struct commit_list *bases =NULL , *base; +int ret =0 ; +
log-tree.c void release_follow_pathspec_slab(struct rev_info *opt) · line 1193
Quoted 10 lines
log-tree.c +10
-0+if (commit_has_path (opt, parent, path) || + !commit_has_path (opt, same, path)) +return 0 ; +if (repo_get_merge_bases (opt->diffopt.repo, parent, same, &bases) <0 ) +exit (128 ); +for (base = bases; base && !ret; base = base->next) + ret =commit_has_path (opt, base->item, path); +commit_list_free (bases); +return ret; +}
With unrelated histories this would return no bases at all, and hence we'd always return 0 in such a case. But 304812ed33d (log: improve --follow following renames for non-linear history, 2026-06-22) explicitly mentioned the use case of sub-tree merges of unrelated histories, so isn't this a regression?
Patrick
Re: [PATCH v4 5/6] log: speed up --follow following renames for non-linear history
Hi Patrick,
Quoted 5 lines · Patrick Steinhardt
With unrelated histories this would return no bases at all, and hence we'd always return 0 in such a case. But
304812ed33d(log: improve --follow following renames for non-linear history, 2026-06-22) explicitly mentioned the use case of sub-tree merges of unrelated histories, so isn't this a regression?
You're right. The existing '--follow finds the pre-merge commit through a subtree merge' test didn't catch this, because there no parent has the path unchanged, so follow_needs_renames() is not called.
When there is no merge base, follow_needs_renames() now returns 1, so renames are followed as before.
I've added a new '--follow finds the history of a copied file through a subtree merge' test to 1/5 in v5 for such a case, and 5/5 now passes it.
Thanks,
Miklos
PATCH v4 6/6 log: fix --follow following an edit on a side the merge did not take
In reply to Miklos VajnaTake a history where F is renamed to G on master, F is edited on a side branch, and the side branch is merged with '-s ours', so the merge keeps G and drops F. Here 'git log --follow G' doesn't show the edit of F on the side branch, while it did before the previous commit.
The problem is that follow_needs_renames() only searches for renames in a parent without the path if the merge base still has the path, which covers the case when the parent renamed the path. Here the other side of the merge did the rename, so the merge base only has the old name. The parent is then told to follow G, which it doesn't have, and the history of F on its side is lost. Searching for renames in each parent that lacks the path would fix this, but that is the expensive search that the previous commit avoids, e.g. 148 merges hit this case for include/linux/rsc_table.h in linux.git.
Fix the problem by also searching for renames when the merge base doesn't have the path, but the parent changed a file since the merge base that the merge no longer has: only such a file can be the old name of the path that still needs to be followed in the parent. This check is cheap, since it only diffs the changes of the side branch, and it rules out 146 of the 148 merges in linux.git. Then do a plain rename detection from the parent to the merge, which only looks at the files deleted by the merge, not at all files of the parent like --follow does.
--- log-tree.c | 96 +++++++++++++++++++++++++++++++++---- t/t4219-log-follow-merge.sh | 31 ++++++++++++ 2 files changed, 117 insertions(+), 10 deletions(-)
log-tree.c +86 -10
@@ -1111,15 +1111,20 @@ static void set_pathspec_to_single_path(struct pathspec *ps, const char *path) PATHSPEC_LITERAL_PATH, "", paths); } -static int commit_has_path (struct rev_info *opt,struct commit *c, -const char *path) +static int tree_has_path (struct rev_info *opt,struct object_id *tree, +const char *path) {struct object_id oid;unsigned short mode; +return !get_tree_entry (opt->diffopt.repo, tree, path, &oid, &mode); +} + +static int commit_has_path (struct rev_info *opt,struct commit *c, +const char *path) +{parse_commit_or_die (c); -return !get_tree_entry (opt->diffopt.repo,get_commit_tree_oid (c), path, - &oid, &mode); +return tree_has_path (opt,get_commit_tree_oid (c), path); } static void remember_follow_pathspec(struct rev_info *opt,
@@ -1169,26 +1174,97 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } +/* +* Did 'parent' change a file since 'base' that the merge (with tree 'oid') no +* longer has? Only such a file can be the source of a rename on the other side +* of the merge that still needs to be followed in 'parent'. +*/ +static int follow_parent_changed_dropped_file (struct rev_info *opt, +struct commit *base, +struct commit *parent, +struct object_id *oid) +{ +struct diff_options diff_opts; +int i, ret =0 ; + +repo_diff_setup (opt->diffopt.repo, &diff_opts); + diff_opts.flags.recursive =1 ; + diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; +diff_setup_done (&diff_opts); +diff_tree_oid (get_commit_tree_oid (base),get_commit_tree_oid (parent), +"" , &diff_opts); +for (i =0 ; i < diff_queued_diff.nr && !ret; i++) { +struct diff_filepair *p = diff_queued_diff.queue[i]; + + ret =DIFF_FILE_VALID (p->two) && + !tree_has_path (opt, oid, p->two->path); + } +diff_queue_clear (&diff_queued_diff); +diff_free (&diff_opts); +return ret; +} + +/* +* Is 'path' in the merge (with tree 'oid') renamed from a file of 'parent'? +* Only look at the files that the merge deleted, not at all files of 'parent' +* like --follow does. +*/ +static int follow_has_rename_source (struct rev_info *opt, +struct commit *parent, +struct object_id *oid,const char *path) +{ +struct diff_options diff_opts; +int i, ret =0 ; + +repo_diff_setup (opt->diffopt.repo, &diff_opts); + diff_opts.flags.recursive =1 ; + diff_opts.detect_rename =DIFF_DETECT_RENAME ; + diff_opts.rename_score = opt->diffopt.rename_score; + diff_opts.single_follow = path; + diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; +diff_setup_done (&diff_opts); +diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &diff_opts); +diffcore_std (&diff_opts); +for (i =0 ; i < diff_queued_diff.nr && !ret; i++) { +struct diff_filepair *p = diff_queued_diff.queue[i]; + + ret = p->status ==DIFF_STATUS_RENAMED && + !strcmp (p->two->path, path); + } +diff_queue_clear (&diff_queued_diff); +diff_free (&diff_opts); +return ret; +} +/* * When the path in a merge is the same as in 'same', does the path still need * renames to be followed in 'parent'? Only if the merge has the path, 'parent' -* doesn't have it, but its fork point with 'same' had it, e.g. 'parent' -* renamed the path. +* doesn't have it, and either: +* +* - its fork point with 'same' had it, e.g. 'parent' renamed the path, or +* - 'same' renamed the path from a file that 'parent' changed. */ static int follow_needs_renames (struct rev_info *opt,struct commit *parent, -struct commit *same,const char *path) +struct commit *same,struct object_id *oid, +const char *path) {struct commit_list *bases =NULL , *base; -int ret =0 ; +int ret =0 , changed_dropped =0 ;if (commit_has_path (opt, parent, path) || !commit_has_path (opt, same, path))return 0 ;if (repo_get_merge_bases (opt->diffopt.repo, parent, same, &bases) <0 )exit (128 ); -for (base = bases; base && !ret; base = base->next) +for (base = bases; base && !ret; base = base->next) { ret =commit_has_path (opt, base->item, path); +if (!ret && !changed_dropped) + changed_dropped =follow_parent_changed_dropped_file ( + opt, base->item, parent, oid); + }commit_list_free (bases); +if (!ret && changed_dropped) + ret =follow_has_rename_source (opt, parent, oid, path);return ret; }
@@ -1313,7 +1389,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log parents = parents->next) {if (parents->item != parent &&follow_needs_renames (opt, parents->item, - parent, path)) + parent, oid, path)) tail = &commit_list_insert (parents->item, tail)->next;else
t/t4219-log-follow-merge.sh +31 -0
@@ -266,4 +266,35 @@ test_expect_success '--follow with --diff-merges=first-parent follows all parent test_cmp expect actual ' +test_expect_success 'setup merge keeping the renamed path, other side edited the old one' ' + git init keep-renamed && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-renamed/F && + git -C keep-renamed add F && + test_tick && + git -C keep-renamed commit -m "O: add F" && + git -C keep-renamed branch B && + git -C keep-renamed mv F G && + test_tick && + git -C keep-renamed commit -m "A1: rename F to G on master" && + + git -C keep-renamed checkout B && + printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-renamed/F && + test_tick && + git -C keep-renamed commit -am "B2: update F on B" && + + git -C keep-renamed checkout master && + test_tick && + git -C keep-renamed merge -s ours -m "M: merge B, keep G, drop F" B +' + +test_expect_success '--follow follows an edit on the side the merge did not take' ' + git -C keep-renamed log --follow --pretty=tformat:%s G >actual && + cat >expect <<-\EOF && + B2: update F on B + A1: rename F to G on master + O: add F + EOF + test_cmp expect actual +' + test_done
Signature · 1 line
2.56.0
Re: [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take
Quoted 15 lines · Miklos Vajna
Take a history where F is renamed to G on master, F is edited on a side branch, and the side branch is merged with '-s ours', so the merge keeps G and drops F. Here 'git log --follow G' doesn't show the edit of F on the side branch, while it did before the previous commit.
The problem is that
follow_needs_renames()only searches for renames in a parent without the path if the merge base still has the path, which covers the case when the parent renamed the path. Here the other side of the merge did the rename, so the merge base only has the old name. The parent is then told to follow G, which it doesn't have, and the history of F on its side is lost. Searching for renames in each parent that lacks the path would fix this, but that is the expensive search that the previous commit avoids, e.g. 148 merges hit this case forinclude/linux/rsc_table.hin linux.git.
Quoted 4 lines
Fix the problem by also searching for renames when the merge base doesn't have the path, but the parent changed a file since the merge base that the merge no longer has: only such a file can be the old name of the path that still needs to be followed in the parent.
This isn't true though in the case where you copy a file though, is it? E.g. the following test case is exactly the same as the one you add in this commit, but instead of using a rename we use a copy:
t/t4219-log-follow-merge.sh +32 -0
@@ -297,4 +297,36 @@ test_expect_success '--follow follows an edit on the side the merge did not take test_cmp expect actual ' +test_expect_success 'setup merge keeping the copied path, other side edited the original' ' + git init keep-copied && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-copied/F && + git -C keep-copied add F && + test_tick && + git -C keep-copied commit -m "O: add F" && + git -C keep-copied branch B && + cp keep-copied/F keep-copied/G && + git -C keep-copied add G && + test_tick && + git -C keep-copied commit -m "A1: copy F to G on master" && + + git -C keep-copied checkout B && + printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-copied/F && + test_tick && + git -C keep-copied commit -am "B2: update F on B" && + + git -C keep-copied checkout master && + test_tick && + git -C keep-copied merge -s ours -m "M: merge B, keep F and G" B +' + +test_expect_success '--follow follows an edit of the copy source on the side the merge did not take' ' + git -C keep-copied log --follow --pretty=tformat:%s G >actual && + cat >expect <<-\EOF && + B2: update F on B + A1: copy F to G on master + O: add F + EOF + test_cmp expect actual +' + test_done
This test passes on master, but fails with your patch series.
Patrick
Re: [PATCH v4 6/6] log: fix --follow following an edit on a side the merge did not take
Hi Patrick,
Quoted 8 lines · Patrick Steinhardt
Quoted 4 lines
Fix the problem by also searching for renames when the merge base doesn't have the path, but the parent changed a file since the merge base that the merge no longer has: only such a file can be the old name of the path that still needs to be followed in the parent.
This isn't true though in the case where you copy a file though, is it? E.g. the following test case is exactly the same as the one you add in this commit, but instead of using a rename we use a copy:
You're right, I didn't consider copying. I've added your test to 1/5 and fixed 5/5 in v5 to handle this correctly.
The pre-filter now also takes a file into account that the merge still has, but with the version of the other side, and the rename check now also detects copies. This makes the linux.git case take about 42s, instead of 37s here.
Thanks,
Miklos
Re: [PATCH v4 0/6] log: speed up --follow following renames for non-linear history
In reply to Miklos VajnaHi Miklos,
Quoted 3 lines · Miklos Vajna
Hi Patrick,
Thanks for the reviews of v3. Changes in v4:
please note that the expectation is that you answer to the review comments directly instead of only answering reviews via the cover letter of the next version. We want there to be discussions, and reviewers should not have to figure out by themselves whether you did or did not address specific concerns.
Quoted 6 lines
- Patch 6/6 fixes the case you found, where the speedup lost history:
one side of the merge renamed the path, and the other side, whose version the merge dropped, edited the old name. It uses your test. I kept it separate from the speedup patch, which is complicated enough already. With the extra check, the linux.git case still takes about 37s.
Hmm, okay. But isn't it the case that your speedup patch introduces a regression where we now don't follow history in some specific cases anymore, and that regression was only introduced in the preceding commit? So it's a bit curious to have this be an extra commit.
Quoted 6 lines
- I also noticed that the simplify patch lost history with --diff-merges=first-parent, which diffs merges only against their first parent, but still walks all parents. Fixed that and added a test for it to 1/6.
Interdiff against v3:
Instead of an interdiff, could you please send a range-diff next time? That makes it was easier to see which specific commits I need to have a look at again, and it also shows diffs to commit messages.
Patrick
Re: [PATCH v4 0/6] log: speed up --follow following renames for non-linear history
Hi Patrick,
Quoted 5 lines · Patrick Steinhardt
please note that the expectation is that you answer to the review comments directly instead of only answering reviews via the cover letter of the next version. We want there to be discussions, and reviewers should not have to figure out by themselves whether you did or did not address specific concerns.
I see, I'll stick to that way from now on.
Quoted 4 lines
Hmm, okay. But isn't it the case that your speedup patch introduces a regression where we now don't follow history in some specific cases anymore, and that regression was only introduced in the preceding commit? So it's a bit curious to have this be an extra commit.
I've squashed patch 6 into patch 5 in v5. This way 5/5 in v5 is a bit larger, but no patch in the series fixes a regression from the same series.
Quoted 5 lines
Quoted 1 line
Interdiff against v3:
Instead of an interdiff, could you please send a range-diff next time? That makes it was easier to see which specific commits I need to have a look at again, and it also shows diffs to commit messages.
I did it that way in v5.
Thanks,
Miklos
v5 cover log: speed up --follow following renames for non-linear history
In reply to Miklos VajnaHi Patrick,
Thanks for the reviews of v4. Changes in v5:
- The fix of the old 6/6 is now squashed into the speedup patch (5/5), so no commit in the series loses history, and its test is moved to 1/5. While at it, I simplified 5/5: instead of two passes of diffs against the parents, it first finds a parent where the path is unchanged, and then only diffs against the other parents if they still need renames to be followed. Because of the squash, the range-diff pairs the new 5/5 with the old 6/6.
- 5/5 now also follows renames when a parent has no merge base with the parent where the path is unchanged, e.g. a subtree merge of unrelated histories. Added a test for this to 1/5.
- 5/5 now also follows copies, not only renames, when the merge dropped the edit of the copy source. Added your test for this to 1/5. With this, the linux.git case takes about 42s, instead of 37s.
Thanks,
Miklos
Miklos Vajna (5):
t4219: test more --follow cases across merges
log: extract showing the remerge/combined diff of a merge commit
log: fix -m --follow losing the history of later parents
log: simplify --follow following renames for non-linear history
log: speed up --follow following renames for non-linear history
log-tree.c | 258 +++++++++++++++++++++++++++++------- t/t4219-log-follow-merge.sh | 230 ++++++++++++++++++++++++++++++++ 2 files changed, 438 insertions(+), 50 deletions(-)
Range-diff against v4:
1: aabf5652c9 ! 1: 5512aa682f t4219: test more --follow cases across merges
@@ Commit message
t4219: test more --follow cases across merges
A later commit in this series will speed up 'git log --follow' for
- merge commits. Add tests for cases which already work, but had no
+ merge commits. Add tests for cases which already work, but have no
test coverage, so it's easy to break them while optimizing:
+ - a subtree merge adding the history of a file that was copied before
- --follow through a fork with renames on both sides, with -c, --cc
and --remerge-diff
- a --no-ff merge taking a rename from the second parent, in both commit
@@ Commit message
- a merge keeping the path, while the other side renamed it
- --diff-merges=first-parent, which diffs merges only against their
first parent, but still walks all parents
+ - a merge keeping the renamed path, while the other side edited the old
+ one
+ - a merge keeping the copied path, while the other side edited the
+ original
+ Helped-by: Patrick Steinhardt <[email protected]>
Signed-off-by: Miklos Vajna <[email protected]>
## t/t4219-log-follow-merge.sh ##
+@@ t/t4219-log-follow-merge.sh: test_expect_success '--follow finds the pre-merge commit through a subtree merge
+ test_cmp expect actual
+ '
+
++test_expect_success 'setup subtree merge adding the history of a copied file' '
++ git init graft &&
++ echo outer >graft/outer.txt &&
++ git -C graft add outer.txt &&
++ test_tick &&
++ git -C graft commit -m "outer init" &&
++ mkdir graft/inner &&
++ cp inner/inner.txt graft/inner/ &&
++ git -C graft add inner &&
++ test_tick &&
++ git -C graft commit -m "copy inner.txt to inner/" &&
++
++ git -C graft fetch ../inner master &&
++ test_tick &&
++ git -C graft merge -s ours --allow-unrelated-histories \
++ -m "Merge the history of inner/" FETCH_HEAD
++'
++
++test_expect_success '--follow finds the history of a copied file through a subtree merge' '
++ git -C graft log --follow --pretty=tformat:%s inner/inner.txt >actual &&
++ cat >expect <<-\EOF &&
++ copy inner.txt to inner/
++ inner init
++ EOF
++ test_cmp expect actual
++'
++
+ test_expect_success 'setup merge of two branches that both renamed a file to README' '
+ git init foo &&
+ mkdir foo/foo &&
@@ t/t4219-log-follow-merge.sh: test_expect_success '--follow follows renames through a fork in a single history
test_cmp expect actual.sorted
'
@@ t/t4219-log-follow-merge.sh: test_expect_success '--follow follows renames throu
+ EOF
+ test_cmp expect actual
+'
++
++test_expect_success 'setup merge keeping the renamed path, other side edited the old one' '
++ git init keep-renamed &&
++ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-renamed/F &&
++ git -C keep-renamed add F &&
++ test_tick &&
++ git -C keep-renamed commit -m "O: add F" &&
++ git -C keep-renamed branch B &&
++ git -C keep-renamed mv F G &&
++ test_tick &&
++ git -C keep-renamed commit -m "A1: rename F to G on master" &&
++
++ git -C keep-renamed checkout B &&
++ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-renamed/F &&
++ test_tick &&
++ git -C keep-renamed commit -am "B2: update F on B" &&
++
++ git -C keep-renamed checkout master &&
++ test_tick &&
++ git -C keep-renamed merge -s ours -m "M: merge B, keep G, drop F" B
++'
++
++test_expect_success '--follow follows an edit on the side the merge did not take' '
++ git -C keep-renamed log --follow --pretty=tformat:%s G >actual &&
++ cat >expect <<-\EOF &&
++ B2: update F on B
++ A1: rename F to G on master
++ O: add F
++ EOF
++ test_cmp expect actual
++'
++
++test_expect_success 'setup merge keeping the copied path, other side edited the original' '
++ git init keep-copied &&
++ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-copied/F &&
++ git -C keep-copied add F &&
++ test_tick &&
++ git -C keep-copied commit -m "O: add F" &&
++ git -C keep-copied branch B &&
++ cp keep-copied/F keep-copied/G &&
++ git -C keep-copied add G &&
++ test_tick &&
++ git -C keep-copied commit -m "A1: copy F to G on master" &&
++
++ git -C keep-copied checkout B &&
++ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-copied/F &&
++ test_tick &&
++ git -C keep-copied commit -am "B2: update F on B" &&
++
++ git -C keep-copied checkout master &&
++ test_tick &&
++ git -C keep-copied merge -s ours -m "M: merge B, keep F and G" B
++'
++
++test_expect_success '--follow follows an edit of the copy source on the side the merge did not take' '
++ git -C keep-copied log --follow --pretty=tformat:%s G >actual &&
++ cat >expect <<-\EOF &&
++ B2: update F on B
++ A1: copy F to G on master
++ O: add F
++ EOF
++ test_cmp expect actual
++'
+
test_done
2: b172890bf4 = 2: f36129714b log: extract showing the remerge/combined diff of a merge commit
3: 6586e8f004 = 3: 85b6067fbd log: fix -m --follow losing the history of later parents
4: b903152bc4 ! 4: 5ba93e49cf log: simplify --follow following renames for non-linear history @@ Commit message
log: simplify --follow following renames for non-linear history
Code added in 304812ed33 (log: improve --follow following renames for
- non-linear history, 2026-06-22) implemented the feature by introducing a
+ non-linear history, 2026-06-22) implements the feature by introducing a
new propagate_follow_pathspec_to_parent() function which adds a new call to diff_tree_oid(). Also it introduces a new loop to record what path each parent of a merge commit should use when following a path.
- This was working, but it caused a performance regression: e.g. 'git log
- --stat --follow -- include/linux/rsc_table.h' in linux.git went from 28s
- to 6m43s. The new code diffs each merge against each of its parents with
- rename detection, which is expensive when a parent doesn't have the
- followed path yet. Fixing this would require adding even more custom
- code, instead of just extending the existing code in log_tree_diff(). So
- first simplify the code here, and fix the performance regression in a
- follow-up commit.
+ This works, but it causes a performance regression: e.g. 'git log
+ --stat --follow -- include/linux/rsc_table.h' in linux.git takes 6m43s,
+ while it took 28s before that commit. The new code diffs each merge
+ against each of its parents with rename detection, which is expensive
+ when a parent doesn't have the followed path yet. Fixing this would
+ require adding even more custom code, instead of just extending the
+ existing code in log_tree_diff(). So first simplify the code here, and
+ fix the performance regression in a follow-up commit.
Remove propagate_follow_pathspec_to_parent() and the loop calling it for each parent. Instead, record the path to follow for each parent in
- the parent loop of log_tree_diff(), right after diff_tree_oid() updated
+ the parent loop of log_tree_diff(), right after diff_tree_oid() updates
the pathspec in case of a rename. Merges are not diffed by default, so with --follow, run the same loop for them as well, but without showing anything. With -c, --cc or --remerge-diff, show the merge diff after the
5: 072d90b865 < -: ---------- log: speed up --follow following renames for non-linear history
6: 7be8cfb3ba ! 5: ddcac2115e log: fix --follow following an edit on a side the merge did not take
@@ Metadata
Author: Miklos Vajna <[email protected]>
## Commit message ##
- log: fix --follow following an edit on a side the merge did not take
+ log: speed up --follow following renames for non-linear history
- Take a history where F is renamed to G on master, F is edited on a side
- branch, and the side branch is merged with '-s ours', so the merge keeps
- G and drops F. Here 'git log --follow G' doesn't show the edit of F on
- the side branch, while it did before the previous commit.
+ Test the performance of 'git log --stat --follow --
+ include/linux/rsc_table.h' on linux.git: since commit 304812ed33 (log:
+ improve --follow following renames for non-linear history, 2026-06-22),
+ it takes 6m43s, while it took about 28 seconds before.
- The problem is that follow_needs_renames() only searches for renames in
- a parent without the path if the merge base still has the path, which
- covers the case when the parent renamed the path. Here the other side
- of the merge did the rename, so the merge base only has the old name.
- The parent is then told to follow G, which it doesn't have, and the
- history of F on its side is lost. Searching for renames in each parent
- that lacks the path would fix this, but that is the expensive search
- that the previous commit avoids, e.g. 148 merges hit this case for
- include/linux/rsc_table.h in linux.git.
+ What happens is that log_tree_diff() diffs each merge against all of its
+ parents with rename following. diff_tree_oid() only does the expensive
+ rename detection when the path is added relative to a parent, but that
+ is common for merges, e.g. when a branch forked before the path was
+ created merges mainline, and this slows down 'git log --follow'.
- Fix the problem by also searching for renames when the merge base
- doesn't have the path, but the parent changed a file since the merge
- base that the merge no longer has: only such a file can be the old name
- of the path that still needs to be followed in the parent. This check
- is cheap, since it only diffs the changes of the side branch, and it
- rules out 146 of the 148 merges in linux.git. Then do a plain rename
- detection from the parent to the merge, which only looks at the files
- deleted by the merge, not at all files of the parent like --follow does.
+ Fix the problem by first diffing the merge against its parents without
+ rename following, to find a parent where the path is unchanged. If there
+ is such a parent, then skip the diff against the other parents and just
+ follow the same path there. The exception is a parent that doesn't have
+ the path, but still needs renames to be followed:
+ - The merge base has the path, i.e. that side renamed or removed it, or
+ there is no merge base, e.g. a subtree merge of unrelated histories.
+ - The merge base doesn't have the path, but the parent changed a file
+ since then, and the merge dropped that change. A copy detection also
+ finds that file as the source of the path, i.e. the other side renamed
+ or copied the path from that file.
+ If the path changed relative to all parents, then diff against them with
+ rename following, as before. Given that this way a parent may be asked
+ to follow a path that it doesn't have, also adjust
+ remember_follow_pathspec(): if children disagree on what should be the
+ path for a commit, then prefer the path that exists in that commit.
- Reported-by: Patrick Steinhardt <[email protected]>
+ Now the above use case takes 42s, somewhat close to the original cost.
+
+ Helped-by: SZEDER Gábor <[email protected]>
+ Helped-by: Patrick Steinhardt <[email protected]>
Signed-off-by: Miklos Vajna <[email protected]>
## log-tree.c ##
+@@
+ #include "range-diff.h"
+ #include "strmap.h"
+ #include "tree.h"
++#include "tree-walk.h"
+ #include "wildmatch.h"
+ #include "write-or-die.h"
+ #include "pager.h"
@@ log-tree.c: static void set_pathspec_to_single_path(struct pathspec *ps, const char *path)
PATHSPEC_LITERAL_PATH, "", paths);
}
--static int commit_has_path(struct rev_info *opt, struct commit *c,
-- const char *path)
+static int tree_has_path(struct rev_info *opt, struct object_id *tree,
+ const char *path)
- {
- struct object_id oid;
- unsigned short mode;
-
++{
++ struct object_id oid;
++ unsigned short mode;
++
+ return !get_tree_entry(opt->diffopt.repo, tree, path, &oid, &mode);
+}
+
+static int commit_has_path(struct rev_info *opt, struct commit *c,
+ const char *path)
+{
- parse_commit_or_die(c);
-- return !get_tree_entry(opt->diffopt.repo, get_commit_tree_oid(c), path,
-- &oid, &mode);
++ parse_commit_or_die(c);
+ return tree_has_path(opt, get_commit_tree_oid(c), path);
- }
-
++}
++
static void remember_follow_pathspec(struct rev_info *opt,
+ struct commit *c, const char *path)
+ {
+@@ log-tree.c: static void remember_follow_pathspec(struct rev_info *opt,
+ init_follow_pathspec_slab(opt->follow_pathspec_slab);
+ }
+ slot = follow_pathspec_slab_at(opt->follow_pathspec_slab, c);
+- if (*slot && !strcmp(*slot, path))
+- return;
++ /* If children of 'c' disagree on the path, prefer the one 'c' has. */
++ if (*slot) {
++ if (!strcmp(*slot, path) || commit_has_path(opt, c, *slot) ||
++ !commit_has_path(opt, c, path))
++ return;
++ }
+ free(*slot);
+ *slot = xstrdup(path);
+ }
@@ log-tree.c: void release_follow_pathspec_slab(struct rev_info *opt)
FREE_AND_NULL(opt->follow_pathspec_slab);
}
++/* Find a parent where 'path' is the same as in the merge. */
++static struct commit *follow_same_parent(struct rev_info *opt,
++ struct commit_list *parents,
++ struct object_id *merge_tree)
++{
++ struct commit *same = NULL;
++
++ opt->diffopt.flags.follow_renames = 0;
++ for (; parents && !same; parents = parents->next) {
++ parse_commit_or_die(parents->item);
++ diff_tree_oid(get_commit_tree_oid(parents->item), merge_tree,
++ "", &opt->diffopt);
++ if (!diff_queued_diff.nr)
++ same = parents->item;
++ diff_queue_clear(&diff_queued_diff);
++ }
++ opt->diffopt.flags.follow_renames = 1;
++ return same;
++}
++
+/*
-+ * Did 'parent' change a file since 'base' that the merge (with tree 'oid') no
-+ * longer has? Only such a file can be the source of a rename on the other side
-+ * of the merge that still needs to be followed in 'parent'.
++ * Did 'parent' change a file since 'base', but the merge tree dropped that
++ * change? I.e. the merge doesn't have the file or it's the same as in 'same'.
+ */
+static int follow_parent_changed_dropped_file(struct rev_info *opt,
+ struct commit *base,
+ struct commit *parent,
-+ struct object_id *oid)
++ struct commit *same,
++ struct object_id *merge_tree)
+{
+ struct diff_options diff_opts;
+ int i, ret = 0;
@@ log-tree.c: void release_follow_pathspec_slab(struct rev_info *opt)
+ "", &diff_opts);
+ for (i = 0; i < diff_queued_diff.nr && !ret; i++) {
+ struct diff_filepair *p = diff_queued_diff.queue[i];
++ struct object_id merge_oid, same_oid;
++ unsigned short mode;
+
-+ ret = DIFF_FILE_VALID(p->two) &&
-+ !tree_has_path(opt, oid, p->two->path);
++ if (!DIFF_FILE_VALID(p->two))
++ continue;
++ if (get_tree_entry(opt->diffopt.repo, merge_tree, p->two->path,
++ &merge_oid, &mode))
++ ret = 1;
++ else
++ ret = !oideq(&merge_oid, &p->two->oid) &&
++ !get_tree_entry(opt->diffopt.repo,
++ get_commit_tree_oid(same),
++ p->two->path, &same_oid, &mode) &&
++ oideq(&merge_oid, &same_oid);
+ }
+ diff_queue_clear(&diff_queued_diff);
+ diff_free(&diff_opts);
+ return ret;
+}
+
-+/*
-+ * Is 'path' in the merge (with tree 'oid') renamed from a file of 'parent'?
-+ * Only look at the files that the merge deleted, not at all files of 'parent'
-+ * like --follow does.
-+ */
++/* Is 'path' in the merge tree renamed or copied from a file of 'parent'? */
+static int follow_has_rename_source(struct rev_info *opt,
+ struct commit *parent,
-+ struct object_id *oid, const char *path)
++ struct object_id *merge_tree,
++ const char *path)
+{
+ struct diff_options diff_opts;
+ int i, ret = 0;
+
+ repo_diff_setup(opt->diffopt.repo, &diff_opts);
+ diff_opts.flags.recursive = 1;
-+ diff_opts.detect_rename = DIFF_DETECT_RENAME;
++ diff_opts.detect_rename = DIFF_DETECT_COPY;
+ diff_opts.rename_score = opt->diffopt.rename_score;
+ diff_opts.single_follow = path;
+ diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_setup_done(&diff_opts);
-+ diff_tree_oid(get_commit_tree_oid(parent), oid, "", &diff_opts);
++ diff_tree_oid(get_commit_tree_oid(parent), merge_tree, "", &diff_opts);
+ diffcore_std(&diff_opts);
+ for (i = 0; i < diff_queued_diff.nr && !ret; i++) {
+ struct diff_filepair *p = diff_queued_diff.queue[i];
+
-+ ret = p->status == DIFF_STATUS_RENAMED &&
++ ret = (p->status == DIFF_STATUS_RENAMED ||
++ p->status == DIFF_STATUS_COPIED) &&
+ !strcmp(p->two->path, path);
+ }
+ diff_queue_clear(&diff_queued_diff);
@@ log-tree.c: void release_follow_pathspec_slab(struct rev_info *opt)
+ return ret;
+}
+
- /*
- * When the path in a merge is the same as in 'same', does the path still need
- * renames to be followed in 'parent'? Only if the merge has the path, 'parent'
-- * doesn't have it, but its fork point with 'same' had it, e.g. 'parent'
-- * renamed the path.
-+ * doesn't have it, and either:
-+ *
-+ * - its fork point with 'same' had it, e.g. 'parent' renamed the path, or
-+ * - 'same' renamed the path from a file that 'parent' changed.
- */
- static int follow_needs_renames(struct rev_info *opt, struct commit *parent,
-- struct commit *same, const char *path)
-+ struct commit *same, struct object_id *oid,
-+ const char *path)
- {
- struct commit_list *bases = NULL, *base;
-- int ret = 0;
++/*
++ * When the path in a merge tree is the same as in 'same', does the path still
++ * need renames to be followed in 'parent'?
++ */
++static int follow_needs_renames(struct rev_info *opt, struct commit *parent,
++ struct commit *same,
++ struct object_id *merge_tree, const char *path)
++{
++ struct commit_list *bases = NULL, *base;
+ int ret = 0, changed_dropped = 0;
-
- if (commit_has_path(opt, parent, path) ||
- !commit_has_path(opt, same, path))
- return 0;
- if (repo_get_merge_bases(opt->diffopt.repo, parent, same, &bases) < 0)
- exit(128);
-- for (base = bases; base && !ret; base = base->next)
++
++ if (parent == same || commit_has_path(opt, parent, path) ||
++ !commit_has_path(opt, same, path))
++ return 0;
++ if (repo_get_merge_bases(opt->diffopt.repo, parent, same, &bases) < 0)
++ exit(128);
++ /* Unrelated histories, e.g. a subtree merge */
++ ret = !bases;
+ for (base = bases; base && !ret; base = base->next) {
- ret = commit_has_path(opt, base->item, path);
++ ret = commit_has_path(opt, base->item, path);
+ if (!ret && !changed_dropped)
+ changed_dropped = follow_parent_changed_dropped_file(
-+ opt, base->item, parent, oid);
++ opt, base->item, parent, same, merge_tree);
+ }
- commit_list_free(bases);
++ commit_list_free(bases);
+ if (!ret && changed_dropped)
-+ ret = follow_has_rename_source(opt, parent, oid, path);
- return ret;
- }
++ ret = follow_has_rename_source(opt, parent, merge_tree, path);
++ return ret;
++}
++
+ /* Show the remerge or combined diff of a merge commit */
+ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,
+ struct commit_list *parents,
+@@ log-tree.c: static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
+ int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;
+ int follow_only = 0;
+ char *path = NULL;
++ struct commit *same = NULL;
+ if (!all_need_diff && !opt->merges_need_diff)
+ return 0;
@@ log-tree.c: static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
- parents = parents->next) {
- if (parents->item != parent &&
- follow_needs_renames(opt, parents->item,
-- parent, path))
-+ parent, oid, path))
- tail = &commit_list_insert(parents->item,
- tail)->next;
- else
-
- ## t/t4219-log-follow-merge.sh ##
-@@ t/t4219-log-follow-merge.sh: test_expect_success '--follow with --diff-merges=first-parent follows all parent
- test_cmp expect actual
- '
+ if (opt->diffopt.flags.follow_renames)
+ path = xstrdup_or_null(
+ pathspec_single_path(&opt->diffopt.pathspec));
++ if (path && is_merge && (follow_only || opt->first_parent_merges))
++ same = follow_same_parent(opt, parents, oid);
-+test_expect_success 'setup merge keeping the renamed path, other side edited the old one' '
-+ git init keep-renamed &&
-+ printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-renamed/F &&
-+ git -C keep-renamed add F &&
-+ test_tick &&
-+ git -C keep-renamed commit -m "O: add F" &&
-+ git -C keep-renamed branch B &&
-+ git -C keep-renamed mv F G &&
-+ test_tick &&
-+ git -C keep-renamed commit -m "A1: rename F to G on master" &&
-+
-+ git -C keep-renamed checkout B &&
-+ printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-renamed/F &&
-+ test_tick &&
-+ git -C keep-renamed commit -am "B2: update F on B" &&
-+
-+ git -C keep-renamed checkout master &&
-+ test_tick &&
-+ git -C keep-renamed merge -s ours -m "M: merge B, keep G, drop F" B
-+'
-+
-+test_expect_success '--follow follows an edit on the side the merge did not take' '
-+ git -C keep-renamed log --follow --pretty=tformat:%s G >actual &&
-+ cat >expect <<-\EOF &&
-+ B2: update F on B
-+ A1: rename F to G on master
-+ O: add F
-+ EOF
-+ test_cmp expect actual
-+'
-+
- test_done
+ showed_log = 0;
+ all_parents = parents;
+@@ log-tree.c: static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
+ struct commit *parent = parents->item;
+
+ parse_commit_or_die(parent);
+- diff_tree_oid(get_commit_tree_oid(parent),
+- oid, "", &opt->diffopt);
++ /*
++ * Same as in a parent: follow the path as-is, unless renames
++ * still need to be followed in this parent.
++ */
++ if (!follow_only || !same ||
++ follow_needs_renames(opt, parent, same, oid, path))
++ diff_tree_oid(get_commit_tree_oid(parent),
++ oid, "", &opt->diffopt);
+ if (follow_only) {
+ diff_queue_clear(&diff_queued_diff);
+ opt->diffopt.found_follow = 0;Signature · 1 line
2.51.0
PATCH v5 1/5 t4219: test more --follow cases across merges
A later commit in this series will speed up 'git log --follow' for merge commits. Add tests for cases which already work, but have no test coverage, so it's easy to break them while optimizing:
- a subtree merge adding the history of a file that was copied before
- --follow through a fork with renames on both sides, with -c, --cc and --remerge-diff
- a --no-ff merge taking a rename from the second parent, in both commit orders, since the result could depend on the walk order
- a merge keeping the path, while the other side renamed it
- --diff-merges=first-parent, which diffs merges only against their first parent, but still walks all parents
- a merge keeping the renamed path, while the other side edited the old one
- a merge keeping the copied path, while the other side edited the original
--- t/t4219-log-follow-merge.sh | 214 ++++++++++++++++++++++++++++++++++++ 1 file changed, 214 insertions(+)
t/t4219-log-follow-merge.sh +214 -0
@@ -31,6 +31,33 @@ test_expect_success '--follow finds the pre-merge commit through a subtree merge test_cmp expect actual ' +test_expect_success 'setup subtree merge adding the history of a copied file' ' + git init graft && + echo outer >graft/outer.txt && + git -C graft add outer.txt && + test_tick && + git -C graft commit -m "outer init" && + mkdir graft/inner && + cp inner/inner.txt graft/inner/ && + git -C graft add inner && + test_tick && + git -C graft commit -m "copy inner.txt to inner/" && + + git -C graft fetch ../inner master && + test_tick && + git -C graft merge -s ours --allow-unrelated-histories \ + -m "Merge the history of inner/" FETCH_HEAD +' + +test_expect_success '--follow finds the history of a copied file through a subtree merge' ' + git -C graft log --follow --pretty=tformat:%s inner/inner.txt >actual && + cat >expect <<-\EOF && + copy inner.txt to inner/ + inner init + EOF + test_cmp expect actual +' + test_expect_success 'setup merge of two branches that both renamed a file to README' ' git init foo && mkdir foo/foo &&
@@ -126,4 +153,191 @@ test_expect_success '--follow follows renames through a fork in a single history test_cmp expect actual.sorted ' +for opt in -c --cc --remerge-diff +do + test_expect_success "--follow follows renames through a fork with $opt" ' + git -C diamond log --follow $opt --pretty=tformat:%s path >log && + grep "^[A-Z]: " log | sort >actual && + cat >expect <<-\EOF && + A: add path0 + B: modify path0 on upper + C: modify path0 on lower + M: merge with rename to path + X: rename path0 to path1 + Y: rename path0 to path2 + Z: modify path + EOF + test_cmp expect actual + ' +done + +# Build a merge that takes the renamed path from the second parent, while the +# first parent only has the old path. $2 decides which side is committed first, +# which decides the walk order. +setup_rename_on_second_parent () { + git init $1 && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >$1/old && + git -C $1 add old && + test_tick && + git -C $1 commit -m "A: add old" && + printf "line 1\nline 2\nline 3 modified\nline 4\nline 5\n" >$1/old && + test_tick && + git -C $1 commit -am "B: modify old" && + git -C $1 branch topic && + + if test "$2" = topic-first + then + git -C $1 checkout topic && + git -C $1 mv old new && + test_tick && + git -C $1 commit -m "R: rename old to new" && + git -C $1 checkout master + fi && + + echo other >$1/other && + git -C $1 add other && + test_tick && + git -C $1 commit -m "M: unrelated change on master" && + + if test "$2" = master-first + then + git -C $1 checkout topic && + git -C $1 mv old new && + test_tick && + git -C $1 commit -m "R: rename old to new" && + git -C $1 checkout master + fi && + + test_tick && + git -C $1 merge --no-ff -m "merge topic" topic +} + +test_expect_success 'setup merge taking a rename from the second parent' ' + setup_rename_on_second_parent rename-master-first master-first && + setup_rename_on_second_parent rename-topic-first topic-first +' + +for order in master-first topic-first +do + test_expect_success "--follow follows a rename from the second parent ($order)" ' + git -C rename-$order log --follow --pretty=tformat:%s new >actual && + cat >expect <<-\EOF && + R: rename old to new + B: modify old + A: add old + EOF + test_cmp expect actual + ' +done + +test_expect_success 'setup merge keeping the path, other side renamed it' ' + git init keep && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep/F && + git -C keep add F && + test_tick && + git -C keep commit -m "O: add F" && + git -C keep branch B && + printf "line 1\nline 2\nline 3 A\nline 4\nline 5\n" >keep/F && + test_tick && + git -C keep commit -am "A1: update F in place" && + + git -C keep checkout B && + git -C keep mv F G && + test_tick && + git -C keep commit -m "B1: rename F to G" && + printf "line 1\nline 2\nline 3\nline 4 B\nline 5\n" >keep/G && + test_tick && + git -C keep commit -am "B2: update G" && + + git -C keep checkout master && + test_tick && + git -C keep merge -s ours -m "M: merge B, keep F, drop G" B +' + +test_expect_success '--follow follows a rename on the side the merge did not take' ' + git -C keep log --follow --pretty=tformat:%s F >actual && + cat >expect <<-\EOF && + B2: update G + B1: rename F to G + A1: update F in place + O: add F + EOF + test_cmp expect actual +' + +test_expect_success '--follow with --diff-merges=first-parent follows all parents' ' + git -C keep log --follow --diff-merges=first-parent --no-patch \ + --pretty=tformat:%s F >actual && + cat >expect <<-\EOF && + B2: update G + B1: rename F to G + A1: update F in place + O: add F + EOF + test_cmp expect actual +' + +test_expect_success 'setup merge keeping the renamed path, other side edited the old one' ' + git init keep-renamed && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-renamed/F && + git -C keep-renamed add F && + test_tick && + git -C keep-renamed commit -m "O: add F" && + git -C keep-renamed branch B && + git -C keep-renamed mv F G && + test_tick && + git -C keep-renamed commit -m "A1: rename F to G on master" && + + git -C keep-renamed checkout B && + printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-renamed/F && + test_tick && + git -C keep-renamed commit -am "B2: update F on B" && + + git -C keep-renamed checkout master && + test_tick && + git -C keep-renamed merge -s ours -m "M: merge B, keep G, drop F" B +' + +test_expect_success '--follow follows an edit on the side the merge did not take' ' + git -C keep-renamed log --follow --pretty=tformat:%s G >actual && + cat >expect <<-\EOF && + B2: update F on B + A1: rename F to G on master + O: add F + EOF + test_cmp expect actual +' + +test_expect_success 'setup merge keeping the copied path, other side edited the original' ' + git init keep-copied && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-copied/F && + git -C keep-copied add F && + test_tick && + git -C keep-copied commit -m "O: add F" && + git -C keep-copied branch B && + cp keep-copied/F keep-copied/G && + git -C keep-copied add G && + test_tick && + git -C keep-copied commit -m "A1: copy F to G on master" && + + git -C keep-copied checkout B && + printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-copied/F && + test_tick && + git -C keep-copied commit -am "B2: update F on B" && + + git -C keep-copied checkout master && + test_tick && + git -C keep-copied merge -s ours -m "M: merge B, keep F and G" B +' + +test_expect_success '--follow follows an edit of the copy source on the side the merge did not take' ' + git -C keep-copied log --follow --pretty=tformat:%s G >actual && + cat >expect <<-\EOF && + B2: update F on B + A1: copy F to G on master + O: add F + EOF + test_cmp expect actual +' + test_done
Signature · 1 line
2.51.0
Re: [PATCH v5 1/5] t4219: test more --follow cases across merges
Quoted 4 lines · Miklos Vajna
A later commit in this series will speed up 'git log --follow' for merge commits. Add tests for cases which already work, but have no test coverage, so it's easy to break them while optimizing:
Quoted 12 lines
- a subtree merge adding the history of a file that was copied before
- --follow through a fork with renames on both sides, with -c, --cc and --remerge-diff
- a --no-ff merge taking a rename from the second parent, in both commit orders, since the result could depend on the walk order
- a merge keeping the path, while the other side renamed it
- --diff-merges=first-parent, which diffs merges only against their first parent, but still walks all parents
- a merge keeping the renamed path, while the other side edited the old one
- a merge keeping the copied path, while the other side edited the original
This explains what tests we are adding, but it doesn't really provide readers with much context as do why they are the expected behaviors.
Quoted 22 lines
Helped-by Patrick Steinhardt <[email protected]>Signed-off-by Miklos Vajna <[email protected]>---
t/t4219-log-follow-merge.sh| 214 ++++++++++++++++++++++++++++++++++++1 file changed, 214 insertions(+)t/t4219-log-follow-merge.sh +0
-0@@ -31,6 +31,33 @@ test_expect_success '--follow finds the pre-merge commit through a subtree merge test_cmp expect actual'
t/t4219-log-follow-merge.sh +7
-0+test_expect_success 'setup subtree merge adding the history of a copied file' ' + git init graft && + echo outer >graft/outer.txt && + git -C graft add outer.txt && + test_tick && + git -C graft commit -m "outer init" && + mkdir graft/inner &&
t/t4219-log-follow-merge.sh test_expect_success '--follow finds the pre-merge commit through a subtree merge · line 50
Quoted 10 lines
t/t4219-log-follow-merge.sh +10
-0+ cp inner/inner.txt graft/inner/ && + git -C graft add inner && + test_tick && + git -C graft commit -m "copy inner.txt to inner/" && + + git -C graft fetch ../inner master && + test_tick && + git -C graft merge -s ours --allow-unrelated-histories \ + -m "Merge the history of inner/" FETCH_HEAD +'
Ok IIUC, this test is setting up a repository has manually copied and committed from another repository, but then later merges the unrelated history. It took me a little bit to parse what exactly was going on here. Maybe some comments would be a bit helpful here?
t/t4219-log-follow-merge.sh test_expect_success '--follow finds the pre-merge commit through a subtree merge · line 59
Quoted 9 lines
t/t4219-log-follow-merge.sh +9
-0+ +test_expect_success '--follow finds the history of a copied file through a subtree merge' ' + git -C graft log --follow --pretty=tformat:%s inner/inner.txt >actual && + cat >expect <<-\EOF && + copy inner.txt to inner/ + inner init + EOF + test_cmp expect actual +'
Ok and here we validate that we follow the file into the now merged unrelated history. I assume that prior to 304812ed33de (log: improve --follow following renames for non-linear history, 2026-06-22), this would not have passed? If so, it might be helpful to more directly mention this in the commit message?
Quoted 15 lines
+
test_expect_success'setup merge of two branches that both renamed a file to README' 'git init foo && mkdir foo/foo &&t/t4219-log-follow-merge.sh +0
-0@@ -126,4 +153,191 @@ test_expect_success '--follow follows renames through a fork in a single history test_cmp expect actual.sorted'
t/t4219-log-follow-merge.sh +7
-0+for opt in -c --cc --remerge-diff +do + test_expect_success "--follow follows renames through a fork with $opt" ' + git -C diamond log --follow $opt --pretty=tformat:%s path >log && + grep "^[A-Z]: " log | sort >actual && + cat >expect <<-\EOF && + A: add path0
t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 172
Quoted 10 lines
t/t4219-log-follow-merge.sh +10
-0+ B: modify path0 on upper + C: modify path0 on lower + M: merge with rename to path + X: rename path0 to path1 + Y: rename path0 to path2 + Z: modify path + EOF + test_cmp expect actual + ' +done
I assume here we are validating that the merge is printed?
Quoted 31 lines
t/t4219-log-follow-merge.sh +31
-0+ +# Build a merge that takes the renamed path from the second parent, while the +# first parent only has the old path. $2 decides which side is committed first, +# which decides the walk order. +setup_rename_on_second_parent () { + git init $1 && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >$1/old && + git -C $1 add old && + test_tick && + git -C $1 commit -m "A: add old" && + printf "line 1\nline 2\nline 3 modified\nline 4\nline 5\n" >$1/old && + test_tick && + git -C $1 commit -am "B: modify old" && + git -C $1 branch topic && + + if test "$2" = topic-first + then + git -C $1 checkout topic && + git -C $1 mv old new && + test_tick && + git -C $1 commit -m "R: rename old to new" && + git -C $1 checkout master + fi && + + echo other >$1/other && + git -C $1 add other && + test_tick && + git -C $1 commit -m "M: unrelated change on master" && + + if test "$2" = master-first + then
t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 213
Quoted 10 lines
t/t4219-log-follow-merge.sh +10
-0+ git -C $1 checkout topic && + git -C $1 mv old new && + test_tick && + git -C $1 commit -m "R: rename old to new" && + git -C $1 checkout master + fi && + + test_tick && + git -C $1 merge --no-ff -m "merge topic" topic +}
Ok, because the walk order is based on commiter date, we have a helper function here that can create various test setups to validate we can follow a file through the history.
Quoted 8 lines
t/t4219-log-follow-merge.sh +8
-0+ +test_expect_success 'setup merge taking a rename from the second parent' ' + setup_rename_on_second_parent rename-master-first master-first && + setup_rename_on_second_parent rename-topic-first topic-first +' + +for order in master-first topic-first +do
t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 231
Quoted 10 lines
t/t4219-log-follow-merge.sh +10
-0+ test_expect_success "--follow follows a rename from the second parent ($order)" ' + git -C rename-$order log --follow --pretty=tformat:%s new >actual && + cat >expect <<-\EOF && + R: rename old to new + B: modify old + A: add old + EOF + test_cmp expect actual + ' +done
Ok, so we expect the results to be same the same and the file to be found on both sides here. I realize we are crafting very specific repository histories to validate here, but I must say the test setup here is rather complex IMO.
Quoted 14 lines
t/t4219-log-follow-merge.sh +14
-0+ +test_expect_success 'setup merge keeping the path, other side renamed it' ' + git init keep && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep/F && + git -C keep add F && + test_tick && + git -C keep commit -m "O: add F" && + git -C keep branch B && + printf "line 1\nline 2\nline 3 A\nline 4\nline 5\n" >keep/F && + test_tick && + git -C keep commit -am "A1: update F in place" && + + git -C keep checkout B && + git -C keep mv F G &&
t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 255
Quoted 10 lines
t/t4219-log-follow-merge.sh +10
-0+ test_tick && + git -C keep commit -m "B1: rename F to G" && + printf "line 1\nline 2\nline 3\nline 4 B\nline 5\n" >keep/G && + test_tick && + git -C keep commit -am "B2: update G" && + + git -C keep checkout master && + test_tick && + git -C keep merge -s ours -m "M: merge B, keep F, drop G" B +'
Ok, here we are creating a history where a topic modifies and renames a file, but the merge drops these changes. Some comments here might also be helpful for readers.
t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 266
Quoted 11 lines
t/t4219-log-follow-merge.sh +11
-0+ +test_expect_success '--follow follows a rename on the side the merge did not take' ' + git -C keep log --follow --pretty=tformat:%s F >actual && + cat >expect <<-\EOF && + B2: update G + B1: rename F to G + A1: update F in place + O: add F + EOF + test_cmp expect actual +'
We expect that the commits from branch B are still inspected even though the merge dropped them. Makes sense.
t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 278
Quoted 12 lines
t/t4219-log-follow-merge.sh +12
-0+ +test_expect_success '--follow with --diff-merges=first-parent follows all parents' ' + git -C keep log --follow --diff-merges=first-parent --no-patch \ + --pretty=tformat:%s F >actual && + cat >expect <<-\EOF && + B2: update G + B1: rename F to G + A1: update F in place + O: add F + EOF + test_cmp expect actual +'
hmmmm, is the expectation here that --diff-merges=first-parent doesn't prevent us from visiting commits from the second parent?
Quoted 11 lines
t/t4219-log-follow-merge.sh +11
-0+ +test_expect_success 'setup merge keeping the renamed path, other side edited the old one' ' + git init keep-renamed && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-renamed/F && + git -C keep-renamed add F && + test_tick && + git -C keep-renamed commit -m "O: add F" && + git -C keep-renamed branch B && + git -C keep-renamed mv F G && + test_tick && + git -C keep-renamed commit -m "A1: rename F to G on master" &&
t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 299
Quoted 10 lines
t/t4219-log-follow-merge.sh +10
-0+ + git -C keep-renamed checkout B && + printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-renamed/F && + test_tick && + git -C keep-renamed commit -am "B2: update F on B" && + + git -C keep-renamed checkout master && + test_tick && + git -C keep-renamed merge -s ours -m "M: merge B, keep G, drop F" B +'
Here the merge keeps master's renamed G and discards branch B's edit to F.
t/t4219-log-follow-merge.sh test_expect_success '--follow follows renames through a fork in a single history · line 309
Quoted 10 lines
t/t4219-log-follow-merge.sh +10
-0+ +test_expect_success '--follow follows an edit on the side the merge did not take' ' + git -C keep-renamed log --follow --pretty=tformat:%s G >actual && + cat >expect <<-\EOF && + B2: update F on B + A1: rename F to G on master + O: add F + EOF + test_cmp expect actual +'
Commits from both sides along with the original commit are expected. Makes sense.
Quoted 33 lines
t/t4219-log-follow-merge.sh +33
-0+ +test_expect_success 'setup merge keeping the copied path, other side edited the original' ' + git init keep-copied && + printf "line 1\nline 2\nline 3\nline 4\nline 5\n" >keep-copied/F && + git -C keep-copied add F && + test_tick && + git -C keep-copied commit -m "O: add F" && + git -C keep-copied branch B && + cp keep-copied/F keep-copied/G && + git -C keep-copied add G && + test_tick && + git -C keep-copied commit -m "A1: copy F to G on master" && + + git -C keep-copied checkout B && + printf "line 1\nline 2\nline 3\nline 4 T\nline 5\n" >keep-copied/F && + test_tick && + git -C keep-copied commit -am "B2: update F on B" && + + git -C keep-copied checkout master && + test_tick && + git -C keep-copied merge -s ours -m "M: merge B, keep F and G" B +' + +test_expect_success '--follow follows an edit of the copy source on the side the merge did not take' ' + git -C keep-copied log --follow --pretty=tformat:%s G >actual && + cat >expect <<-\EOF && + B2: update F on B + A1: copy F to G on master + O: add F + EOF + test_cmp expect actual +' +
Quoted 1 line
test_done
Ok, these tests are similar to last set, but copied instead of renamed.
-Justin
PATCH v5 2/5 log: extract showing the remerge/combined diff of a merge commit
In reply to Miklos VajnaNo functional changes intended. A later commit in this series will also call log_tree_merge_diff() after the parent loop of log_tree_diff(), so extract that code to a function to avoid duplicating it.
--- log-tree.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-)
log-tree.c +22 -14
@@ -1180,6 +1180,26 @@ static void propagate_follow_pathspec_to_parent(struct rev_info *opt,diff_free (&diff_opts); } +/* Show the remerge or combined diff of a merge commit */ +static int log_tree_merge_diff (struct rev_info *opt,struct commit *commit, +struct commit_list *parents, +struct object_id *oid) +{ +int octopus = (parents->next->next !=NULL ); + +if (opt->remerge_diff) { +if (octopus) { +show_log (opt); +fprintf (opt->diffopt.file, +"diff: warning: Skipping remerge-diff " +"for octopus merges.\n" ); +return 1 ; + } +return do_remerge_diff (opt, parents, oid); + } +return do_diff_combined (opt, commit); +} +/* * Show the diff of a commit. *
@@ -1220,20 +1240,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log }if (is_merge) { -int octopus = (parents->next->next !=NULL ); - -if (opt->remerge_diff) { -if (octopus) { -show_log (opt); -fprintf (opt->diffopt.file, -"diff: warning: Skipping remerge-diff " -"for octopus merges.\n" ); -return 1 ; - } -return do_remerge_diff (opt, parents, oid); - } -if (opt->combine_merges) -return do_diff_combined (opt, commit); +if (opt->remerge_diff || opt->combine_merges) +return log_tree_merge_diff (opt, commit, parents, oid);if (opt->separate_merges) {if (!opt->first_parent_merges) {/* Show parent info for multiple diffs */
Signature · 1 line
2.51.0
PATCH v5 3/5 log: fix -m --follow losing the history of later parents
In reply to Miklos VajnaWith 'git log -m --follow', a merge is diffed against each of its parents, with rename following. Take the diamond case of t4219: the two sides of a fork rename path0 to path1 and to path2, and the merge renames the file to path. Here 'git log -m --follow path' doesn't show "A: add path0" and "C: modify path0 on lower".
The problem is that the parent loop in log_tree_diff() doesn't restore the followed path between parents. When the diff against the first parent finds a rename, the followed path is updated, and this has two effects. First, the second parent is diffed with the already renamed path, not with the path of the merge. Second, the path that propagate_follow_pathspec_to_parent() records for each parent also starts from that updated path, so history is lost on the side of the second parent. Without -m, merges are not diffed, so the followed path is not updated there.
Fix the problem by saving the path of the merge before the parent loop, and restoring it before diffing each parent and after the loop. Do this only for merges: for a commit with a single parent, the renamed path is what --follow needs to continue with. Add a test for this, which shows the full history of the diamond case with -m.
--- log-tree.c | 13 +++++++++++++ t/t4219-log-follow-merge.sh | 16 ++++++++++++++++ 2 files changed, 29 insertions(+)
log-tree.c +13 -0
@@ -1212,6 +1212,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct logstruct object_id *oid;int is_merge;int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status; +char *path =NULL ;if (!all_need_diff && !opt->merges_need_diff)return 0 ;
@@ -1251,10 +1252,18 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct logreturn 0 ; } +if (is_merge && opt->diffopt.flags.follow_renames) + path =xstrdup_or_null ( +pathspec_single_path (&opt->diffopt.pathspec)); + showed_log =0 ;for (;;) {struct commit *parent = parents->item; +/* Diff each parent with the path of the merge */ +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path);parse_commit_or_die (parent);diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &opt->diffopt);
@@ -1269,6 +1278,10 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log log->parent = parents->item; opt->loginfo = log; } +/* Propagate to parents starting from the path of the merge, too */ +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, path); +free (path);return showed_log; }
t/t4219-log-follow-merge.sh +16 -0
@@ -171,6 +171,22 @@ do ' done +test_expect_success '--follow follows renames through a fork with -m' ' + git -C diamond log --follow -m --pretty=tformat:%s path >log && + grep "^[A-Z]: " log | sort >actual && + cat >expect <<-\EOF && + A: add path0 + B: modify path0 on upper + C: modify path0 on lower + M: merge with rename to path + M: merge with rename to path + X: rename path0 to path1 + Y: rename path0 to path2 + Z: modify path + EOF + test_cmp expect actual +' + # Build a merge that takes the renamed path from the second parent, while the # first parent only has the old path. $2 decides which side is committed first, # which decides the walk order.
Signature · 1 line
2.51.0
PATCH v5 4/5 log: simplify --follow following renames for non-linear history
In reply to Miklos VajnaCode added in 304812ed33 (log: improve --follow following renames for non-linear history, 2026-06-22) implements the feature by introducing a new propagate_follow_pathspec_to_parent() function which adds a new call to diff_tree_oid(). Also it introduces a new loop to record what path each parent of a merge commit should use when following a path.
This works, but it causes a performance regression: e.g. 'git log --stat --follow -- include/linux/rsc_table.h' in linux.git takes 6m43s, while it took 28s before that commit. The new code diffs each merge against each of its parents with rename detection, which is expensive when a parent doesn't have the followed path yet. Fixing this would require adding even more custom code, instead of just extending the existing code in log_tree_diff(). So first simplify the code here, and fix the performance regression in a follow-up commit.
Remove propagate_follow_pathspec_to_parent() and the loop calling it for each parent. Instead, record the path to follow for each parent in the parent loop of log_tree_diff(), right after diff_tree_oid() updates the pathspec in case of a rename. Merges are not diffed by default, so with --follow, run the same loop for them as well, but without showing anything. With -c, --cc or --remerge-diff, show the merge diff after the loop. With --diff-merges=first-parent, keep diffing the other parents silently, unless the walk only follows first parents. This gets rid of the additional call to diff_tree_oid(). The rework has no positive performance impact yet, it just simplifies the code.
The previous commit restores the path of the merge before diffing each parent, and also after the loop, for propagate_follow_pathspec_to_parent(). Keep restoring it between parents, but not after the loop, since there is no later propagation anymore.
--- log-tree.c | 96 +++++++++++++++++++++++------------------------------- 1 file changed, 41 insertions(+), 55 deletions(-)
log-tree.c +41 -55
@@ -1153,33 +1153,6 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } -/* Compute a path to follow in parent, if there is one */ -static void propagate_follow_pathspec_to_parent (struct rev_info *opt, -struct commit *commit, -struct commit *parent) -{ -struct diff_options diff_opts; -const char *path; - -parse_commit_or_die (parent); -repo_diff_setup (opt->diffopt.repo, &diff_opts); -copy_pathspec (&diff_opts.pathspec, &opt->diffopt.pathspec); - diff_opts.flags.recursive =1 ; - diff_opts.flags.follow_renames =1 ; - diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; -diff_setup_done (&diff_opts); -diff_tree_oid (get_commit_tree_oid (parent), -get_commit_tree_oid (commit), -"" , &diff_opts); - - path =pathspec_single_path (&diff_opts.pathspec); -if (path) -remember_follow_pathspec (opt, parent, path); - -diff_queue_clear (&diff_queued_diff); -diff_free (&diff_opts); -} -/* Show the remerge or combined diff of a merge commit */ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit, struct commit_list *parents,
@@ -1208,10 +1181,11 @@ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,static int log_tree_diff (struct rev_info *opt,struct commit *commit,struct log_info *log) {int showed_log; -struct commit_list *parents; +struct commit_list *parents, *all_parents;struct object_id *oid;int is_merge;int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status; +int follow_only =0 ;char *path =NULL ;if (!all_need_diff && !opt->merges_need_diff)
@@ -1241,46 +1215,73 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log }if (is_merge) { -if (opt->remerge_diff || opt->combine_merges) +int merge_diff = opt->remerge_diff || opt->combine_merges; + +if (merge_diff && !opt->diffopt.flags.follow_renames)return log_tree_merge_diff (opt, commit, parents, oid); -if (opt->separate_merges) { +if (opt->separate_merges && !merge_diff) {if (!opt->first_parent_merges) {/* Show parent info for multiple diffs */ log->parent = parents->item; } + }else if (opt->diffopt.flags.follow_renames) { +/* Diff only to find the path to follow in parents */ + follow_only =1 ; }else return 0 ; } -if (is_merge && opt->diffopt.flags.follow_renames) +if (opt->diffopt.flags.follow_renames) path =xstrdup_or_null (pathspec_single_path (&opt->diffopt.pathspec)); showed_log =0 ; + all_parents = parents;for (;;) {struct commit *parent = parents->item; -/* Diff each parent with the path of the merge */ -if (path) -set_pathspec_to_single_path (&opt->diffopt.pathspec, - path);parse_commit_or_die (parent);diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &opt->diffopt); -log_tree_diff_flush (opt); +if (follow_only) { +diff_queue_clear (&diff_queued_diff); + opt->diffopt.found_follow =0 ; + }else { +log_tree_diff_flush (opt); + } showed_log |= !opt->loginfo; +/* The diff updated the pathspec in case of a rename */ +if (path) +remember_follow_pathspec (opt, parent, +pathspec_single_path (&opt->diffopt.pathspec)); +/* Set up the log info for the next parent, if any.. */ parents = parents->next; -if (!parents || opt->first_parent_merges) +if (!parents)break ; +if (opt->first_parent_merges) { +if (!path || opt->first_parent_only) +break ; +/* Only find the path to follow in the other parents */ + follow_only =1 ; + } +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path); +if (follow_only) +continue ; log->parent = parents->item; opt->loginfo = log; } -/* Propagate to parents starting from the path of the merge, too */ -if (path) -set_pathspec_to_single_path (&opt->diffopt.pathspec, path); +if (follow_only && (opt->remerge_diff || opt->combine_merges)) { +/* Show the merge diff with the path of this commit */ +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path); + showed_log =log_tree_merge_diff (opt, commit, all_parents, oid); + }free (path);return showed_log; }
@@ -1319,21 +1320,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)fprintf (opt->diffopt.file,"\n%s\n" , opt->break_bar);if (shown)show_diff_of_diff (opt); - -/* Record what path each parent of this commit should use */ -if (opt->diffopt.flags.follow_renames) { -struct commit_list *parents =get_saved_parents (opt, commit); -if (parents && parents->next) { -struct commit_list *p; -for (p = parents; p; p = p->next) -propagate_follow_pathspec_to_parent (opt, commit, - p->item); - }else if (parents) { -remember_follow_pathspec (opt, parents->item, -pathspec_single_path (&opt->diffopt.pathspec)); - } - } - opt->loginfo =NULL ;maybe_flush_or_die (opt->diffopt.file,"stdout" ); opt->diffopt.no_free = no_free;
Signature · 1 line
2.51.0
Re: [PATCH v5 4/5] log: simplify --follow following renames for non-linear history
Quoted 98 lines · Miklos Vajna
Code added in
304812ed33(log: improve --follow following renames for non-linear history, 2026-06-22) implements the feature by introducing a newpropagate_follow_pathspec_to_parent()function which adds a new call todiff_tree_oid(). Also it introduces a new loop to record what path each parent of a merge commit should use when following a path.This works, but it causes a performance regression: e.g. 'git log --stat --follow --
include/linux/rsc_table.h' in linux.git takes 6m43s, while it took 28s before that commit. The new code diffs each merge against each of its parents with rename detection, which is expensive when a parent doesn't have the followed path yet. Fixing this would require adding even more custom code, instead of just extending the existing code inlog_tree_diff(). So first simplify the code here, and fix the performance regression in a follow-up commit.Remove
propagate_follow_pathspec_to_parent()and the loop calling it for each parent. Instead, record the path to follow for each parent in the parent loop oflog_tree_diff(), right afterdiff_tree_oid()updates the pathspec in case of a rename. Merges are not diffed by default, so with --follow, run the same loop for them as well, but without showing anything. With -c, --cc or --remerge-diff, show the merge diff after the loop. With --diff-merges=first-parent, keep diffing the other parents silently, unless the walk only follows first parents. This gets rid of the additional call todiff_tree_oid(). The rework has no positive performance impact yet, it just simplifies the code.The previous commit restores the path of the merge before diffing each parent, and also after the loop, for
propagate_follow_pathspec_to_parent(). Keep restoring it between parents, but not after the loop, since there is no later propagation anymore.Signed-off-by Miklos Vajna <[email protected]>--- log-tree.c | 96 +++++++++++++++++++++++------------------------------- 1 file changed, 41 insertions(+), 55 deletions(-)log-tree.c +5
-30@@ -1153,33 +1153,6 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } -/* Compute a path to follow in parent, if there is one */ -static void propagate_follow_pathspec_to_parent (struct rev_info *opt, -struct commit *commit, -struct commit *parent) -{ -struct diff_options diff_opts; -const char *path; - -parse_commit_or_die (parent); -repo_diff_setup (opt->diffopt.repo, &diff_opts); -copy_pathspec (&diff_opts.pathspec, &opt->diffopt.pathspec); - diff_opts.flags.recursive =1 ; - diff_opts.flags.follow_renames =1 ; - diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; -diff_setup_done (&diff_opts); -diff_tree_oid (get_commit_tree_oid (parent), -get_commit_tree_oid (commit), -"" , &diff_opts); - - path =pathspec_single_path (&diff_opts.pathspec); -if (path) -remember_follow_pathspec (opt, parent, path); - -diff_queue_clear (&diff_queued_diff); -diff_free (&diff_opts); -} -/* Show the remerge or combined diff of a merge commit */ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit, struct commit_list *parents,@@ -1208,10 +1181,11 @@ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit,static int log_tree_diff (struct rev_info *opt,struct commit *commit,struct log_info *log) {int showed_log; -struct commit_list *parents; +struct commit_list *parents, *all_parents;struct object_id *oid;int is_merge;int all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status; +int follow_only =0 ;char *path =NULL ;if (!all_need_diff && !opt->merges_need_diff)@@ -1241,46 +1215,73 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log }if (is_merge) { -if (opt->remerge_diff || opt->combine_merges) +int merge_diff = opt->remerge_diff || opt->combine_merges; + +if (merge_diff && !opt->diffopt.flags.follow_renames)return log_tree_merge_diff (opt, commit, parents, oid); -if (opt->separate_merges) {
log-tree.c static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log · line 1231
Quoted 10 lines
log-tree.c +4
-0+if (opt->separate_merges && !merge_diff) {if (!opt->first_parent_merges) {/* Show parent info for multiple diffs */ log->parent = parents->item; } + }else if (opt->diffopt.flags.follow_renames) { +/* Diff only to find the path to follow in parents */ + follow_only =1 ; }else return 0 ;
This part of code has become pretty tough to read if you ask me. I've tried to rewrite it:
if (is_merge) {
/* -c, --cc or --remerge-diff: show one diff for the whole merge. */
int merge_diff = opt->remerge_diff || opt->combine_merges;
/* Without --follow the parent loop is only needed for -m. */
if (!opt->diffopt.flags.follow_renames) {
/* -c, --cc or --remerge-diff: show it, no parent loop. */
if (merge_diff)
return log_tree_merge_diff(opt, commit, parents, oid);
/* No -m either, i.e. merge diffs are off: show nothing. */
if (!opt->separate_merges)
return 0;
}
/* Run the loop silently, unless -m shows a diff per parent. */
follow_only = !opt->separate_merges || merge_diff;
/* -m showing all parents: label each diff "(from <parent>)". */
if (!follow_only && !opt->first_parent_merges)
/* Label the first diff, others are labeled in parent loop. */
log->parent = parents->item;
}The comments I've added are possibly too verbose for the Git project, but they helped me understand what happens at each line. I hope it's functionally the same?
(This is as far as I have gotten in the review today, but I wanted to send this out already)
Quoted 91 lines
log-tree.c +32
-25} -if (is_merge && opt->diffopt.flags.follow_renames) +if (opt->diffopt.flags.follow_renames) path =xstrdup_or_null (pathspec_single_path (&opt->diffopt.pathspec)); showed_log =0 ; + all_parents = parents;for (;;) {struct commit *parent = parents->item; -/* Diff each parent with the path of the merge */ -if (path) -set_pathspec_to_single_path (&opt->diffopt.pathspec, - path);parse_commit_or_die (parent);diff_tree_oid (get_commit_tree_oid (parent), oid,"" , &opt->diffopt); -log_tree_diff_flush (opt); +if (follow_only) { +diff_queue_clear (&diff_queued_diff); + opt->diffopt.found_follow =0 ; + }else { +log_tree_diff_flush (opt); + } showed_log |= !opt->loginfo; +/* The diff updated the pathspec in case of a rename */ +if (path) +remember_follow_pathspec (opt, parent, +pathspec_single_path (&opt->diffopt.pathspec)); +/* Set up the log info for the next parent, if any.. */ parents = parents->next; -if (!parents || opt->first_parent_merges) +if (!parents)break ; +if (opt->first_parent_merges) { +if (!path || opt->first_parent_only) +break ; +/* Only find the path to follow in the other parents */ + follow_only =1 ; + } +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path); +if (follow_only) +continue ; log->parent = parents->item; opt->loginfo = log; } -/* Propagate to parents starting from the path of the merge, too */ -if (path) -set_pathspec_to_single_path (&opt->diffopt.pathspec, path); +if (follow_only && (opt->remerge_diff || opt->combine_merges)) { +/* Show the merge diff with the path of this commit */ +if (path) +set_pathspec_to_single_path (&opt->diffopt.pathspec, + path); + showed_log =log_tree_merge_diff (opt, commit, all_parents, oid); + }free (path);return showed_log; }@@ -1319,21 +1320,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)fprintf (opt->diffopt.file,"\n%s\n" , opt->break_bar);if (shown)show_diff_of_diff (opt); - -/* Record what path each parent of this commit should use */ -if (opt->diffopt.flags.follow_renames) { -struct commit_list *parents =get_saved_parents (opt, commit); -if (parents && parents->next) { -struct commit_list *p; -for (p = parents; p; p = p->next) -propagate_follow_pathspec_to_parent (opt, commit, - p->item); - }else if (parents) { -remember_follow_pathspec (opt, parents->item, -pathspec_single_path (&opt->diffopt.pathspec)); - } - } - opt->loginfo =NULL ;maybe_flush_or_die (opt->diffopt.file,"stdout" ); opt->diffopt.no_free = no_free;Signature · 1 line
2.51.0
Signature · 2 lines
Laters, Toon
PATCH v5 5/5 log: speed up --follow following renames for non-linear history
In reply to Miklos VajnaTest the performance of 'git log --stat --follow -- include/linux/rsc_table.h' on linux.git: since commit 304812ed33 (log:
improve --follow following renames for non-linear history, 2026-06-22), it takes 6m43s, while it took about 28 seconds before.
What happens is that log_tree_diff() diffs each merge against all of its parents with rename following. diff_tree_oid() only does the expensive rename detection when the path is added relative to a parent, but that is common for merges, e.g. when a branch forked before the path was created merges mainline, and this slows down 'git log --follow'.
Fix the problem by first diffing the merge against its parents without rename following, to find a parent where the path is unchanged. If there is such a parent, then skip the diff against the other parents and just follow the same path there. The exception is a parent that doesn't have the path, but still needs renames to be followed:
- The merge base has the path, i.e. that side renamed or removed it, or there is no merge base, e.g. a subtree merge of unrelated histories.
- The merge base doesn't have the path, but the parent changed a file since then, and the merge dropped that change. A copy detection also finds that file as the source of the path, i.e. the other side renamed or copied the path from that file.
If the path changed relative to all parents, then diff against them with rename following, as before. Given that this way a parent may be asked to follow a path that it doesn't have, also adjust remember_follow_pathspec(): if children disagree on what should be the path for a commit, then prefer the path that exists in that commit.
Now the above use case takes 42s, somewhat close to the original cost.
--- log-tree.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 155 insertions(+), 4 deletions(-)
log-tree.c +155 -4
@@ -31,6 +31,7 @@#include "range-diff.h" #include "strmap.h" #include "tree.h" +#include "tree-walk.h" #include "wildmatch.h" #include "write-or-die.h" #include "pager.h"
@@ -1110,6 +1111,22 @@ static void set_pathspec_to_single_path(struct pathspec *ps, const char *path) PATHSPEC_LITERAL_PATH, "", paths); } +static int tree_has_path (struct rev_info *opt,struct object_id *tree, +const char *path) +{ +struct object_id oid; +unsigned short mode; + +return !get_tree_entry (opt->diffopt.repo, tree, path, &oid, &mode); +} + +static int commit_has_path (struct rev_info *opt,struct commit *c, +const char *path) +{ +parse_commit_or_die (c); +return tree_has_path (opt,get_commit_tree_oid (c), path); +} +static void remember_follow_pathspec (struct rev_info *opt,struct commit *c,const char *path) {
@@ -1122,8 +1139,12 @@ static void remember_follow_pathspec(struct rev_info *opt,init_follow_pathspec_slab (opt->follow_pathspec_slab); } slot =follow_pathspec_slab_at (opt->follow_pathspec_slab, c); -if (*slot && !strcmp (*slot, path)) -return ; +/* If children of 'c' disagree on the path, prefer the one 'c' has. */ +if (*slot) { +if (!strcmp (*slot, path) ||commit_has_path (opt, c, *slot) || + !commit_has_path (opt, c, path)) +return ; + }free (*slot); *slot =xstrdup (path); }
@@ -1153,6 +1174,127 @@ void release_follow_pathspec_slab(struct rev_info *opt)FREE_AND_NULL (opt->follow_pathspec_slab); } +/* Find a parent where 'path' is the same as in the merge. */ +static struct commit *follow_same_parent (struct rev_info *opt, +struct commit_list *parents, +struct object_id *merge_tree) +{ +struct commit *same =NULL ; + + opt->diffopt.flags.follow_renames =0 ; +for (; parents && !same; parents = parents->next) { +parse_commit_or_die (parents->item); +diff_tree_oid (get_commit_tree_oid (parents->item), merge_tree, +"" , &opt->diffopt); +if (!diff_queued_diff.nr) + same = parents->item; +diff_queue_clear (&diff_queued_diff); + } + opt->diffopt.flags.follow_renames =1 ; +return same; +} + +/* +* Did 'parent' change a file since 'base', but the merge tree dropped that +* change? I.e. the merge doesn't have the file or it's the same as in 'same'. +*/ +static int follow_parent_changed_dropped_file (struct rev_info *opt, +struct commit *base, +struct commit *parent, +struct commit *same, +struct object_id *merge_tree) +{ +struct diff_options diff_opts; +int i, ret =0 ; + +repo_diff_setup (opt->diffopt.repo, &diff_opts); + diff_opts.flags.recursive =1 ; + diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; +diff_setup_done (&diff_opts); +diff_tree_oid (get_commit_tree_oid (base),get_commit_tree_oid (parent), +"" , &diff_opts); +for (i =0 ; i < diff_queued_diff.nr && !ret; i++) { +struct diff_filepair *p = diff_queued_diff.queue[i]; +struct object_id merge_oid, same_oid; +unsigned short mode; + +if (!DIFF_FILE_VALID (p->two)) +continue ; +if (get_tree_entry (opt->diffopt.repo, merge_tree, p->two->path, + &merge_oid, &mode)) + ret =1 ; +else + ret = !oideq (&merge_oid, &p->two->oid) && + !get_tree_entry (opt->diffopt.repo, +get_commit_tree_oid (same), + p->two->path, &same_oid, &mode) && +oideq (&merge_oid, &same_oid); + } +diff_queue_clear (&diff_queued_diff); +diff_free (&diff_opts); +return ret; +} + +/* Is 'path' in the merge tree renamed or copied from a file of 'parent'? */ +static int follow_has_rename_source (struct rev_info *opt, +struct commit *parent, +struct object_id *merge_tree, +const char *path) +{ +struct diff_options diff_opts; +int i, ret =0 ; + +repo_diff_setup (opt->diffopt.repo, &diff_opts); + diff_opts.flags.recursive =1 ; + diff_opts.detect_rename =DIFF_DETECT_COPY ; + diff_opts.rename_score = opt->diffopt.rename_score; + diff_opts.single_follow = path; + diff_opts.output_format =DIFF_FORMAT_NO_OUTPUT ; +diff_setup_done (&diff_opts); +diff_tree_oid (get_commit_tree_oid (parent), merge_tree,"" , &diff_opts); +diffcore_std (&diff_opts); +for (i =0 ; i < diff_queued_diff.nr && !ret; i++) { +struct diff_filepair *p = diff_queued_diff.queue[i]; + + ret = (p->status ==DIFF_STATUS_RENAMED || + p->status ==DIFF_STATUS_COPIED ) && + !strcmp (p->two->path, path); + } +diff_queue_clear (&diff_queued_diff); +diff_free (&diff_opts); +return ret; +} + +/* +* When the path in a merge tree is the same as in 'same', does the path still +* need renames to be followed in 'parent'? +*/ +static int follow_needs_renames (struct rev_info *opt,struct commit *parent, +struct commit *same, +struct object_id *merge_tree,const char *path) +{ +struct commit_list *bases =NULL , *base; +int ret =0 , changed_dropped =0 ; + +if (parent == same ||commit_has_path (opt, parent, path) || + !commit_has_path (opt, same, path)) +return 0 ; +if (repo_get_merge_bases (opt->diffopt.repo, parent, same, &bases) <0 ) +exit (128 ); +/* Unrelated histories, e.g. a subtree merge */ + ret = !bases; +for (base = bases; base && !ret; base = base->next) { + ret =commit_has_path (opt, base->item, path); +if (!ret && !changed_dropped) + changed_dropped =follow_parent_changed_dropped_file ( + opt, base->item, parent, same, merge_tree); + } +commit_list_free (bases); +if (!ret && changed_dropped) + ret =follow_has_rename_source (opt, parent, merge_tree, path); +return ret; +} +/* Show the remerge or combined diff of a merge commit */ static int log_tree_merge_diff(struct rev_info *opt, struct commit *commit, struct commit_list *parents,
@@ -1187,6 +1329,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct logint all_need_diff = opt->diff || opt->diffopt.flags.exit_with_status;int follow_only =0 ;char *path =NULL ; +struct commit *same =NULL ;if (!all_need_diff && !opt->merges_need_diff)return 0 ;
@@ -1234,6 +1377,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct logif (opt->diffopt.flags.follow_renames) path =xstrdup_or_null (pathspec_single_path (&opt->diffopt.pathspec)); +if (path && is_merge && (follow_only || opt->first_parent_merges)) + same =follow_same_parent (opt, parents, oid); showed_log =0 ; all_parents = parents;
@@ -1241,8 +1386,14 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct logstruct commit *parent = parents->item;parse_commit_or_die (parent); -diff_tree_oid (get_commit_tree_oid (parent), - oid,"" , &opt->diffopt); +/* +* Same as in a parent: follow the path as-is, unless renames +* still need to be followed in this parent. +*/ +if (!follow_only || !same || +follow_needs_renames (opt, parent, same, oid, path)) +diff_tree_oid (get_commit_tree_oid (parent), + oid,"" , &opt->diffopt);if (follow_only) {diff_queue_clear (&diff_queued_diff); opt->diffopt.found_follow =0 ;
Signature · 1 line
2.51.0
Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history
In reply to Miklos VajnaQuoted 12 lines · Miklos Vajna
Hi Patrick,
Thanks for the reviews of v4. Changes in v5:
- The fix of the old 6/6 is now squashed into the speedup patch (5/5), so no commit in the series loses history, and its test is moved to 1/5. While at it, I simplified 5/5: instead of two passes of diffs against the parents, it first finds a parent where the path is unchanged, and then only diffs against the other parents if they still need renames to be followed. Because of the squash, the range-diff pairs the new 5/5 with the old 6/6.
Quoted 7 lines
- 5/5 now also follows renames when a parent has no merge base with the parent where the path is unchanged, e.g. a subtree merge of unrelated histories. Added a test for this to 1/5.
- 5/5 now also follows copies, not only renames, when the merge dropped the edit of the copy source. Added your test for this to 1/5. With this, the linux.git case takes about 42s, instead of 37s.
I'm a bit late to the party, but I decided to run some benchmarks against linux.git to showcase performance with 304812ed33de (log:
improve --follow following renames for non-linear history, 2026-06-22) reverted, at the regression, and with v5 of this series applied:
Benchmark 1: git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33 reverted)
Time (mean ± σ): 23.493 s ± 0.645 s [User: 23.001 s, System: 0.389 s]
Range (min … max): 22.748 s … 23.883 s 3 runs
Benchmark 2: git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33)
Time (mean ± σ): 335.883 s ± 4.866 s [User: 333.773 s, System: 0.577 s]
Range (min … max): 332.296 s … 341.422 s 3 runs
Benchmark 3: git log --stat --follow -- include/linux/rsc_table.h (rev = v5)
Time (mean ± σ): 38.688 s ± 0.379 s [User: 38.081 s, System: 0.415 s]
Range (min … max): 38.250 s … 38.923 s 3 runs
Summary
git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33 reverted) ran
14.30 ± 0.44 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33)
1.65 ± 0.05 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = v5)While the fix in this series does improve the state of things, it is still overall a significant performance regression that I expect will result in increased file history timeouts on the server-side for some repositories we host at GitLab. Rename following for a file's history even prior to this regression can be expensive for large repositories.
While the original patch and this series do improve correctness of following a file through history, I wonder if the increased cost is ultimately worth it in most scenarios. Maybe this could be something opt-in with another flag instead of toggled on my default?
With this in mind, maybe it would be worth it to revert 304812ed33de (log: improve --follow following renames for non-linear history, 2026-06-22) and restart with a fresh series. IMO, it might make the changes here a bit easier to validate also.
Thanks,
-Justin
Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history
Quoted 49 lines · Justin Tobler
Quoted 12 lines · Miklos Vajna
Hi Patrick,
Thanks for the reviews of v4. Changes in v5:
- The fix of the old 6/6 is now squashed into the speedup patch (5/5), so no commit in the series loses history, and its test is moved to 1/5. While at it, I simplified 5/5: instead of two passes of diffs against the parents, it first finds a parent where the path is unchanged, and then only diffs against the other parents if they still need renames to be followed. Because of the squash, the range-diff pairs the new 5/5 with the old 6/6.
Quoted 7 lines
- 5/5 now also follows renames when a parent has no merge base with the parent where the path is unchanged, e.g. a subtree merge of unrelated histories. Added a test for this to 1/5.
- 5/5 now also follows copies, not only renames, when the merge dropped the edit of the copy source. Added your test for this to 1/5. With this, the linux.git case takes about 42s, instead of 37s.
I'm a bit late to the party, but I decided to run some benchmarks against linux.git to showcase performance with
304812ed33de(log:
improve --follow following renames for non-linear history, 2026-06-22) reverted, at the regression, and with v5 of this series applied:Benchmark 1: git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33 reverted) Time (mean ± σ): 23.493 s ± 0.645 s [User: 23.001 s, System: 0.389 s] Range (min … max): 22.748 s … 23.883 s 3 runs Benchmark 2: git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33) Time (mean ± σ): 335.883 s ± 4.866 s [User: 333.773 s, System: 0.577 s] Range (min … max): 332.296 s … 341.422 s 3 runs Benchmark 3: git log --stat --follow -- include/linux/rsc_table.h (rev = v5) Time (mean ± σ): 38.688 s ± 0.379 s [User: 38.081 s, System: 0.415 s] Range (min … max): 38.250 s … 38.923 s 3 runs Summary git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33 reverted) ran 14.30 ± 0.44 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = 304812ed33) 1.65 ± 0.05 times faster than git log --stat --follow -- include/linux/rsc_table.h (rev = v5)While the fix in this series does improve the state of things, it is still overall a significant performance regression that I expect will result in increased file history timeouts on the server-side for some repositories we host at GitLab. Rename following for a file's history even prior to this regression can be expensive for large repositories.
Quoted 4 lines
While the original patch and this series do improve correctness of following a file through history, I wonder if the increased cost is ultimately worth it in most scenarios. Maybe this could be something opt-in with another flag instead of toggled on my default?
Yeah, by now I'm also leaning into this direction. The performance cost is definitely way too high for us, so we need a way to disable this new feature altogether at GitLab.
Quoted 4 lines
With this in mind, maybe it would be worth it to revert
304812ed33de(log: improve --follow following renames for non-linear history, 2026-06-22) and restart with a fresh series. IMO, it might make the changes here a bit easier to validate also.
I don't have a strong sense on whether or not we should revert the whole series. I find the patches quite hard to review, but that may not be because we're fixing things incrementally and rather be because I myself am not familiar with this subsystem.
But in case we decide to not revert I'd like to see a toggle to disable this feature as a first step. As mentioned, this change has caused a rather big regression for GitLab, so we'd ideally have a way to disable this feature soon so that we can mitigate it.
I don't want to make you (Miklos) responsible for delivering that of course, I just want us to align on a fast path forward. In case we don't revert we can help out and contribute such a default-disabled toggle. And if so, that would also give us more time to fix the issue properly.
Thanks!
Patrick
Re: [PATCH v5 0/5] log: speed up --follow following renames for non-linear history
Hi Patrick,
Quoted 8 lines · Patrick Steinhardt
Quoted 4 lines
While the original patch and this series do improve correctness of following a file through history, I wonder if the increased cost is ultimately worth it in most scenarios. Maybe this could be something opt-in with another flag instead of toggled on my default?
Yeah, by now I'm also leaning into this direction. The performance cost is definitely way too high for us, so we need a way to disable this new feature altogether at GitLab.
I have no problem with turning this from default to opt-in. I would be happy to not revert, since a lot of effort went into this, and some see this as a bug in git-log, not showing some history which is there.
Quoted 9 lines
I don't have a strong sense on whether or not we should revert the whole series. I find the patches quite hard to review, but that may not be because we're fixing things incrementally and rather be because I myself am not familiar with this subsystem.
But in case we decide to not revert I'd like to see a toggle to disable this feature as a first step. As mentioned, this change has caused a rather big regression for GitLab, so we'd ideally have a way to disable this feature soon so that we can mitigate it.
Makes sense. To be honest, I'm not too happy about the complexity the last patch in the series introduces -- but having performance that is somewhat close to the old behavior seems to justify it.
Quoted 4 lines
I don't want to make you (Miklos) responsible for delivering that of course, I just want us to align on a fast path forward. In case we don't revert we can help out and contribute such a default-disabled toggle. And if so, that would also give us more time to fix the issue properly.
Yes, aligning on what we want would be good. :-) I'm OK to replace the last patch in the series with a patch that turns this off by default, but then how to opt in, would be be fine to have a new config option for this (similar to log.follow)? Would that do what you need or you want something else?
Thanks,
Miklos
Re: [PATCH] log: speed up --follow following renames for non-linear history
In reply to Miklos VajnaQuoted 22 lines · Miklos Vajna
log-tree.c +22
-0+/* Compute a path to follow in each parent of a merge */ +static void propagate_follow_pathspec_to_parents (struct rev_info *opt, +struct commit *commit, +struct commit_list *parents) +{ +const char *path =pathspec_single_path (&opt->diffopt.pathspec); +struct commit_list *p; +enum follow_path_change *changes; +int nr =commit_list_count (parents), i; +int has_same =0 ; + +if (!path) +return ; + +ALLOC_ARRAY (changes, nr); +for (p = parents, i =0 ; p; p = p->next, i++) { + changes[i] =get_follow_path_change (opt, commit, p->item); +if (changes[i] ==FOLLOW_PATH_SAME ) { + has_same =1 ; +break ; + } + }
log-tree.c static void propagate_follow_pathspec_to_parent(struct rev_info *opt, · line 1249
Quoted 10 lines
log-tree.c +10
-0+ +for (p = parents, i =0 ; p; p = p->next, i++) { +if (!has_same && changes[i] ==FOLLOW_PATH_ADDED ) +/* Never the same, added here: expensive rename detection */ +propagate_follow_pathspec_to_parent (opt, commit, p->item); +else +remember_follow_pathspec (opt, p->item, path); + } +free (changes); +}
Hmph, so if you have two branches A and B that forked from a common ancestor that had file F,
- A updates F in place
- B renames F to G in one commit
- B then updates the renamed G on in another commit
- A merges B but decides to keep the original name F and original content, deleting G.
then because has_same would be the same, you do not call propagate_follow_pathspec_to_parent() to prepare the traversal into branch B's history when you cross the merge, even when we are not simplifying the history?
That somehow sounds broken to me. Am I missing something?
Hi Patrick,
Quoted 5 lines · Patrick Steinhardt
I've bisected the regression to
304812ed33(log: improve --follow following renames for non-linear history, 2026-06-22). I don't have time right now to debug any further, so I decided to just forward the report to the mailing list. I've also Cc'd the original author of the above commit.
Thanks for the report.
To some extent, this is expected: previously log --follow had a design issue, as described at
<https://lore.kernel.org/git/[email protected]/>. Now log --follow finds more commits has it traverses non-linear history, which takes more time.
Let's see if we can find some creative solution that keeps the new design (storing / restoring the path to follow as you traverse non-linear history) and at the same time improves performance for your use-case. Quite possibly my patch is/was naive and we can avoid some not needed work while still correctly following a path across renames.
I see Gabor already commented in the thread and same some ideas, but I haven't read those yet.
Regards,
Miklos