Skip to content

Commit a3540ed

Browse files
szedergitster
authored andcommitted
line-log: avoid unnecessary tree diffs when processing merge commits
In process_ranges_merge_commit(), the line-level log first creates an array of diff queues by iterating over all parents of a merge commit and computing a tree diff for each. Then in a second loop it iterates over those diff queues, and if it finds that none of the interesting paths were modified in one of them, then it will return early. This means that when none of the interesting paths were modified between a merge and its first parent, then the tree diff between the merge and its second (Nth...) parent was computed in vain. Unify these two loops, so when it iterates over all parents of a merge commit, then it first computes the tree diff between the merge and that particular parent and then processes the resulting diff queue right away. This way we can spare some tree diff computing, thereby speeding up line-level log in repositories with mergy history: # git.git, 25.8% of commits are merges: Benchmark 1: ./git_v2.51.0 -C ~/src/git log -L:'lookup_commit(':commit.c v2.51.0 Time (mean ± σ): 1.001 s ± 0.009 s [User: 0.906 s, System: 0.095 s] Range (min … max): 0.991 s … 1.023 s 10 runs Benchmark 2: ./git -C ~/src/git log -L:'lookup_commit(':commit.c v2.51.0 Time (mean ± σ): 445.5 ms ± 3.4 ms [User: 358.8 ms, System: 84.3 ms] Range (min … max): 440.1 ms … 450.3 ms 10 runs Summary './git -C ~/src/git log -L:'lookup_commit(':commit.c v2.51.0' ran 2.25 ± 0.03 times faster than './git_v2.51.0 -C ~/src/git log -L:'lookup_commit(':commit.c v2.51.0' # linux.git, 7.5% of commits are merges: Benchmark 1: ./git_v2.51.0 -C ~/src/linux.git log -L:build_restore_work_registers:arch/mips/mm/tlbex.c v6.16 Time (mean ± σ): 3.246 s ± 0.007 s [User: 2.835 s, System: 0.409 s] Range (min … max): 3.232 s … 3.255 s 10 runs Benchmark 2: ./git -C ~/src/linux.git log -L:build_restore_work_registers:arch/mips/mm/tlbex.c v6.16 Time (mean ± σ): 2.467 s ± 0.014 s [User: 2.113 s, System: 0.353 s] Range (min … max): 2.455 s … 2.505 s 10 runs Summary './git -C ~/src/linux.git log -L:build_restore_work_registers:arch/mips/mm/tlbex.c v6.16' ran 1.32 ± 0.01 times faster than './git_v2.51.0 -C ~/src/linux.git log -L:build_restore_work_registers:arch/mips/mm/tlbex.c v6.16' And since now each iteration computes a tree diff and processes its result, there is no reason to store the diff queues for each merge parent anymore, so replace that diff queue array with a loop-local diff queue variable. With this change the static free_diffqueues() helper function in 'line-log.c' has no more callers left, remove it. Signed-off-by: SZEDER Gábor <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent c44beea commit a3540ed

File tree

1 file changed

+5
-15
lines changed

1 file changed

+5
-15
lines changed

‎line-log.c‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,13 +1087,6 @@ static struct diff_filepair *diff_filepair_dup(struct diff_filepair *pair)
10871087
return new_filepair;
10881088
}
10891089

1090-
static void free_diffqueues(int n, struct diff_queue_struct *dq)
1091-
{
1092-
for (int i = 0; i < n; i++)
1093-
diff_queue_clear(&dq[i]);
1094-
free(dq);
1095-
}
1096-
10971090
static int process_all_files(struct line_log_data **range_out,
10981091
struct rev_info *rev,
10991092
struct diff_queue_struct *queue,
@@ -1209,7 +1202,6 @@ static int process_ranges_ordinary_commit(struct rev_info *rev, struct commit *c
12091202
static int process_ranges_merge_commit(struct rev_info *rev, struct commit *commit,
12101203
struct line_log_data *range)
12111204
{
1212-
struct diff_queue_struct *diffqueues;
12131205
struct line_log_data **cand;
12141206
struct commit **parents;
12151207
struct commit_list *p;
@@ -1220,20 +1212,19 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm
12201212
if (nparents > 1 && rev->first_parent_only)
12211213
nparents = 1;
12221214

1223-
ALLOC_ARRAY(diffqueues, nparents);
12241215
CALLOC_ARRAY(cand, nparents);
12251216
ALLOC_ARRAY(parents, nparents);
12261217

12271218
p = commit->parents;
12281219
for (i = 0; i < nparents; i++) {
1220+
struct diff_queue_struct diffqueue = DIFF_QUEUE_INIT;
1221+
int changed;
12291222
parents[i] = p->item;
12301223
p = p->next;
1231-
queue_diffs(range, &rev->diffopt, &diffqueues[i], commit, parents[i]);
1232-
}
1224+
queue_diffs(range, &rev->diffopt, &diffqueue, commit, parents[i]);
12331225

1234-
for (i = 0; i < nparents; i++) {
1235-
int changed;
1236-
changed = process_all_files(&cand[i], rev, &diffqueues[i], range);
1226+
changed = process_all_files(&cand[i], rev, &diffqueue, range);
1227+
diff_queue_clear(&diffqueue);
12371228
if (!changed) {
12381229
/*
12391230
* This parent can take all the blame, so we
@@ -1267,7 +1258,6 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm
12671258
free(cand[i]);
12681259
}
12691260
free(cand);
1270-
free_diffqueues(nparents, diffqueues);
12711261
return ret;
12721262

12731263
/* NEEDSWORK evil merge detection stuff */

0 commit comments

Comments
 (0)