Mercurial > gnulib
changeset 17619:d09167851e07
diffseq: remove TOO_EXPENSIVE heuristic
Problem with diffutils reported by Vincent Lefevre in
<http://bugs.gnu.org/16848>. The simplest solution is to remove
the TOO_EXPENSIVE heuristic that I added to GNU diff in 1993.
Although appropriate for circa-1993 hardware, these days the heuristic
seems to be more trouble than it's worth.
* lib/diffseq.h: Modernize citations.
(struct context): Remove member too_expensive.
All uses changed.
(struct partition): Remove members lo_minimal, hi_minimal.
All uses changed.
(diag, compareseq): Remove arg find_minimal. All uses changed.
(diag): Remove the TOO_EXPENSIVE heuristic that I added back in
1993 to make 'diff' run faster (but not as well) on large inputs.
These days, computers are fast enough that it's typically better
to run slower but more accurately.
* lib/fstrcmp.c: Remove duplicate comment.
* lib/fstrcmp.c (strcmp_bounded):
* lib/git-merge-changelog.c (compute_differences):
Adjust to diffseq.h changes.
| author | Paul Eggert <eggert@cs.ucla.edu> |
|---|---|
| date | Sun, 23 Feb 2014 16:16:31 -0800 |
| parents | 4941d0da6e24 |
| children | 0dd22c534074 |
| files | ChangeLog lib/diffseq.h lib/fstrcmp.c lib/git-merge-changelog.c |
| diffstat | 4 files changed, 42 insertions(+), 154 deletions(-) [+] |
line wrap: on
line diff
--- a/ChangeLog Sun Feb 23 14:20:18 2014 -0800 +++ b/ChangeLog Sun Feb 23 16:16:31 2014 -0800 @@ -1,5 +1,26 @@ 2014-02-23 Paul Eggert <eggert@cs.ucla.edu> + diffseq: remove TOO_EXPENSIVE heuristic + Problem with diffutils reported by Vincent Lefevre in + <http://bugs.gnu.org/16848>. The simplest solution is to remove + the TOO_EXPENSIVE heuristic that I added to GNU diff in 1993. + Although appropriate for circa-1993 hardware, these days the heuristic + seems to be more trouble than it's worth. + * lib/diffseq.h: Modernize citations. + (struct context): Remove member too_expensive. + All uses changed. + (struct partition): Remove members lo_minimal, hi_minimal. + All uses changed. + (diag, compareseq): Remove arg find_minimal. All uses changed. + (diag): Remove the TOO_EXPENSIVE heuristic that I added back in + 1993 to make 'diff' run faster (but not as well) on large inputs. + These days, computers are fast enough that it's typically better + to run slower but more accurately. + * lib/fstrcmp.c: Remove duplicate comment. + * lib/fstrcmp.c (strcmp_bounded): + * lib/git-merge-changelog.c (compute_differences): + Adjust to diffseq.h changes. + savedir: simplify by using stpcpy * lib/savedir.c (direntry_t): Remove size member. All uses removed. (streamsavedir): Use stpcpy instead.
--- a/lib/diffseq.h Sun Feb 23 14:20:18 2014 -0800 +++ b/lib/diffseq.h Sun Feb 23 16:16:31 2014 -0800 @@ -26,18 +26,15 @@ distance" in Wikipedia. The basic algorithm is described in: - "An O(ND) Difference Algorithm and its Variations", Eugene Myers, - Algorithmica Vol. 1 No. 2, 1986, pp. 251-266; - see especially section 4.2, which describes the variation used below. + "An O(ND) Difference Algorithm and its Variations", Eugene W. Myers, + Algorithmica Vol. 1, 1986, pp. 251-266, + <http://dx.doi.org/10.1007/BF01840446>. + See especially section 4.2, which describes the variation used below. The basic algorithm was independently discovered as described in: - "Algorithms for Approximate String Matching", E. Ukkonen, - Information and Control Vol. 64, 1985, pp. 100-118. - - Unless the 'find_minimal' flag is set, this code uses the TOO_EXPENSIVE - heuristic, by Paul Eggert, to limit the cost to O(N**1.5 log N) - at the price of producing suboptimal output for large inputs with - many differences. */ + "Algorithms for Approximate String Matching", Esko Ukkonen, + Information and Control Vol. 64, 1985, pp. 100-118, + <http://dx.doi.org/10.1016/S0019-9958(85)80046-2>. */ /* Before including this file, you need to define: ELEMENT The element type of the vectors being compared. @@ -120,15 +117,12 @@ OFFSET *bdiag; #ifdef USE_HEURISTIC - /* This corresponds to the diff -H flag. With this heuristic, for - vectors with a constant small density of changes, the algorithm is - linear in the vectors size. */ + /* This corresponds to the diff --speed-large-files flag. With this + heuristic, for vectors with a constant small density of changes, + the algorithm is linear in the vector size. */ bool heuristic; #endif - /* Edit scripts longer than this are too expensive to compute. */ - OFFSET too_expensive; - /* Snakes bigger than this are considered "big". */ #define SNAKE_LIMIT 20 }; @@ -138,12 +132,6 @@ /* Midpoints of this partition. */ OFFSET xmid; OFFSET ymid; - - /* True if low half will be analyzed minimally. */ - bool lo_minimal; - - /* Likewise for high half. */ - bool hi_minimal; }; @@ -155,17 +143,10 @@ When the two searches meet, we have found the midpoint of the shortest edit sequence. - If FIND_MINIMAL is true, find the minimal edit script regardless of - expense. Otherwise, if the search is too expensive, use heuristics to - stop the search and report a suboptimal answer. - - Set PART->(xmid,ymid) to the midpoint (XMID,YMID). The diagonal number + Set *PART to the midpoint (XMID,YMID). The diagonal number XMID - YMID equals the number of inserted elements minus the number of deleted elements (counting only elements before the midpoint). - Set PART->lo_minimal to true iff the minimal edit script for the - left half of the partition is known; similarly for PART->hi_minimal. - This function assumes that the first elements of the specified portions of the two vectors do not match, and likewise that the last elements do not match. The caller must trim matching elements from the beginning and end @@ -175,7 +156,7 @@ suboptimal diff output. It cannot cause incorrect diff output. */ static void -diag (OFFSET xoff, OFFSET xlim, OFFSET yoff, OFFSET ylim, bool find_minimal, +diag (OFFSET xoff, OFFSET xlim, OFFSET yoff, OFFSET ylim, struct partition *part, struct context *ctxt) { OFFSET *const fd = ctxt->fdiag; /* Give the compiler a chance. */ @@ -235,7 +216,6 @@ { part->xmid = x; part->ymid = y; - part->lo_minimal = part->hi_minimal = true; return; } } @@ -268,14 +248,10 @@ { part->xmid = x; part->ymid = y; - part->lo_minimal = part->hi_minimal = true; return; } } - if (find_minimal) - continue; - #ifdef USE_HEURISTIC /* Heuristic: check occasionally for a diagonal that has made lots of progress compared with the edit distance. If we have any @@ -319,11 +295,7 @@ } } if (best > 0) - { - part->lo_minimal = true; - part->hi_minimal = false; - return; - } + return; } { @@ -358,77 +330,10 @@ } } if (best > 0) - { - part->lo_minimal = false; - part->hi_minimal = true; - return; - } + return; } } #endif /* USE_HEURISTIC */ - - /* Heuristic: if we've gone well beyond the call of duty, give up - and report halfway between our best results so far. */ - if (c >= ctxt->too_expensive) - { - OFFSET fxybest; - OFFSET fxbest IF_LINT (= 0); - OFFSET bxybest; - OFFSET bxbest IF_LINT (= 0); - - /* Find forward diagonal that maximizes X + Y. */ - fxybest = -1; - for (d = fmax; d >= fmin; d -= 2) - { - OFFSET x = MIN (fd[d], xlim); - OFFSET y = x - d; - if (ylim < y) - { - x = ylim + d; - y = ylim; - } - if (fxybest < x + y) - { - fxybest = x + y; - fxbest = x; - } - } - - /* Find backward diagonal that minimizes X + Y. */ - bxybest = OFFSET_MAX; - for (d = bmax; d >= bmin; d -= 2) - { - OFFSET x = MAX (xoff, bd[d]); - OFFSET y = x - d; - if (y < yoff) - { - x = yoff + d; - y = yoff; - } - if (x + y < bxybest) - { - bxybest = x + y; - bxbest = x; - } - } - - /* Use the better of the two diagonals. */ - if ((xlim + ylim) - bxybest < fxybest - (xoff + yoff)) - { - part->xmid = fxbest; - part->ymid = fxybest - fxbest; - part->lo_minimal = true; - part->hi_minimal = false; - } - else - { - part->xmid = bxbest; - part->ymid = bxybest - bxbest; - part->lo_minimal = false; - part->hi_minimal = true; - } - return; - } } #undef XREF_YREF_EQUAL } @@ -442,9 +347,6 @@ Note that XLIM, YLIM are exclusive bounds. All indices into the vectors are origin-0. - If FIND_MINIMAL, find a minimal difference no matter how - expensive it is. - The results are recorded by invoking NOTE_DELETE and NOTE_INSERT. Return false if terminated normally, or true if terminated through early @@ -452,7 +354,7 @@ static bool compareseq (OFFSET xoff, OFFSET xlim, OFFSET yoff, OFFSET ylim, - bool find_minimal, struct context *ctxt) + struct context *ctxt) { #ifdef ELEMENT ELEMENT const *xv = ctxt->xvec; /* Help the compiler. */ @@ -498,12 +400,12 @@ struct partition part IF_LINT2 (= { .xmid = 0, .ymid = 0 }); /* Find a point of correspondence in the middle of the vectors. */ - diag (xoff, xlim, yoff, ylim, find_minimal, &part, ctxt); + diag (xoff, xlim, yoff, ylim, &part, ctxt); /* Use the partitions to split this problem into subproblems. */ - if (compareseq (xoff, part.xmid, yoff, part.ymid, part.lo_minimal, ctxt)) + if (compareseq (xoff, part.xmid, yoff, part.ymid, ctxt)) return true; - if (compareseq (part.xmid, xlim, part.ymid, ylim, part.hi_minimal, ctxt)) + if (compareseq (part.xmid, xlim, part.ymid, ylim, ctxt)) return true; }
--- a/lib/fstrcmp.c Sun Feb 23 14:20:18 2014 -0800 +++ b/lib/fstrcmp.c Sun Feb 23 16:16:31 2014 -0800 @@ -13,32 +13,8 @@ GNU General Public License for more details. You should have received a copy of the GNU General Public License - along with this program. If not, see <http://www.gnu.org/licenses/>. - - - Derived from GNU diff 2.7, analyze.c et al. - - The basic idea is to consider two vectors as similar if, when - transforming the first vector into the second vector through a - sequence of edits (inserts and deletes of one element each), - this sequence is short - or equivalently, if the ordered list - of elements that are untouched by these edits is long. For a - good introduction to the subject, read about the "Levenshtein - distance" in Wikipedia. + along with this program. If not, see <http://www.gnu.org/licenses/>. */ - The basic algorithm is described in: - "An O(ND) Difference Algorithm and its Variations", Eugene Myers, - Algorithmica Vol. 1 No. 2, 1986, pp. 251-266; - see especially section 4.2, which describes the variation used below. - - The basic algorithm was independently discovered as described in: - "Algorithms for Approximate String Matching", E. Ukkonen, - Information and Control Vol. 64, 1985, pp. 100-118. - - Unless the 'find_minimal' flag is set, this code uses the TOO_EXPENSIVE - heuristic, by Paul Eggert, to limit the cost to O(N**1.5 log N) - at the price of producing suboptimal output for large inputs with - many differences. */ #include <config.h> @@ -203,16 +179,6 @@ ctxt.xvec = string1; ctxt.yvec = string2; - /* Set TOO_EXPENSIVE to be approximate square root of input size, - bounded below by 256. */ - ctxt.too_expensive = 1; - for (i = xvec_length + yvec_length; - i != 0; - i >>= 2) - ctxt.too_expensive <<= 1; - if (ctxt.too_expensive < 256) - ctxt.too_expensive = 256; - /* Allocate memory for fdiag and bdiag from a thread-local pool. */ fdiag_len = xvec_length + yvec_length + 3; gl_once (keys_init_once, keys_init); @@ -252,7 +218,7 @@ /* Now do the main comparison algorithm */ ctxt.edit_count = - ctxt.edit_count_limit; - if (compareseq (0, xvec_length, 0, yvec_length, 0, &ctxt)) /* Prob: 98% */ + if (compareseq (0, xvec_length, 0, yvec_length, &ctxt)) /* Prob: 98% */ /* The edit_count passed the limit. Hence the result would be < lower_bound. We can return any value < lower_bound instead. */ return 0.0;
--- a/lib/git-merge-changelog.c Sun Feb 23 14:20:18 2014 -0800 +++ b/lib/git-merge-changelog.c Sun Feb 23 16:16:31 2014 -0800 @@ -678,11 +678,10 @@ ctxt.index_mapping_reverse[j] = 0; ctxt.fdiag = XNMALLOC (2 * (n1 + n2 + 3), ssize_t) + n2 + 1; ctxt.bdiag = ctxt.fdiag + n1 + n2 + 3; - ctxt.too_expensive = n1 + n2; /* Store in ctxt.index_mapping and ctxt.index_mapping_reverse a -1 for each removed or added entry. */ - compareseq (0, n1, 0, n2, 0, &ctxt); + compareseq (0, n1, 0, n2, &ctxt); /* Complete the index_mapping and index_mapping_reverse arrays. */ i = 0;