Skip to content

Commit f239fff

Browse files
newrengitster
authored andcommitted
merge-ort: store filepairs and filespecs in our mem_pool
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: 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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a8791ef commit f239fff

File tree

3 files changed

+19
-17
lines changed

3 files changed

+19
-17
lines changed

diffcore-rename.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,7 +1334,6 @@ static void free_filespec_data(struct diff_filespec *spec)
13341334
diff_free_filespec_data(spec);
13351335
}
13361336

1337-
MAYBE_UNUSED
13381337
static void pool_free_filespec(struct mem_pool *pool,
13391338
struct diff_filespec *spec)
13401339
{
@@ -1351,7 +1350,6 @@ static void pool_free_filespec(struct mem_pool *pool,
13511350
free_filespec_data(spec);
13521351
}
13531352

1354-
MAYBE_UNUSED
13551353
void pool_diff_free_filepair(struct mem_pool *pool,
13561354
struct diff_filepair *p)
13571355
{
@@ -1370,6 +1368,7 @@ void pool_diff_free_filepair(struct mem_pool *pool,
13701368
}
13711369

13721370
void diffcore_rename_extended(struct diff_options *options,
1371+
struct mem_pool *pool,
13731372
struct strintmap *relevant_sources,
13741373
struct strintmap *dirs_removed,
13751374
struct strmap *dir_rename_count,
@@ -1683,7 +1682,7 @@ void diffcore_rename_extended(struct diff_options *options,
16831682
pair_to_free = p;
16841683

16851684
if (pair_to_free)
1686-
diff_free_filepair(pair_to_free);
1685+
pool_diff_free_filepair(pool, pair_to_free);
16871686
}
16881687
diff_debug_queue("done copying original", &outq);
16891688

@@ -1693,7 +1692,7 @@ void diffcore_rename_extended(struct diff_options *options,
16931692

16941693
for (i = 0; i < rename_dst_nr; i++)
16951694
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);
16971696

16981697
cleanup_dir_rename_info(&info, dirs_removed, dir_rename_count != NULL);
16991698
FREE_AND_NULL(rename_dst);
@@ -1710,5 +1709,5 @@ void diffcore_rename_extended(struct diff_options *options,
17101709

17111710
void diffcore_rename(struct diff_options *options)
17121711
{
1713-
diffcore_rename_extended(options, NULL, NULL, NULL, NULL);
1712+
diffcore_rename_extended(options, NULL, NULL, NULL, NULL, NULL);
17141713
}

diffcore.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ void partial_clear_dir_rename_count(struct strmap *dir_rename_count);
181181
void diffcore_break(struct repository *, int);
182182
void diffcore_rename(struct diff_options *);
183183
void diffcore_rename_extended(struct diff_options *options,
184+
struct mem_pool *pool,
184185
struct strintmap *relevant_sources,
185186
struct strintmap *dirs_removed,
186187
struct strmap *dir_rename_count,

merge-ort.c

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,6 @@ static void path_msg(struct merge_options *opt,
690690
strbuf_addch(sb, '\n');
691691
}
692692

693-
MAYBE_UNUSED
694693
static struct diff_filespec *pool_alloc_filespec(struct mem_pool *pool,
695694
const char *path)
696695
{
@@ -712,7 +711,6 @@ static struct diff_filespec *pool_alloc_filespec(struct mem_pool *pool,
712711
return spec;
713712
}
714713

715-
MAYBE_UNUSED
716714
static struct diff_filepair *pool_diff_queue(struct mem_pool *pool,
717715
struct diff_queue_struct *queue,
718716
struct diff_filespec *one,
@@ -930,6 +928,7 @@ static void add_pair(struct merge_options *opt,
930928
unsigned dir_rename_mask)
931929
{
932930
struct diff_filespec *one, *two;
931+
struct mem_pool *pool = opt->priv->pool;
933932
struct rename_info *renames = &opt->priv->renames;
934933
int names_idx = is_add ? side : 0;
935934

@@ -980,11 +979,11 @@ static void add_pair(struct merge_options *opt,
980979
return;
981980
}
982981

983-
one = alloc_filespec(pathname);
984-
two = alloc_filespec(pathname);
982+
one = pool_alloc_filespec(pool, pathname);
983+
two = pool_alloc_filespec(pool, pathname);
985984
fill_filespec(is_add ? two : one,
986985
&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);
988987
}
989988

990989
static void collect_rename_info(struct merge_options *opt,
@@ -2893,6 +2892,7 @@ static void use_cached_pairs(struct merge_options *opt,
28932892
{
28942893
struct hashmap_iter iter;
28952894
struct strmap_entry *entry;
2895+
struct mem_pool *pool = opt->priv->pool;
28962896

28972897
/*
28982898
* 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,
29062906
new_name = old_name;
29072907

29082908
/* 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);
29122912
pairs->queue[pairs->nr-1]->status = entry->value ? 'R' : 'D';
29132913
}
29142914
}
@@ -3016,6 +3016,7 @@ static int detect_regular_renames(struct merge_options *opt,
30163016
diff_queued_diff = renames->pairs[side_index];
30173017
trace2_region_enter("diff", "diffcore_rename", opt->repo);
30183018
diffcore_rename_extended(&diff_opts,
3019+
opt->priv->pool,
30193020
&renames->relevant_sources[side_index],
30203021
&renames->dirs_removed[side_index],
30213022
&renames->dir_rename_count[side_index],
@@ -3066,7 +3067,7 @@ static int collect_renames(struct merge_options *opt,
30663067

30673068
if (p->status != 'A' && p->status != 'R') {
30683069
possibly_cache_new_pair(renames, p, side_index, NULL);
3069-
diff_free_filepair(p);
3070+
pool_diff_free_filepair(opt->priv->pool, p);
30703071
continue;
30713072
}
30723073

@@ -3079,7 +3080,7 @@ static int collect_renames(struct merge_options *opt,
30793080

30803081
possibly_cache_new_pair(renames, p, side_index, new_path);
30813082
if (p->status != 'R' && !new_path) {
3082-
diff_free_filepair(p);
3083+
pool_diff_free_filepair(opt->priv->pool, p);
30833084
continue;
30843085
}
30853086

@@ -3197,7 +3198,7 @@ static int detect_and_process_renames(struct merge_options *opt,
31973198
side_pairs = &renames->pairs[s];
31983199
for (i = 0; i < side_pairs->nr; ++i) {
31993200
struct diff_filepair *p = side_pairs->queue[i];
3200-
diff_free_filepair(p);
3201+
pool_diff_free_filepair(opt->priv->pool, p);
32013202
}
32023203
}
32033204

@@ -3210,7 +3211,8 @@ static int detect_and_process_renames(struct merge_options *opt,
32103211
if (combined.nr) {
32113212
int i;
32123213
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]);
32143216
free(combined.queue);
32153217
}
32163218

0 commit comments

Comments
 (0)