This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1940

patch: avoid repeated binary checks if all files in a patch are text
ClosedPublic

Authored by joerg.sonnenberger on Jan 25 2018, 5:36 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG079b27b5a869: patch: avoid repeated binary checks if all files in a patch are text

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

lothiraldan added a subscriber: lothiraldan.Jan 26 2018, 5:53 AM

Do we have some numbers about how many seconds we win on big diffs?

joerg.sonnenberger added a comment.Jan 26 2018, 7:20 AM

I haven't seen a performance difference for regular runs, but it does show up in profiles, so eliminating it avoids wasting mental power by developers down the line.

Closed by commit rHG079b27b5a869: patch: avoid repeated binary checks if all files in a patch are text (authored by joerg.sonnenberger). · Explain WhyFeb 1 2018, 4:47 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/mdiff.py (7 lines)
Mmercurial/patch.py (12 lines)

Diff 5081

mercurial/mdiff.py

lines2 = splitnewlines(text2) lines2 = splitnewlines(text2)
old = wsclean(opts, "".join(lines1[s[0]:s[1]])) old = wsclean(opts, "".join(lines1[s[0]:s[1]]))
new = wsclean(opts, "".join(lines2[s[2]:s[3]])) new = wsclean(opts, "".join(lines2[s[2]:s[3]]))
if old == new: if old == new:
type = '~' type = '~'
yield s, type yield s, type
yield s1, '=' yield s1, '='
def unidiff(a, ad, b, bd, fn1, fn2, opts=defaultopts): def unidiff(a, ad, b, bd, fn1, fn2, opts=defaultopts, check_binary=True):
"""Return a unified diff as a (headers, hunks) tuple. """Return a unified diff as a (headers, hunks) tuple.
If the diff is not null, `headers` is a list with unified diff header If the diff is not null, `headers` is a list with unified diff header
lines "--- <original>" and "+++ <new>" and `hunks` is a generator yielding lines "--- <original>" and "+++ <new>" and `hunks` is a generator yielding
(hunkrange, hunklines) coming from _unidiff(). (hunkrange, hunklines) coming from _unidiff().
Otherwise, `headers` and `hunks` are empty. Otherwise, `headers` and `hunks` are empty.
Setting `check_binary` to false will skip the binary check, i.e. when
it has been done in advance. Files are expected to be text in this case.
""" """
def datetag(date, fn=None): def datetag(date, fn=None):
if not opts.git and not opts.nodates: if not opts.git and not opts.nodates:
return '\t%s' % date return '\t%s' % date
if fn and ' ' in fn: if fn and ' ' in fn:
return '\t' return '\t'
return '' return ''
fn2 = util.pconvert(fn2) fn2 = util.pconvert(fn2)
def checknonewline(lines): def checknonewline(lines):
for text in lines: for text in lines:
if text[-1:] != '\n': if text[-1:] != '\n':
text += "\n\ No newline at end of file\n" text += "\n\ No newline at end of file\n"
yield text yield text
if not opts.text and (util.binary(a) or util.binary(b)): if not opts.text and check_binary and (util.binary(a) or util.binary(b)):
if a and b and len(a) == len(b) and a == b: if a and b and len(a) == len(b) and a == b:
return sentinel return sentinel
headerlines = [] headerlines = []
hunks = (None, ['Binary file %s has changed\n' % fn1]), hunks = (None, ['Binary file %s has changed\n' % fn1]),
elif not a: elif not a:
b = splitnewlines(b) b = splitnewlines(b)
if a is None: if a is None:
l1 = '--- /dev/null%s' % datetag(epoch) l1 = '--- /dev/null%s' % datetag(epoch)

mercurial/patch.py

fctx1 = getfilectx(f1, ctx1) fctx1 = getfilectx(f1, ctx1)
if opts.git or losedatafn: if opts.git or losedatafn:
flag1 = ctx1.flags(f1) flag1 = ctx1.flags(f1)
if f2: if f2:
fctx2 = getfilectx(f2, ctx2) fctx2 = getfilectx(f2, ctx2)
if opts.git or losedatafn: if opts.git or losedatafn:
flag2 = ctx2.flags(f2) flag2 = ctx2.flags(f2)
# if binary is True, output "summary" or "base85", but not "text diff" # if binary is True, output "summary" or "base85", but not "text diff"
binary = not opts.text and any(f.isbinary() if opts.text:
check_binary = True
binary = False
else:
check_binary = any(f.isbinary()
for f in [fctx1, fctx2] if f is not None) for f in [fctx1, fctx2] if f is not None)
binary = check_binary
if losedatafn and not opts.git: if losedatafn and not opts.git:
if (binary or if (binary or
# copy/rename # copy/rename
f2 in copy or f2 in copy or
# empty file creation # empty file creation
(not f1 and isempty(fctx2)) or (not f1 and isempty(fctx2)) or
# empty file deletion # empty file deletion
flag = flag2 flag = flag2
header.append('index %s..%s %s' % header.append('index %s..%s %s' %
(gitindex(content1)[0:opts.index], (gitindex(content1)[0:opts.index],
gitindex(content2)[0:opts.index], gitindex(content2)[0:opts.index],
gitmode[flag])) gitmode[flag]))
uheaders, hunks = mdiff.unidiff(content1, date1, uheaders, hunks = mdiff.unidiff(content1, date1,
content2, date2, content2, date2,
path1, path2, opts=opts) path1, path2, opts=opts,
check_binary=check_binary)
header.extend(uheaders) header.extend(uheaders)
yield fctx1, fctx2, header, hunks yield fctx1, fctx2, header, hunks
def diffstatsum(stats): def diffstatsum(stats):
maxfile, maxtotal, addtotal, removetotal, binary = 0, 0, 0, 0, False maxfile, maxtotal, addtotal, removetotal, binary = 0, 0, 0, 0, False
for f, a, r, b in stats: for f, a, r, b in stats:
maxfile = max(maxfile, encoding.colwidth(f)) maxfile = max(maxfile, encoding.colwidth(f))
maxtotal = max(maxtotal, a + r) maxtotal = max(maxtotal, a + r)