Skip to content

Commit d3a035b

Browse files
gitstergitster
committed
Merge branch 'en/merge-ort-perf'
The "ort" merge strategy. * en/merge-ort-perf: merge-ort: begin performance work; instrument with trace2_region_* calls merge-ort: ignore the directory rename split conflict for now merge-ort: fix massive leak
2 parents a21e27e + 557ac03 commit d3a035b

File tree

2 files changed

+94
-1
lines changed

2 files changed

+94
-1
lines changed

‎diffcore-rename.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,7 @@ void diffcore_rename(struct diff_options *options)
465465
int num_destinations, dst_cnt;
466466
struct progress *progress = NULL;
467467

468+
trace2_region_enter("diff", "setup", options->repo);
468469
if (!minimum_score)
469470
minimum_score = DEFAULT_RENAME_SCORE;
470471

@@ -510,14 +511,17 @@ void diffcore_rename(struct diff_options *options)
510511
register_rename_src(p);
511512
}
512513
}
514+
trace2_region_leave("diff", "setup", options->repo);
513515
if (rename_dst_nr == 0 || rename_src_nr == 0)
514516
goto cleanup; /* nothing to do */
515517

518+
trace2_region_enter("diff", "exact renames", options->repo);
516519
/*
517520
* We really want to cull the candidates list early
518521
* with cheap tests in order to avoid doing deltas.
519522
*/
520523
rename_count = find_exact_renames(options);
524+
trace2_region_leave("diff", "exact renames", options->repo);
521525

522526
/* Did we only want exact renames? */
523527
if (minimum_score == MAX_SCORE)
@@ -545,6 +549,7 @@ void diffcore_rename(struct diff_options *options)
545549
break;
546550
}
547551

