Skip to content

Commit

Permalink
merge-ort: skip rename detection entirely if possible
Browse files Browse the repository at this point in the history
diffcore_rename_extended() will do a bunch of setup, then check for
exact renames, then abort before inexact rename detection if there are
no more sources or destinations that need to be matched.  It will
sometimes be the case, however, that either
  * we start with neither any sources or destinations
  * we start with no *relevant* sources
In the first of these two cases, the setup and exact rename detection
will be very cheap since there are 0 files to operate on.  In the second
case, it is quite possible to have thousands of files with none of the
source ones being relevant.  Avoid calling diffcore_rename_extended() or
even some of the setup before diffcore_rename_extended() when we can
determine that rename detection is unnecessary.

For the testcases mentioned in commit 557ac03 ("merge-ort: begin
performance work; instrument with trace2_region_* calls", 2020-10-28),
this change improves the performance as follows:

                            Before                  After
    no-renames:        6.003 s ±  0.048 s     5.708 s ±  0.111 s
    mega-renames:    114.009 s ±  0.236 s   102.171 s ±  0.440 s
    just-one-mega:     3.489 s ±  0.017 s     3.471 s ±  0.015 s

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
newren authored and gitster committed Mar 11, 2021
1 parent 174791f commit f89b4f2
Showing 1 changed file with 44 additions and 0 deletions.
44 changes: 44 additions & 0 deletions merge-ort.c
Original file line number Diff line number Diff line change
Expand Up @@ -2157,6 +2157,19 @@ static int process_renames(struct merge_options *opt,
return clean_merge;
}

static inline int possible_side_renames(struct rename_info *renames,
unsigned side_index)
{
return renames->pairs[side_index].nr > 0 &&
!strset_empty(&renames->relevant_sources[side_index]);
}

static inline int possible_renames(struct rename_info *renames)
{
return possible_side_renames(renames, 1) ||
possible_side_renames(renames, 2);
}

static void resolve_diffpair_statuses(struct diff_queue_struct *q)
{
/*
Expand Down Expand Up @@ -2193,6 +2206,16 @@ static void detect_regular_renames(struct merge_options *opt,
struct diff_options diff_opts;
struct rename_info *renames = &opt->priv->renames;

if (!possible_side_renames(renames, side_index)) {
/*
* No rename detection needed for this side, but we still need
* to make sure 'adds' are marked correctly in case the other
* side had directory renames.
*/
resolve_diffpair_statuses(&renames->pairs[side_index]);
return;
}

repo_diff_setup(opt->repo, &diff_opts);
diff_opts.flags.recursive = 1;
diff_opts.flags.rename_empty = 0;
Expand Down Expand Up @@ -2310,6 +2333,8 @@ static int detect_and_process_renames(struct merge_options *opt,
int need_dir_renames, s, clean = 1;

memset(&combined, 0, sizeof(combined));
if (!possible_renames(renames))
goto cleanup;

trace2_region_enter("merge", "regular renames", opt->repo);
detect_regular_renames(opt, MERGE_SIDE1);
Expand Down Expand Up @@ -2344,6 +2369,25 @@ static int detect_and_process_renames(struct merge_options *opt,
clean &= process_renames(opt, &combined);
trace2_region_leave("merge", "process renames", opt->repo);

goto simple_cleanup; /* collect_renames() handles some of cleanup */

cleanup:
/*
* Free now unneeded filepairs, which would have been handled
* in collect_renames() normally but we skipped that code.
*/
for (s = MERGE_SIDE1; s <= MERGE_SIDE2; s++) {
struct diff_queue_struct *side_pairs;
int i;

side_pairs = &renames->pairs[s];
for (i = 0; i < side_pairs->nr; ++i) {
struct diff_filepair *p = side_pairs->queue[i];
diff_free_filepair(p);
}
}

simple_cleanup:
/* Free memory for renames->pairs[] and combined */
for (s = MERGE_SIDE1; s <= MERGE_SIDE2; s++) {
free(renames->pairs[s].queue);
Expand Down

0 comments on commit f89b4f2

Please sign in to comment.