Now that we extend matches backwards in the inner loop, the final
adjustment has no effect.
(A similar extension for the forward direction is trickier and has
less benefit.)
For very large diffs that have large numbers of identical lines (JSON
dumps) that also have large blocks of identical text, bdiff could become
confused about which block matches which because it can only match
very limited regions. The result is very large diffs for small sets of edits.
The earlier recursion rebalancing fix made this behavior more frequent because
it's now more prone to match block 1 to block 2. One frequent user of
large JSON files reported being unable to pass the resulting diffs
through their code review system.
Prior to this change, bdiff would calculate the length of a match at
(i, j) as 1 + length found at (i-1, j-1). With large number of popular
(ignored) lines, this often meant matches couldn't be extended
backwards at all and thus all matching regions were very small.
Disabling the popularity threshold is not an option because it brings
back quadratic behavior.
Instead, we extend a match backwards until we either found a previously
discovered match or we find a mismatching line. This thus successfully
bridges over any popular lines inside and before a matching region.
The larger regions then significant reduce the probability of confusion.
This causes the longest_match search to limit itself to a window of
30000 lines during search (roughly 1MB), thus avoiding a full O(N*M)
search that might occur in repetitive structured inputs. For a
particular class of many MB pathological test cases, this generated
the following timings:
size before after
10x 1.25s 1.24s
100x 57s 33s
1000x >8400s 400s
The times on the right quickly become much faster and appear more linear.
While windowing means deltas are no longer "optimal", the resulting
deltas were within a couple percent of expected size. While we've yet
to have a report of a file with the level of repetition necessary to
hit this case, some JSON/XML database dump scenario is fairly likely
to hit it.
This may also slightly improve the average-case performance for deltas
of large binaries.
For highly structured files like JSON or XML dumps with large numbers
of duplicate lines (eg braces) and isolated matching lines, bdiff
could find large numbers of equally good spans. Because it prefers
earlier matches, this would result in pathologically unbalance
recursion that resulted in quadratic performance.
This patch makes it prefer matches closer to the middle that tend to
balance recursion. This change improves the speed of a pathological
test case from 1100s to 9s.
Included is a smaller test that has a roughly 50x safety margin on the
performance it accepts. It's likely to fail on pure builds because
difflib also has a recursion-balancing problem.
The longest_match code compares all the possible positions in two
files to find the best match. Given a pair of sequences, it
effectively searches a grid like this:
a b b b c . d e . f
0 1 2 3 4 5 6 7 8 9
a 1 - - - - - - - - -
b - 2 1 1 - - - - - -
b - 1 3 2 - - - - - -
b - 1 2 4 - - - - - -
. - - - - - 1 - - 1 -
Here, the 4 in the middle says "the first four lines of the
file match", which it can compute be comparing the fourth lines and
then adding one to the result found when comparing the third lines in
the entry to the upper left.
We generally avoid the quadratic worst case by only looking at lines
that match, which is precomputed. We also avoid quadratic storage by
only keeping a single column vector and then keeping track of the best
match.
Unfortunately, this can get us into trouble with the sequences above.
Because we want to reuse the '3' value when calculating the '4', we
need to be careful not to overwrite it with the '2' we calculate
immediately before. If we scan left to right, top to bottom, we're
going to have a problem: we'll overwrite our 3 before we use it and
calculate a suboptimal best match.
To address this, we can either keep two column vectors and swap
between them (which significantly complicates bookkeeping), or change
our scanning order. If we instead scan from left to right, bottom to
top, we'll avoid ever overwriting values we'll need in the future.
This unfortunately needs several changes to be made simultaneously:
- change the order we build the initial hash chains for the b sequence
- change the sentinel values from INT_MAX to -1
- change the visit order in the longest_match inner loop
- add a tie-breaker preference for earlier matches
This last is needed because we previously had an implicit tie-breaker
from our visitation order that our test suite relies on. Later matches
can also trigger a bug in the normalization code in diff().
This bug is hidden by the current bias towards matches at the
beginning of the file. When this bias is tweaked later to address
recursion balancing, the normalization code could cause the next block
to shrink to a negative length, thus creating invalid delta chunks. We
add checks here to disallow that.
This bug requires test cases that are an awkwardly large size for the test
suite, but is very rapidly picked up by the included torture tester.
This just makes the code harder to read without any performance
advantage. We're going to make the check here more complex, let's make
it simpler first.
Eliminates
mercurial/bdiff.c(383) : warning C4244: 'function' : conversion from '__int64'
to 'uint32_t', possible loss of data
mercurial/bdiff.c(384) : warning C4244: 'function' : conversion from '__int64'
to 'uint32_t', possible loss of data
mercurial/bdiff.c(385) : warning C4244: 'function' : conversion from
'Py_ssize_t' to 'uint32_t', possible loss of data
when compiling for Windows x64 target using the Microsoft compiler.
Reduces the conversion warnings
mercurial/bdiff.c(61) : warning C4244: '=' : conversion from '__int64' to
'int', possible loss of data
mercurial/bdiff.c(307) : warning C4244: 'function' : conversion from
'Py_ssize_t' to 'int', possible loss of data
mercurial/bdiff.c(308) : warning C4244: 'function' : conversion from
'Py_ssize_t' to 'int', possible loss of data
mercurial/bdiff.c(362) : warning C4244: '+=' : conversion from '__int64' to
'int', possible loss of data
mercurial/bdiff.c(380) : warning C4244: '=' : conversion from '__int64' to
'int', possible loss of data
mercurial/bdiff.c(381) : warning C4244: 'function' : conversion from '__int64'
to 'uint32_t', possible loss of data
mercurial/bdiff.c(382) : warning C4244: 'function' : conversion from '__int64'
to 'uint32_t', possible loss of data
mercurial/bdiff.c(416) : warning C4244: '=' : conversion from 'Py_ssize_t' to
'int', possible loss of data
to
mercurial/bdiff.c(383) : warning C4244: 'function' : conversion from '__int64'
to 'uint32_t', possible loss of data
mercurial/bdiff.c(384) : warning C4244: 'function' : conversion from '__int64'
to 'uint32_t', possible loss of data
mercurial/bdiff.c(385) : warning C4244: 'function' : conversion from
'Py_ssize_t' to 'uint32_t', possible loss of data
on the three putbe32() calls in the function bdiff
when compiling for Windows x64 target using the Microsoft compiler.
This means that threaded webservers will have more of a chance of
doing something useful while the C extension is busy computing a
delta. Not doing this was causing problems for Google Code with a 25
meg text file that takes O(7 minutes) to deltify.
If fixws() is called on a zero-length string, malloc(0) is called and
expected to return a pointer. Which it does on e.g. Linux. AIX returns
NULL, which it is also legal, but the malloc() is then assumed to have
failed. So ensure a valid pointer is always returned.
This patch adds support for py3k in bdiff.c. This is accomplished by including
a header file responsible for abstracting the API differences between python 2
and python 3.
Patch from Jason Orendorff
The lower the threshold, the stronger the popularity hack's
influence. So at 3999 lines, the hack is disabled; and at 4000 lines,
the hack is enabled at maximum strength (t=4).
No source file in mercurial/crew is over 4000 lines. But there are, oh,
a few such files in Mozilla. I can testify that this hack causes hg to
generate some correct but eyebrow-raising patches.
I think the hack should phase in gradually. The threshold should be high
for small files where we don't need it so much. Like this:
t = (bn < 31000) ? 1000000 / bn : bn / 1000;
That would leave the popularity hack disabled for small files, then
gradually phase it in:
bn < 1000 -- t > bn (popularity hack is completely disabled)
bn == 1000 -- t = 1000 (still effectively disabled)
bn == 2000 -- t = 500 (only hits unusual files)
bn == 10000 -- t = 100 (only hits especially common lines)
bn == 31000 -- t = 31 (hack is at maximum power)
bn == 32000 -- t = 32 (hack could backfire, ease off)
When the common part of a diff can be moved forward, move it forward.
Otherwise we don't get deterministic results (it would depends on the way we
split for the recursion).
That way we get identical hunks when doing the same change, it helps to solve
issue1295 (inconsistent diffs on different side during a merge).
- adjust the common line threshold to .1%
this speeds up a delta of 7M lines of source from 10m to 40s
- adjust the scaling of the hash array down a bit as it was raising the peak
memory usage significantly
lyhash is a very simple and fast hash function that had the fewest
hash collisions on a 3.9M line text corpus and 190k line binary corpus
and should have significantly fewer collisions than the current hash
function.
pretty easy to find after I recompiled the python interpreter and
mercurial for profiling.
In "bdiff.c" function "equatelines" allocates the minimum hash table
size, which can lead to tons of collisions. I introduced an
"overcommit" factor of 16, this is, I allocate 16 times more memory
than the minimum value. Overcommiting 128 times does not improve the
performance over the 16-times case.
bdiff.blocks() returns a dummy match at the end of both files; the
length of that chunk is never set, so it will sometimes contain random
heap garbage. There are apparently workarounds for this elsewhere:
# bdiff sometimes gives huge matches past eof, this check eats them,
Python 2.5 doesn't like it when we mix str objects and the "t#" format
in PyArg_ParseTuple. Change it to use "s#". Tested with python 2.3, 2.4
and 2.5.
manifest.add gives revlog.addrevision a buffer object, which may
be cached and used for a second call in the same session (as mq does
when pushing multiple patches). The other option would be to cast the
buffer to str when caching it.
