merge-ort: store filepairs and filespecs in our mem_pool

For the testcases mentioned in commit 557ac0350d ("merge-ort: begin performance work; instrument with trace2_region_* calls", 2020-10-28), this change improves the performance as follows: Before After no-renames: 198.1 ms ± 2.6 ms 198.5 ms ± 3.4 ms mega-renames: 715.8 ms ± 4.0 ms 679.1 ms ± 5.6 ms just-one-mega: 276.8 ms ± 4.2 ms 271.9 ms ± 2.8 ms Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Jul 30, 2021 at 11:47 UTC f239fff4c183b231134115bc3b38b4f8e3982d3e
3 files changed +19 -17
diffcore-rename.c
+4 -5
@@ -1334,7 +1334,6 @@ static void free_filespec_data(struct diff_filespec *spec)
1334 diff_free_filespec_data(spec);
1335 }
1336
1337 -MAYBE_UNUSED
1337 static void pool_free_filespec(struct mem_pool *pool,
1338 struct diff_filespec *spec)
1339 {
@@ -1351,7 +1350,6 @@ static void pool_free_filespec(struct mem_pool *pool,
1350 free_filespec_data(spec);
1351 }
1352
1354 -MAYBE_UNUSED
1353 void pool_diff_free_filepair(struct mem_pool *pool,
1354 struct diff_filepair *p)
1355 {
@@ -1370,6 +1368,7 @@ void pool_diff_free_filepair(struct mem_pool *pool,
1368 }
1369
1370 void diffcore_rename_extended(struct diff_options *options,
1371 + struct mem_pool *pool,
1372 struct strintmap *relevant_sources,
1373 struct strintmap *dirs_removed,
1374 struct strmap *dir_rename_count,
@@ -1683,7 +1682,7 @@ void diffcore_rename_extended(struct diff_options *options,
1682 pair_to_free = p;
1683
1684 if (pair_to_free)
1686 - diff_free_filepair(pair_to_free);
1685 + pool_diff_free_filepair(pool, pair_to_free);
1686 }
1687 diff_debug_queue("done copying original", &outq);
1688
@@ -1693,7 +1692,7 @@ void diffcore_rename_extended(struct diff_options *options,
1692
1693 for (i = 0; i < rename_dst_nr; i++)
1694 if (rename_dst[i].filespec_to_free)
1696 - free_filespec(rename_dst[i].filespec_to_free);
1695 + pool_free_filespec(pool, rename_dst[i].filespec_to_free);
1696
1697 cleanup_dir_rename_info(&info, dirs_removed, dir_rename_count != NULL);
1698 FREE_AND_NULL(rename_dst);
@@ -1710,5 +1709,5 @@ void diffcore_rename_extended(struct diff_options *options,
1709
1710 void diffcore_rename(struct diff_options *options)
1711 {
1713 - diffcore_rename_extended(options, NULL, NULL, NULL, NULL);
1712 + diffcore_rename_extended(options, NULL, NULL, NULL, NULL, NULL);
1713 }
diffcore.h
+1
@@ -181,6 +181,7 @@ void partial_clear_dir_rename_count(struct strmap *dir_rename_count);
181 void diffcore_break(struct repository *, int);
182 void diffcore_rename(struct diff_options *);
183 void diffcore_rename_extended(struct diff_options *options,
184 + struct mem_pool *pool,
185 struct strintmap *relevant_sources,
186 struct strintmap *dirs_removed,
187 struct strmap *dir_rename_count,
merge-ort.c
+14 -12
@@ -690,7 +690,6 @@ static void path_msg(struct merge_options *opt,
690 strbuf_addch(sb, '\n');
691 }
692
693 -MAYBE_UNUSED
693 static struct diff_filespec *pool_alloc_filespec(struct mem_pool *pool,
694 const char *path)
695 {
@@ -712,7 +711,6 @@ static struct diff_filespec *pool_alloc_filespec(struct mem_pool *pool,
711 return spec;
712 }
713
715 -MAYBE_UNUSED
714 static struct diff_filepair *pool_diff_queue(struct mem_pool *pool,
715 struct diff_queue_struct *queue,
716 struct diff_filespec *one,
@@ -930,6 +928,7 @@ static void add_pair(struct merge_options *opt,
928 unsigned dir_rename_mask)
929 {
930 struct diff_filespec *one, *two;
931 + struct mem_pool *pool = opt->priv->pool;
932 struct rename_info *renames = &opt->priv->renames;
933 int names_idx = is_add ? side : 0;
934
@@ -980,11 +979,11 @@ static void add_pair(struct merge_options *opt,
979 return;
980 }
981
983 - one = alloc_filespec(pathname);
984 - two = alloc_filespec(pathname);
982 + one = pool_alloc_filespec(pool, pathname);
983 + two = pool_alloc_filespec(pool, pathname);
984 fill_filespec(is_add ? two : one,
985 &names[names_idx].oid, 1, names[names_idx].mode);
987 - diff_queue(&renames->pairs[side], one, two);
986 + pool_diff_queue(pool, &renames->pairs[side], one, two);
987 }
988
989 static void collect_rename_info(struct merge_options *opt,
@@ -2893,6 +2892,7 @@ static void use_cached_pairs(struct merge_options *opt,
2892 {
2893 struct hashmap_iter iter;
2894 struct strmap_entry *entry;
2895 + struct mem_pool *pool = opt->priv->pool;
2896
2897 /*
2898 * Add to side_pairs all entries from renames->cached_pairs[side_index].
@@ -2906,9 +2906,9 @@ static void use_cached_pairs(struct merge_options *opt,
2906 new_name = old_name;
2907
2908 /* We don't care about oid/mode, only filenames and status */
2909 - one = alloc_filespec(old_name);
2910 - two = alloc_filespec(new_name);
2911 - diff_queue(pairs, one, two);
2909 + one = pool_alloc_filespec(pool, old_name);
2910 + two = pool_alloc_filespec(pool, new_name);
2911 + pool_diff_queue(pool, pairs, one, two);
2912 pairs->queue[pairs->nr-1]->status = entry->value ? 'R' : 'D';
2913 }
2914 }
@@ -3016,6 +3016,7 @@ static int detect_regular_renames(struct merge_options *opt,
3016 diff_queued_diff = renames->pairs[side_index];
3017 trace2_region_enter("diff", "diffcore_rename", opt->repo);
3018 diffcore_rename_extended(&diff_opts,
3019 + opt->priv->pool,
3020 &renames->relevant_sources[side_index],
3021 &renames->dirs_removed[side_index],
3022 &renames->dir_rename_count[side_index],
@@ -3066,7 +3067,7 @@ static int collect_renames(struct merge_options *opt,
3067
3068 if (p->status != 'A' && p->status != 'R') {
3069 possibly_cache_new_pair(renames, p, side_index, NULL);
3069 - diff_free_filepair(p);
3070 + pool_diff_free_filepair(opt->priv->pool, p);
3071 continue;
3072 }
3073
@@ -3079,7 +3080,7 @@ static int collect_renames(struct merge_options *opt,
3080
3081 possibly_cache_new_pair(renames, p, side_index, new_path);
3082 if (p->status != 'R' && !new_path) {
3082 - diff_free_filepair(p);
3083 + pool_diff_free_filepair(opt->priv->pool, p);
3084 continue;
3085 }
3086
@@ -3197,7 +3198,7 @@ cleanup:
3198 side_pairs = &renames->pairs[s];
3199 for (i = 0; i < side_pairs->nr; ++i) {
3200 struct diff_filepair *p = side_pairs->queue[i];
3200 - diff_free_filepair(p);
3201 + pool_diff_free_filepair(opt->priv->pool, p);
3202 }
3203 }
3204
@@ -3210,7 +3211,8 @@ simple_cleanup:
3211 if (combined.nr) {
3212 int i;
3213 for (i = 0; i < combined.nr; i++)
3213 - diff_free_filepair(combined.queue[i]);
3214 + pool_diff_free_filepair(opt->priv->pool,
3215 + combined.queue[i]);
3216 free(combined.queue);
3217 }
3218