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>

diff --git a/builtin-blame.c b/builtin-blame.c
index fc1586350f94ae48e7e48a51818517b465e7a40d..8deeee12b613dbd26c64cc1005151d67d0af7a4c 100644 (file)
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -39,7 +39,7 @@ static int show_root;
 static int reverse;
 static int blank_boundary;
 static int incremental;
 static int reverse;
 static int blank_boundary;
 static int incremental;

-static int xdl_opts = XDF_NEED_MINIMAL;
+static int xdl_opts;

 
 static enum date_mode blame_date_mode = DATE_ISO8601;
 static size_t blame_date_width;
 
 static enum date_mode blame_date_mode = DATE_ISO8601;
 static size_t blame_date_width;

diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index 1e70073a7ed022675031706a8e9f8c57ff3aa2a9..e5e860fa78e32355bc3b61999178dd64d00de75b 100644 (file)
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -25,7 +25,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
        const char *names[3] = { NULL, NULL, NULL };
        mmfile_t mmfs[3];
        mmbuffer_t result = {NULL, 0};
        const char *names[3] = { NULL, NULL, NULL };
        mmfile_t mmfs[3];
        mmbuffer_t result = {NULL, 0};

-       xmparam_t xmp = {{XDF_NEED_MINIMAL}};
+       xmparam_t xmp = {{0}};

        int ret = 0, i = 0, to_stdout = 0;
        int level = XDL_MERGE_ZEALOUS_ALNUM;
        int style = 0, quiet = 0;
        int ret = 0, i = 0, to_stdout = 0;
        int level = XDL_MERGE_ZEALOUS_ALNUM;
        int style = 0, quiet = 0;

diff --git a/builtin-merge-tree.c b/builtin-merge-tree.c
index a4a4f2ce4c3f147062070c2acc08eaf9f4d40be8..fc00d794d641c146023a78d7d8e4143f0fdac302 100644 (file)
--- a/builtin-merge-tree.c
+++ b/builtin-merge-tree.c
@@ -106,7 +106,7 @@ static void show_diff(struct merge_list *entry)
        xdemitconf_t xecfg;
        xdemitcb_t ecb;
 
        xdemitconf_t xecfg;
        xdemitcb_t ecb;
 

-       xpp.flags = XDF_NEED_MINIMAL;
+       xpp.flags = 0;

        memset(&xecfg, 0, sizeof(xecfg));
        xecfg.ctxlen = 3;
        ecb.outf = show_outf;
        memset(&xecfg, 0, sizeof(xecfg));
        xecfg.ctxlen = 3;
        ecb.outf = show_outf;

