Skip to content

Commit 03f2915

Browse files
yndolggitster
authored andcommitted
xdiff: disable cleanup_records heuristic with --minimal
The cleanup_records function marks some lines as changed before running the actual diff algorithm. For most lines, this is a good performance optimization, but it also marks lines that are surrounded by many changed lines as changed as well. This can cause redundant changes and longer-than-necessary diffs. Whether this results in better-looking diffs is subjective. However, the --minimal flag explicitly requests the shortest possible diff. The change results in shorter diffs in about 1.3% of all diffs in Git's history. Performance wise, I have measured the impact on "git log -p -3000 --minimal > /dev/null". With this change, I get Time (mean ± σ): 2.363 s ± 0.023 s (25 runs) and without this patch I measured Time (mean ± σ): 2.362 s ± 0.035 s (25 runs). As the difference is well within the margin of error, this does not seem to have an impact on performance. Signed-off-by: Niels Glodny <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent f65182a commit 03f2915

File tree

3 files changed

+18
-2
lines changed

3 files changed

+18
-2
lines changed

t/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,7 @@ integration_tests = [
501501
't4068-diff-symmetric-merge-base.sh',
502502
't4069-remerge-diff.sh',
503503
't4070-diff-pairs.sh',
504+
't4071-diff-minimal.sh',
504505
't4100-apply-stat.sh',
505506
't4101-apply-nonl.sh',
506507
't4102-apply-rename.sh',

t/t4071-diff-minimal.sh

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#!/bin/sh
2+
3+
test_description='minimal diff algorithm'
4+
5+
. ./test-lib.sh
6+
7+
test_expect_success 'minimal diff should not mark changes between changed lines' '
8+
test_write_lines x x x x >pre &&
9+
test_write_lines x x x A B C D x E F G >post &&
10+
test_expect_code 1 git diff --no-index --minimal pre post >diff &&
11+
test_grep ! ^[+-]x diff
12+
'
13+
14+
test_done

xdiff/xprepare.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
368368
xrecord_t **recs;
369369
xdlclass_t *rcrec;
370370
char *dis, *dis1, *dis2;
371+
int need_min = !!(cf->flags & XDF_NEED_MINIMAL);
371372

372373
if (!XDL_CALLOC_ARRAY(dis, xdf1->nrec + xdf2->nrec + 2))
373374
return -1;
@@ -379,15 +380,15 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
379380
for (i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart]; i <= xdf1->dend; i++, recs++) {
380381
rcrec = cf->rcrecs[(*recs)->ha];
381382
nm = rcrec ? rcrec->len2 : 0;
382-
dis1[i] = (nm == 0) ? 0: (nm >= mlim) ? 2: 1;
383+
dis1[i] = (nm == 0) ? 0: (nm >= mlim && !need_min) ? 2: 1;
383384
}
384385

385386
if ((mlim = xdl_bogosqrt(xdf2->nrec)) > XDL_MAX_EQLIMIT)
386387
mlim = XDL_MAX_EQLIMIT;
387388
for (i = xdf2->dstart, recs = &xdf2->recs[xdf2->dstart]; i <= xdf2->dend; i++, recs++) {
388389
rcrec = cf->rcrecs[(*recs)->ha];
389390
nm = rcrec ? rcrec->len1 : 0;
390-
dis2[i] = (nm == 0) ? 0: (nm >= mlim) ? 2: 1;
391+
dis2[i] = (nm == 0) ? 0: (nm >= mlim && !need_min) ? 2: 1;
391392
}
392393

393394
for (nreff = 0, i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart];

0 commit comments

Comments
 (0)