@matheusbsbs / matheus-bagliano / commits / 582aa00bdf
Fork 0

git diff too slow for a file

Ever since the xdiff library had been introduced to git, all its callers have used the flag XDF_NEED_MINIMAL. It makes sure that the smallest possible diff is produced, but that takes quite some time if there are lots of differences that can be expressed in multiple ways. This flag makes a difference for only 0.1% of the non-merge commits in the git repo of Linux, both in terms of diff size and execution time. The patches there are mostly nice and small. SungHyun Nam however reported a case in a different repo where a diff took more than 20 times longer to generate with XDF_NEED_MINIMAL than without. Rebasing became really slow. This patch removes this flag from all callers. The default of xdiff is saner because it has minimal to no impact in the normal case of small diffs and doesn't incur that much of a speed penalty for large ones. A follow-up patch may introduce a command line option to set the flag if the user needs it, similar to GNU diff's -d/--minimal. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>

René Scharfe committed May 2, 2010 at 15:04 UTC 582aa00bdffb27abcf1b27d541b4c231a395d3b8
7 files changed +11 -11
builtin-blame.c
+1 -1
@@ -39,7 +39,7 @@ static int show_root;
39 static int reverse;
40 static int blank_boundary;
41 static int incremental;
42 -static int xdl_opts = XDF_NEED_MINIMAL;
42 +static int xdl_opts;
43
44 static enum date_mode blame_date_mode = DATE_ISO8601;
45 static size_t blame_date_width;
builtin-merge-file.c
+1 -1
@@ -25,7 +25,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
25 const char *names[3] = { NULL, NULL, NULL };
26 mmfile_t mmfs[3];
27 mmbuffer_t result = {NULL, 0};
28 - xmparam_t xmp = {{XDF_NEED_MINIMAL}};
28 + xmparam_t xmp = {{0}};
29 int ret = 0, i = 0, to_stdout = 0;
30 int level = XDL_MERGE_ZEALOUS_ALNUM;
31 int style = 0, quiet = 0;
builtin-merge-tree.c
+1 -1
@@ -106,7 +106,7 @@ static void show_diff(struct merge_list *entry)
106 xdemitconf_t xecfg;
107 xdemitcb_t ecb;
108
109 - xpp.flags = XDF_NEED_MINIMAL;
109 + xpp.flags = 0;
110 memset(&xecfg, 0, sizeof(xecfg));
111 xecfg.ctxlen = 3;
112 ecb.outf = show_outf;
builtin-rerere.c
+1 -1
@@ -89,7 +89,7 @@ static int diff_two(const char *file1, const char *label1,
89 printf("--- a/%s\n+++ b/%s\n", label1, label2);
90 fflush(stdout);
91 memset(&xpp, 0, sizeof(xpp));
92 - xpp.flags = XDF_NEED_MINIMAL;
92 + xpp.flags = 0;
93 memset(&xecfg, 0, sizeof(xecfg));
94 xecfg.ctxlen = 3;
95 ecb.outf = outf;
combine-diff.c
+1 -1
@@ -221,7 +221,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
221 parent_file.ptr = grab_blob(parent, mode, &sz);
222 parent_file.size = sz;
223 memset(&xpp, 0, sizeof(xpp));
224 - xpp.flags = XDF_NEED_MINIMAL;
224 + xpp.flags = 0;
225 memset(&xecfg, 0, sizeof(xecfg));
226 memset(&state, 0, sizeof(state));
227 state.nmask = nmask;
diff.c
+5 -5
@@ -714,7 +714,7 @@ static void diff_words_show(struct diff_words_data *diff_words)
714 memset(&xecfg, 0, sizeof(xecfg));
715 diff_words_fill(&diff_words->minus, &minus, diff_words->word_regex);
716 diff_words_fill(&diff_words->plus, &plus, diff_words->word_regex);
717 - xpp.flags = XDF_NEED_MINIMAL;
717 + xpp.flags = 0;
718 /* as only the hunk header will be parsed, we need a 0-context */
719 xecfg.ctxlen = 0;
720 xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,
@@ -1743,7 +1743,7 @@ static void builtin_diff(const char *name_a,
1743 check_blank_at_eof(&mf1, &mf2, &ecbdata);
1744 ecbdata.file = o->file;
1745 ecbdata.header = header.len ? &header : NULL;
1746 - xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
1746 + xpp.flags = o->xdl_opts;
1747 xecfg.ctxlen = o->context;
1748 xecfg.interhunkctxlen = o->interhunkcontext;
1749 xecfg.flags = XDL_EMIT_FUNCNAMES;
@@ -1833,7 +1833,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
1833
1834 memset(&xpp, 0, sizeof(xpp));
1835 memset(&xecfg, 0, sizeof(xecfg));
1836 - xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
1836 + xpp.flags = o->xdl_opts;
1837 xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
1838 &xpp, &xecfg, &ecb);
1839 }
@@ -1882,7 +1882,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
1882 memset(&xpp, 0, sizeof(xpp));
1883 memset(&xecfg, 0, sizeof(xecfg));
1884 xecfg.ctxlen = 1; /* at least one context line */
1885 - xpp.flags = XDF_NEED_MINIMAL;
1885 + xpp.flags = 0;
1886 xdi_diff_outf(&mf1, &mf2, checkdiff_consume, &data,
1887 &xpp, &xecfg, &ecb);
1888
@@ -3419,7 +3419,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
3419 len2, p->two->path);
3420 git_SHA1_Update(&ctx, buffer, len1);
3421
3422 - xpp.flags = XDF_NEED_MINIMAL;
3422 + xpp.flags = 0;
3423 xecfg.ctxlen = 3;
3424 xecfg.flags = XDL_EMIT_FUNCNAMES;
3425 xdi_diff_outf(&mf1, &mf2, patch_id_consume, &data,
merge-file.c
+1 -1
@@ -60,7 +60,7 @@ static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)
60 xdemitcb_t ecb;
61
62 memset(&xpp, 0, sizeof(xpp));
63 - xpp.flags = XDF_NEED_MINIMAL;
63 + xpp.flags = 0;
64 memset(&xecfg, 0, sizeof(xecfg));
65 xecfg.ctxlen = 3;
66 xecfg.flags = XDL_EMIT_COMMON;