diffcore-rename: compute dir_rename_counts in stages
Compute dir_rename_counts based just on exact renames to start, as that can provide us useful information in find_basename_matches(). This is done by moving the code from compute_dir_rename_counts() into initialize_dir_rename_info(), resulting in it being computed earlier and based just on exact renames. Since that's an incomplete result, we augment the counts via calling update_dir_rename_counts() after each basename-guide and inexact rename detection match is found. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren committed
Feb 27, 2021 at 00:30 UTC
1ad69eb0dcfa61e175e7540517bd4e6e7a66822a
1 file changed
+70
-40
diffcore-rename.c
+70
-40
@@ -419,6 +419,28 @@ static void update_dir_rename_counts(struct dir_rename_info *info,
419
char new_dir_first_char = new_dir[0];
420
int first_time_in_loop = 1;
421
422
+ if (!info->setup)
423
+ /*
424
+ * info->setup is 0 here in two cases: (1) all auxiliary
425
+ * vars (like dirs_removed) were NULL so
426
+ * initialize_dir_rename_info() returned early, or (2)
427
+ * either break detection or copy detection are active so
428
+ * that we never called initialize_dir_rename_info(). In
429
+ * the former case, we don't have enough info to know if
430
+ * directories were renamed (because dirs_removed lets us
431
+ * know about a necessary prerequisite, namely if they were
432
+ * removed), and in the latter, we don't care about
433
+ * directory renames or find_basename_matches.
434
+ *
435
+ * This matters because both basename and inexact matching
436
+ * will also call update_dir_rename_counts(). In either of
437
+ * the above two cases info->dir_rename_counts will not
438
+ * have been properly initialized which prevents us from
439
+ * updating it, but in these two cases we don't care about
440
+ * dir_rename_counts anyway, so we can just exit early.
441
+ */
442
+ return;
443
+
444
while (1) {
445
dirname_munge(old_dir);
446
dirname_munge(new_dir);
@@ -479,45 +501,29 @@ static void update_dir_rename_counts(struct dir_rename_info *info,
501
free(new_dir);
502
}
503
482
-static void compute_dir_rename_counts(struct dir_rename_info *info,
483
- struct strset *dirs_removed,
484
- struct strmap *dir_rename_count)
504
+static void initialize_dir_rename_info(struct dir_rename_info *info,
505
+ struct strset *dirs_removed,
506
+ struct strmap *dir_rename_count)
507
{
508
int i;
509
488
- info->setup = 1;
489
- info->dir_rename_count = dir_rename_count;
490
-
491
- for (i = 0; i < rename_dst_nr; ++i) {
492
- /* File not part of directory rename counts if not a rename */
493
- if (!rename_dst[i].is_rename)
494
- continue;
495
-
496
- /*
497
- * Make dir_rename_count contain a map of a map:
498
- * old_directory -> {new_directory -> count}
499
- * In other words, for every pair look at the directories for
500
- * the old filename and the new filename and count how many
501
- * times that pairing occurs.
502
- */
503
- update_dir_rename_counts(info, dirs_removed,
504
- rename_dst[i].p->one->path,
505
- rename_dst[i].p->two->path);
510
+ if (!dirs_removed) {
511
+ info->setup = 0;
512
+ return;
513
}
507
-}
508
-
509
-static void initialize_dir_rename_info(struct dir_rename_info *info)
510
-{
511
- int i;
512
-
514
info->setup = 1;
515
516
+ info->dir_rename_count = dir_rename_count;
517
+ if (!info->dir_rename_count) {
518
+ info->dir_rename_count = xmalloc(sizeof(*dir_rename_count));
519
+ strmap_init(info->dir_rename_count);
520
+ }
521
strintmap_init_with_options(&info->idx_map, -1, NULL, 0);
522
strmap_init_with_options(&info->dir_rename_guess, NULL, 0);
517
- info->dir_rename_count = NULL;
523
524
/*
520
- * Loop setting up both info->idx_map.
525
+ * Loop setting up both info->idx_map, and doing setup of
526
+ * info->dir_rename_count.
527
*/
528
for (i = 0; i < rename_dst_nr; ++i) {
529
/*
@@ -527,7 +533,20 @@ static void initialize_dir_rename_info(struct dir_rename_info *info)
533
if (!rename_dst[i].is_rename) {
534
char *filename = rename_dst[i].p->two->path;
535
strintmap_set(&info->idx_map, filename, i);
536
+ continue;
537
}
538
+
539
+ /*
540
+ * For everything else (i.e. renamed files), make
541
+ * dir_rename_count contain a map of a map:
542
+ * old_directory -> {new_directory -> count}
543
+ * In other words, for every pair look at the directories for
544
+ * the old filename and the new filename and count how many
545
+ * times that pairing occurs.
546
+ */
547
+ update_dir_rename_counts(info, dirs_removed,
548
+ rename_dst[i].p->one->path,
549
+ rename_dst[i].p->two->path);
550
}
551
}
552
@@ -682,7 +701,8 @@ static int idx_possible_rename(char *filename, struct dir_rename_info *info)
701
702
static int find_basename_matches(struct diff_options *options,
703
int minimum_score,
685
- struct dir_rename_info *info)
704
+ struct dir_rename_info *info,
705
+ struct strset *dirs_removed)
706
{
707
/*
708
* When I checked in early 2020, over 76% of file renames in linux
@@ -810,6 +830,8 @@ static int find_basename_matches(struct diff_options *options,
830
continue;
831
record_rename_pair(dst_index, src_index, score);
832
renames++;
833
+ update_dir_rename_counts(info, dirs_removed,
834
+ one->path, two->path);
835
836
/*
837
* Found a rename so don't need text anymore; if we
@@ -893,7 +915,12 @@ static int too_many_rename_candidates(int num_destinations, int num_sources,
915
return 1;
916
}
917
896
-static int find_renames(struct diff_score *mx, int dst_cnt, int minimum_score, int copies)
918
+static int find_renames(struct diff_score *mx,
919
+ int dst_cnt,
920
+ int minimum_score,
921
+ int copies,
922
+ struct dir_rename_info *info,
923
+ struct strset *dirs_removed)
924
{
925
int count = 0, i;
926
@@ -910,6 +937,9 @@ static int find_renames(struct diff_score *mx, int dst_cnt, int minimum_score, i
937
continue;
938
record_rename_pair(mx[i].dst, mx[i].src, mx[i].score);
939
count++;
940
+ update_dir_rename_counts(info, dirs_removed,
941
+ rename_src[mx[i].src].p->one->path,
942
+ rename_dst[mx[i].dst].p->two->path);
943
}
944
return count;
945
}
@@ -981,6 +1011,8 @@ void diffcore_rename_extended(struct diff_options *options,
1011
info.setup = 0;
1012
assert(!dir_rename_count || strmap_empty(dir_rename_count));
1013
want_copies = (detect_rename == DIFF_DETECT_COPY);
1014
+ if (dirs_removed && (break_idx || want_copies))
1015
+ BUG("dirs_removed incompatible with break/copy detection");
1016
if (!minimum_score)
1017
minimum_score = DEFAULT_RENAME_SCORE;
1018
@@ -1074,14 +1106,15 @@ void diffcore_rename_extended(struct diff_options *options,
1106
1107
/* Preparation for basename-driven matching. */
1108
trace2_region_enter("diff", "dir rename setup", options->repo);
1077
- initialize_dir_rename_info(&info);
1109
+ initialize_dir_rename_info(&info,
1110
+ dirs_removed, dir_rename_count);
1111
trace2_region_leave("diff", "dir rename setup", options->repo);
1112
1113
/* Utilize file basenames to quickly find renames. */
1114
trace2_region_enter("diff", "basename matches", options->repo);
1115
rename_count += find_basename_matches(options,
1116
min_basename_score,
1084
- &info);
1117
+ &info, dirs_removed);
1118
trace2_region_leave("diff", "basename matches", options->repo);
1119
1120
/*
@@ -1167,18 +1200,15 @@ void diffcore_rename_extended(struct diff_options *options,
1200
/* cost matrix sorted by most to least similar pair */
1201
STABLE_QSORT(mx, dst_cnt * NUM_CANDIDATE_PER_DST, score_compare);
1202
1170
- rename_count += find_renames(mx, dst_cnt, minimum_score, 0);
1203
+ rename_count += find_renames(mx, dst_cnt, minimum_score, 0,
1204
+ &info, dirs_removed);
1205
if (want_copies)
1172
- rename_count += find_renames(mx, dst_cnt, minimum_score, 1);
1206
+ rename_count += find_renames(mx, dst_cnt, minimum_score, 1,
1207
+ &info, dirs_removed);
1208
free(mx);
1209
trace2_region_leave("diff", "inexact renames", options->repo);
1210
1211
cleanup:
1177
- /*
1178
- * Now that renames have been computed, compute dir_rename_count */
1179
- if (dirs_removed && dir_rename_count)
1180
- compute_dir_rename_counts(&info, dirs_removed, dir_rename_count);
1181
-
1212
/* At this point, we have found some renames and copies and they
1213
* are recorded in rename_dst. The original list is still in *q.
1214
*/