diff --git a/builtin-rerere.c b/builtin-rerere.c
index 34f9acee910406c7ba0bf9eed76267a0cf8f46c2..0048f9ef7fee24e5e058ef226f3b0fc93703fcf1 100644 (file)
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -89,7 +89,7 @@ static int diff_two(const char *file1, const char *label1,
        printf("--- a/%s\n+++ b/%s\n", label1, label2);
        fflush(stdout);
        memset(&xpp, 0, sizeof(xpp));
        printf("--- a/%s\n+++ b/%s\n", label1, label2);
        fflush(stdout);
        memset(&xpp, 0, sizeof(xpp));

-       xpp.flags = XDF_NEED_MINIMAL;
+       xpp.flags = 0;

        memset(&xecfg, 0, sizeof(xecfg));
        xecfg.ctxlen = 3;
        ecb.outf = outf;
        memset(&xecfg, 0, sizeof(xecfg));
        xecfg.ctxlen = 3;
        ecb.outf = outf;

diff --git a/combine-diff.c b/combine-diff.c
index 3480dae82416bda357dee2f0d545ac6dadc6a26f..13a812896116c81ab6423b0b9d4f1b9e52496e9f 100644 (file)
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -221,7 +221,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
        parent_file.ptr = grab_blob(parent, mode, &sz);
        parent_file.size = sz;
        memset(&xpp, 0, sizeof(xpp));
        parent_file.ptr = grab_blob(parent, mode, &sz);
        parent_file.size = sz;
        memset(&xpp, 0, sizeof(xpp));

-       xpp.flags = XDF_NEED_MINIMAL;
+       xpp.flags = 0;

        memset(&xecfg, 0, sizeof(xecfg));
        memset(&state, 0, sizeof(state));
        state.nmask = nmask;
        memset(&xecfg, 0, sizeof(xecfg));
        memset(&state, 0, sizeof(state));
        state.nmask = nmask;

diff --git a/diff.c b/diff.c
index edec0f6b818541b79e2773934ab1d28ce279805c..0924274dd3c21e6cf247a974cf0e492d907d8736 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -714,7 +714,7 @@ static void diff_words_show(struct diff_words_data *diff_words)
        memset(&xecfg, 0, sizeof(xecfg));
        diff_words_fill(&diff_words->minus, &minus, diff_words->word_regex);
        diff_words_fill(&diff_words->plus, &plus, diff_words->word_regex);
        memset(&xecfg, 0, sizeof(xecfg));
        diff_words_fill(&diff_words->minus, &minus, diff_words->word_regex);
        diff_words_fill(&diff_words->plus, &plus, diff_words->word_regex);

-       xpp.flags = XDF_NEED_MINIMAL;
+       xpp.flags = 0;

        /* as only the hunk header will be parsed, we need a 0-context */
        xecfg.ctxlen = 0;
        xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,
        /* as only the hunk header will be parsed, we need a 0-context */
        xecfg.ctxlen = 0;
        xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,


@@ -1743,7 +1743,7 @@ static void builtin_diff(const char *name_a,
                        check_blank_at_eof(&mf1, &mf2, &ecbdata);
                ecbdata.file = o->file;
                ecbdata.header = header.len ? &header : NULL;
                        check_blank_at_eof(&mf1, &mf2, &ecbdata);
                ecbdata.file = o->file;
                ecbdata.header = header.len ? &header : NULL;

-               xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
+               xpp.flags = o->xdl_opts;

                xecfg.ctxlen = o->context;
                xecfg.interhunkctxlen = o->interhunkcontext;
                xecfg.flags = XDL_EMIT_FUNCNAMES;
                xecfg.ctxlen = o->context;
                xecfg.interhunkctxlen = o->interhunkcontext;
                xecfg.flags = XDL_EMIT_FUNCNAMES;


@@ -1833,7 +1833,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
 
                memset(&xpp, 0, sizeof(xpp));
                memset(&xecfg, 0, sizeof(xecfg));
 
                memset(&xpp, 0, sizeof(xpp));
                memset(&xecfg, 0, sizeof(xecfg));

-               xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
+               xpp.flags = o->xdl_opts;

                xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
                              &xpp, &xecfg, &ecb);
        }
                xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
                              &xpp, &xecfg, &ecb);
        }


@@ -1882,7 +1882,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
                memset(&xpp, 0, sizeof(xpp));
                memset(&xecfg, 0, sizeof(xecfg));
                xecfg.ctxlen = 1; /* at least one context line */
                memset(&xpp, 0, sizeof(xpp));
                memset(&xecfg, 0, sizeof(xecfg));
                xecfg.ctxlen = 1; /* at least one context line */

-               xpp.flags = XDF_NEED_MINIMAL;
+               xpp.flags = 0;

                xdi_diff_outf(&mf1, &mf2, checkdiff_consume, &data,
                              &xpp, &xecfg, &ecb);
 
                xdi_diff_outf(&mf1, &mf2, checkdiff_consume, &data,
                              &xpp, &xecfg, &ecb);
 


@@ -3419,7 +3419,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
                                        len2, p->two->path);
                git_SHA1_Update(&ctx, buffer, len1);
 
                                        len2, p->two->path);
                git_SHA1_Update(&ctx, buffer, len1);
 

-               xpp.flags = XDF_NEED_MINIMAL;
+               xpp.flags = 0;

                xecfg.ctxlen = 3;
                xecfg.flags = XDL_EMIT_FUNCNAMES;
                xdi_diff_outf(&mf1, &mf2, patch_id_consume, &data,
                xecfg.ctxlen = 3;
                xecfg.flags = XDL_EMIT_FUNCNAMES;
                xdi_diff_outf(&mf1, &mf2, patch_id_consume, &data,

diff --git a/merge-file.c b/merge-file.c
index fd34d76e1516b2c944778a11a5670d382f245873..cafc274e2d0438c346ecc6d8df4290b740470756 100644 (file)
--- a/merge-file.c
+++ b/merge-file.c
@@ -60,7 +60,7 @@ static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)
        xdemitcb_t ecb;
 
        memset(&xpp, 0, sizeof(xpp));
        xdemitcb_t ecb;
 
        memset(&xpp, 0, sizeof(xpp));

-       xpp.flags = XDF_NEED_MINIMAL;
+       xpp.flags = 0;

        memset(&xecfg, 0, sizeof(xecfg));
        xecfg.ctxlen = 3;
        xecfg.flags = XDL_EMIT_COMMON;
        memset(&xecfg, 0, sizeof(xecfg));
        xecfg.ctxlen = 3;
        xecfg.flags = XDL_EMIT_COMMON;