552+
trace2_region_enter("diff", "inexact renames", options->repo);
548553
if (options->show_rename_progress) {
549554
progress = start_delayed_progress(
550555
_("Performing inexact rename detection"),
@@ -600,11 +605,13 @@ void diffcore_rename(struct diff_options *options)
600605
if (detect_rename == DIFF_DETECT_COPY)
601606
rename_count += find_renames(mx, dst_cnt, minimum_score, 1);
602607
free(mx);
608+
trace2_region_leave("diff", "inexact renames", options->repo);
603609

604610
cleanup:
605611
/* At this point, we have found some renames and copies and they
606612
* are recorded in rename_dst. The original list is still in *q.
607613
*/
614+
trace2_region_enter("diff", "write back to queue", options->repo);
608615
DIFF_QUEUE_CLEAR(&outq);
609616
for (i = 0; i < q->nr; i++) {
610617
struct diff_filepair *p = q->queue[i];
@@ -680,5 +687,6 @@ void diffcore_rename(struct diff_options *options)
680687
strintmap_clear(break_idx);
681688
FREE_AND_NULL(break_idx);
682689
}
690+
trace2_region_leave("diff", "write back to queue", options->repo);
683691
return;
684692
}

‎merge-ort.c‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,9 @@ static int collect_merge_info(struct merge_options *opt,
752752
init_tree_desc(t + 1, side1->buffer, side1->size);
753753
init_tree_desc(t + 2, side2->buffer, side2->size);
754754

755+
trace2_region_enter("merge", "traverse_trees", opt->repo);
755756
ret = traverse_trees(NULL, 3, t, &info);
757+
trace2_region_leave("merge", "traverse_trees", opt->repo);
756758

757759
return ret;
758760
}
@@ -1439,7 +1441,18 @@ static void get_provisional_directory_renames(struct merge_options *opt,
14391441
"no destination getting a majority of the "
14401442
"files."),
14411443
source_dir);
1442-
*clean = 0;
1444+
/*
1445+
* We should mark this as unclean IF something attempts
1446+
* to use this rename. We do not yet have the logic
1447+
* in place to detect if this directory rename is being
1448+
* used, and optimizations that reduce the number of
1449+
* renames cause this to falsely trigger. For now,
1450+
* just disable it, causing t6423 testcase 2a to break.
1451+
* We'll later fix the detection, and when we do we
1452+
* will re-enable setting *clean to 0 (and thereby fix
1453+
* t6423 testcase 2a).
1454+
*/
1455+
/* *clean = 0; */
14431456
} else {
14441457
strmap_put(&renames->dir_renames[side],
14451458
source_dir, (void*)best);
@@ -2094,9 +2107,12 @@ static void detect_regular_renames(struct merge_options *opt,
20942107
diff_opts.show_rename_progress = opt->show_rename_progress;
20952108
diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
20962109
diff_setup_done(&diff_opts);
2110+
2111+
trace2_region_enter("diff", "diffcore_rename", opt->repo);
20972112
diff_tree_oid(&merge_base->object.oid, &side->object.oid, "",
20982113
&diff_opts);
20992114
diffcore_std(&diff_opts);
2115+
trace2_region_leave("diff", "diffcore_rename", opt->repo);
21002116

21012117
if (diff_opts.needed_rename_limit > renames->needed_limit)
21022118
renames->needed_limit = diff_opts.needed_rename_limit;
@@ -2195,9 +2211,12 @@ static int detect_and_process_renames(struct merge_options *opt,
21952211

21962212
memset(&combined, 0, sizeof(combined));
21972213

2214+
trace2_region_enter("merge", "regular renames", opt->repo);
21982215
detect_regular_renames(opt, merge_base, side1, MERGE_SIDE1);
21992216
detect_regular_renames(opt, merge_base, side2, MERGE_SIDE2);
2217+
trace2_region_leave("merge", "regular renames", opt->repo);
22002218

2219+
trace2_region_enter("merge", "directory renames", opt->repo);
22012220
need_dir_renames =
22022221
!opt->priv->call_depth &&
22032222
(opt->detect_directory_renames == MERGE_DIRECTORY_RENAMES_TRUE ||
@@ -2219,8 +2238,11 @@ static int detect_and_process_renames(struct merge_options *opt,
22192238
&renames->dir_renames[1],
22202239
&renames->dir_renames[2]);
22212240
QSORT(combined.queue, combined.nr, compare_pairs);
2241+
trace2_region_leave("merge", "directory renames", opt->repo);
22222242

2243+
trace2_region_enter("merge", "process renames", opt->repo);
22232244
clean &= process_renames(opt, &combined);
2245+
trace2_region_leave("merge", "process renames", opt->repo);
22242246

22252247
/* Free memory for renames->pairs[] and combined */
22262248
for (s = MERGE_SIDE1; s <= MERGE_SIDE2; s++) {
@@ -2902,20 +2924,30 @@ static void process_entries(struct merge_options *opt,
29022924
STRING_LIST_INIT_NODUP,
29032925
NULL, 0 };
29042926

2927+
trace2_region_enter("merge", "process_entries setup", opt->repo);
29052928
if (strmap_empty(&opt->priv->paths)) {
29062929
oidcpy(result_oid, opt->repo->hash_algo->empty_tree);
29072930
return;
29082931
}
29092932

29102933
/* Hack to pre-allocate plist to the desired size */
2934+
trace2_region_enter("merge", "plist grow", opt->repo);
29112935
ALLOC_GROW(plist.items, strmap_get_size(&opt->priv->paths), plist.alloc);
2936+
trace2_region_leave("merge", "plist grow", opt->repo);
29122937

29132938
/* Put every entry from paths into plist, then sort */
2939+
trace2_region_enter("merge", "plist copy", opt->repo);
29142940
strmap_for_each_entry(&opt->priv->paths, &iter, e) {
29152941
string_list_append(&plist, e->key)->util = e->value;
29162942
}
2943+
trace2_region_leave("merge", "plist copy", opt->repo);
2944+
2945+
trace2_region_enter("merge", "plist special sort", opt->repo);
29172946
plist.cmp = string_list_df_name_compare;
29182947
string_list_sort(&plist);
2948+
trace2_region_leave("merge", "plist special sort", opt->repo);
2949+
2950+
trace2_region_leave("merge", "process_entries setup", opt->repo);
29192951

29202952
/*
29212953
* Iterate over the items in reverse order, so we can handle paths
@@ -2926,6 +2958,7 @@ static void process_entries(struct merge_options *opt,
29262958
* (because it allows us to know whether the directory is still in
29272959
* the way when it is time to process the file at the same path).
29282960
*/
2961+
trace2_region_enter("merge", "processing", opt->repo);
29292962
for (entry = &plist.items[plist.nr-1]; entry >= plist.items; --entry) {
29302963
char *path = entry->string;
29312964
/*
@@ -2944,7 +2977,9 @@ static void process_entries(struct merge_options *opt,
29442977
process_entry(opt, path, ci, &dir_metadata);
29452978
}
29462979
}
2980+
trace2_region_leave("merge", "processing", opt->repo);
29472981

2982+
trace2_region_enter("merge", "process_entries cleanup", opt->repo);
29482983
if (dir_metadata.offsets.nr != 1 ||
29492984
(uintptr_t)dir_metadata.offsets.items[0].util != 0) {
29502985
printf("dir_metadata.offsets.nr = %d (should be 1)\n",
@@ -2959,6 +2994,7 @@ static void process_entries(struct merge_options *opt,
29592994
string_list_clear(&plist, 0);
29602995
string_list_clear(&dir_metadata.versions, 0);
29612996
string_list_clear(&dir_metadata.offsets, 0);
2997+
trace2_region_leave("merge", "process_entries cleanup", opt->repo);
29622998
}
29632999

29643000
/*** Function Grouping: functions related to merge_switch_to_result() ***/
@@ -3117,19 +3153,23 @@ void merge_switch_to_result(struct merge_options *opt,
31173153
if (result->clean >= 0 && update_worktree_and_index) {
31183154
struct merge_options_internal *opti = result->priv;
31193155

3156+
trace2_region_enter("merge", "checkout", opt->repo);
31203157
if (checkout(opt, head, result->tree)) {
31213158
/* failure to function */
31223159
result->clean = -1;
31233160
return;
31243161
}
3162+
trace2_region_leave("merge", "checkout", opt->repo);
31253163

3164+
trace2_region_enter("merge", "record_conflicted", opt->repo);
31263165
if (record_conflicted_index_entries(opt, opt->repo->index,
31273166
&opti->paths,
31283167
&opti->conflicted)) {
31293168
/* failure to function */
31303169
result->clean = -1;
31313170
return;
31323171
}
3172+
trace2_region_leave("merge", "record_conflicted", opt->repo);
31333173
}
31343174

31353175
if (display_update_msgs) {
@@ -3139,6 +3179,8 @@ void merge_switch_to_result(struct merge_options *opt,
31393179
struct string_list olist = STRING_LIST_INIT_NODUP;
31403180
int i;
31413181

3182+
trace2_region_enter("merge", "display messages", opt->repo);
3183+
31423184
/* Hack to pre-allocate olist to the desired size */
31433185
ALLOC_GROW(olist.items, strmap_get_size(&opti->output),
31443186
olist.alloc);
@@ -3160,6 +3202,8 @@ void merge_switch_to_result(struct merge_options *opt,
31603202
/* Also include needed rename limit adjustment now */
31613203
diff_warn_rename_limit("merge.renamelimit",
31623204
opti->renames.needed_limit, 0);
3205+
3206+
trace2_region_leave("merge", "display messages", opt->repo);
31633207
}
31643208

31653209
merge_finalize(opt, result);
@@ -3201,6 +3245,7 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
32013245
int i;
32023246

32033247
/* Sanity checks on opt */
3248+
trace2_region_enter("merge", "sanity checks", opt->repo);
32043249
assert(opt->repo);
32053250

32063251
assert(opt->branch1 && opt->branch2);
@@ -3227,11 +3272,30 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
32273272
assert(opt->obuf.len == 0);
32283273

32293274
assert(opt->priv == NULL);
3275+
if (result->priv) {
3276+
opt->priv = result->priv;
3277+
result->priv = NULL;
3278+
/*
3279+
* opt->priv non-NULL means we had results from a previous
3280+
* run; do a few sanity checks that user didn't mess with
3281+
* it in an obvious fashion.
3282+
*/
3283+
assert(opt->priv->call_depth == 0);
3284+
assert(!opt->priv->toplevel_dir ||
3285+
0 == strlen(opt->priv->toplevel_dir));
3286+
}
3287+
trace2_region_leave("merge", "sanity checks", opt->repo);
32303288

32313289
/* Default to histogram diff. Actually, just hardcode it...for now. */
32323290
opt->xdl_opts = DIFF_WITH_ALG(opt, HISTOGRAM_DIFF);
32333291

32343292
/* Initialization of opt->priv, our internal merge data */
3293+
trace2_region_enter("merge", "allocate/init", opt->repo);
3294+
if (opt->priv) {
3295+
clear_or_reinit_internal_opts(opt->priv, 1);
3296+
trace2_region_leave("merge", "allocate/init", opt->repo);
3297+
return;
3298+
}
32353299
opt->priv = xcalloc(1, sizeof(*opt->priv));
32363300

32373301
/* Initialization of various renames fields */
@@ -3264,6 +3328,8 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
32643328
* subset of the overall paths that have special output.
32653329
*/
32663330
strmap_init(&opt->priv->output);
3331+
3332+
trace2_region_leave("merge", "allocate/init", opt->repo);
32673333
}
32683334

32693335
/*** Function Grouping: merge_incore_*() and their internal variants ***/
@@ -3279,6 +3345,7 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt,
32793345
{
32803346
struct object_id working_tree_oid;
32813347

3348+
trace2_region_enter("merge", "collect_merge_info", opt->repo);
32823349
if (collect_merge_info(opt, merge_base, side1, side2) != 0) {
32833350
/*
32843351
* TRANSLATORS: The %s arguments are: 1) tree hash of a merge
@@ -3291,10 +3358,16 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt,
32913358
result->clean = -1;
32923359
return;
32933360
}
3361+
trace2_region_leave("merge", "collect_merge_info", opt->repo);
32943362

3363+
trace2_region_enter("merge", "renames", opt->repo);
32953364
result->clean = detect_and_process_renames(opt, merge_base,
32963365
side1, side2);
3366+
trace2_region_leave("merge", "renames", opt->repo);
3367+
3368+
trace2_region_enter("merge", "process_entries", opt->repo);
32973369
process_entries(opt, &working_tree_oid);
3370+
trace2_region_leave("merge", "process_entries", opt->repo);
32983371

32993372
/* Set return values */
33003373
result->tree = parse_tree_indirect(&working_tree_oid);
@@ -3395,9 +3468,15 @@ void merge_incore_nonrecursive(struct merge_options *opt,
33953468
struct tree *side2,
33963469
struct merge_result *result)
33973470
{
3471+
trace2_region_enter("merge", "incore_nonrecursive", opt->repo);
3472+
3473+
trace2_region_enter("merge", "merge_start", opt->repo);
33983474
assert(opt->ancestor != NULL);
33993475
merge_start(opt, result);
3476+
trace2_region_leave("merge", "merge_start", opt->repo);
3477+
34003478
merge_ort_nonrecursive_internal(opt, merge_base, side1, side2, result);
3479+
trace2_region_leave("merge", "incore_nonrecursive", opt->repo);
34013480
}
34023481

34033482
void merge_incore_recursive(struct merge_options *opt,
@@ -3406,9 +3485,15 @@ void merge_incore_recursive(struct merge_options *opt,
34063485
struct commit *side2,
34073486
struct merge_result *result)
34083487
{
3488+
trace2_region_enter("merge", "incore_recursive", opt->repo);
3489+
34093490
/* We set the ancestor label based on the merge_bases */
34103491
assert(opt->ancestor == NULL);
34113492

3493+
trace2_region_enter("merge", "merge_start", opt->repo);
34123494
merge_start(opt, result);
3495+
trace2_region_leave("merge", "merge_start", opt->repo);
3496+
34133497
merge_ort_internal(opt, merge_bases, side1, side2, result);
3498+
trace2_region_leave("merge", "incore_recursive", opt->repo);
34143499
}

0 commit comments

Comments
 (0)