diffcore-rename: allow different missing_object_cb functions
estimate_similarity() was setting up a diff_populate_filespec_options every time it was called, requiring the caller of estimate_similarity() to pass in some data needed to set up this option. Currently the needed data consisted of a single variable (skip_unmodified), but we want to also have the different estimate_similarity() callsites start using different missing_object_cb functions as well. Rather than also passing that data in, just have the caller pass in the whole diff_populate_filespec_options, and reduce the number of times we need to set it up. As a side note, this also drops the number of calls to has_promisor_remote() dramatically. If L is the number of basename paths to compare, M is the number of inexact sources, and N is the number of inexact destinations, then the number of calls to has_promisor_remote() drops from L+M*N down to at most 2 -- one for each of the sites that calls estimate_similarity(). has_promisor_remote() is a very fast function so this almost certainly has no measurable performance impact, but it seems cleaner to avoid calling that function so many times. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren committed
Jun 22, 2021 at 08:04 UTC
d331dd3b0c829fe9019f0113a095ed95bc06f227
1 file changed
+39
-19
diffcore-rename.c
+39
-19
@@ -126,7 +126,7 @@ static int estimate_similarity(struct repository *r,
126
struct diff_filespec *src,
127
struct diff_filespec *dst,
128
int minimum_score,
129
- int skip_unmodified)
129
+ struct diff_populate_filespec_options *dpf_opt)
130
{
131
/* src points at a file that existed in the original tree (or
132
* optionally a file in the destination tree) and dst points
@@ -143,15 +143,6 @@ static int estimate_similarity(struct repository *r,
143
*/
144
unsigned long max_size, delta_size, base_size, src_copied, literal_added;
145
int score;
146
- struct diff_populate_filespec_options dpf_options = {
147
- .check_size_only = 1
148
- };
149
- struct prefetch_options prefetch_options = {r, skip_unmodified};
150
-
151
- if (r == the_repository && has_promisor_remote()) {
152
- dpf_options.missing_object_cb = prefetch;
153
- dpf_options.missing_object_data = &prefetch_options;
154
- }
146
147
/* We deal only with regular files. Symlink renames are handled
148
* only when they are exact matches --- in other words, no edits
@@ -169,11 +160,13 @@ static int estimate_similarity(struct repository *r,
160
* is a possible size - we really should have a flag to
161
* say whether the size is valid or not!)
162
*/
163
+ dpf_opt->check_size_only = 1;
164
+
165
if (!src->cnt_data &&
173
- diff_populate_filespec(r, src, &dpf_options))
166
+ diff_populate_filespec(r, src, dpf_opt))
167
return 0;
168
if (!dst->cnt_data &&
176
- diff_populate_filespec(r, dst, &dpf_options))
169
+ diff_populate_filespec(r, dst, dpf_opt))
170
return 0;
171
172
max_size = ((src->size > dst->size) ? src->size : dst->size);
@@ -191,11 +184,11 @@ static int estimate_similarity(struct repository *r,
184
if (max_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE)
185
return 0;
186
194
- dpf_options.check_size_only = 0;
187
+ dpf_opt->check_size_only = 0;
188
196
- if (!src->cnt_data && diff_populate_filespec(r, src, &dpf_options))
189
+ if (!src->cnt_data && diff_populate_filespec(r, src, dpf_opt))
190
return 0;
198
- if (!dst->cnt_data && diff_populate_filespec(r, dst, &dpf_options))
191
+ if (!dst->cnt_data && diff_populate_filespec(r, dst, dpf_opt))
192
return 0;
193
194
if (diffcore_count_changes(r, src, dst,
@@ -862,7 +855,11 @@ static int find_basename_matches(struct diff_options *options,
855
int i, renames = 0;
856
struct strintmap sources;
857
struct strintmap dests;
865
-
858
+ struct diff_populate_filespec_options dpf_options = {
859
+ .check_binary = 0,
860
+ .missing_object_cb = NULL,
861
+ .missing_object_data = NULL
862
+ };
863
/*
864
* The prefeteching stuff wants to know if it can skip prefetching
865
* blobs that are unmodified...and will then do a little extra work
@@ -873,7 +870,10 @@ static int find_basename_matches(struct diff_options *options,
870
* the extra work necessary to check if rename_src entries are
871
* unmodified would be a small waste.
872
*/
876
- int skip_unmodified = 0;
873
+ struct prefetch_options prefetch_options = {
874
+ .repo = options->repo,
875
+ .skip_unmodified = 0
876
+ };
877
878
/*
879
* Create maps of basename -> fullname(s) for remaining sources and
@@ -910,6 +910,11 @@ static int find_basename_matches(struct diff_options *options,
910
strintmap_set(&dests, base, i);
911
}
912
913
+ if (options->repo == the_repository && has_promisor_remote()) {
914
+ dpf_options.missing_object_cb = prefetch;
915
+ dpf_options.missing_object_data = &prefetch_options;
916
+ }
917
+
918
/* Now look for basename matchups and do similarity estimation */
919
for (i = 0; i < rename_src_nr; ++i) {
920
char *filename = rename_src[i].p->one->path;
@@ -953,7 +958,7 @@ static int find_basename_matches(struct diff_options *options,
958
one = rename_src[src_index].p->one;
959
two = rename_dst[dst_index].p->two;
960
score = estimate_similarity(options->repo, one, two,
956
- minimum_score, skip_unmodified);
961
+ minimum_score, &dpf_options);
962
963
/* If sufficiently similar, record as rename pair */
964
if (score < minimum_score)
@@ -1272,6 +1277,14 @@ void diffcore_rename_extended(struct diff_options *options,
1277
int num_sources, want_copies;
1278
struct progress *progress = NULL;
1279
struct dir_rename_info info;
1280
+ struct diff_populate_filespec_options dpf_options = {
1281
+ .check_binary = 0,
1282
+ .missing_object_cb = NULL,
1283
+ .missing_object_data = NULL
1284
+ };
1285
+ struct prefetch_options prefetch_options = {
1286
+ .repo = options->repo
1287
+ };
1288
1289
trace2_region_enter("diff", "setup", options->repo);
1290
info.setup = 0;
@@ -1433,6 +1446,13 @@ void diffcore_rename_extended(struct diff_options *options,
1446
(uint64_t)num_destinations * (uint64_t)num_sources);
1447
}
1448
1449
+ /* Finish setting up dpf_options */
1450
+ prefetch_options.skip_unmodified = skip_unmodified;
1451
+ if (options->repo == the_repository && has_promisor_remote()) {
1452
+ dpf_options.missing_object_cb = prefetch;
1453
+ dpf_options.missing_object_data = &prefetch_options;
1454
+ }
1455
+
1456
CALLOC_ARRAY(mx, st_mult(NUM_CANDIDATE_PER_DST, num_destinations));
1457
for (dst_cnt = i = 0; i < rename_dst_nr; i++) {
1458
struct diff_filespec *two = rename_dst[i].p->two;
@@ -1458,7 +1478,7 @@ void diffcore_rename_extended(struct diff_options *options,
1478
this_src.score = estimate_similarity(options->repo,
1479
one, two,
1480
minimum_score,
1461
- skip_unmodified);
1481
+ &dpf_options);
1482
this_src.name_score = basename_same(one, two);
1483
this_src.dst = i;
1484
this_src.src = j;