This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8582

grep: reduce the cost of pathauditor checks when grepping working copy
ClosedPublic

Authored by valentin.gatienbaron on May 25 2020, 7:42 PM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Commits
rHG233ee525dcef: grep: reduce the cost of pathauditor checks when grepping working copy
Summary

Running time hg grep zxczxczxczxczxc -l on mozilla-central:

before:

real	0m20,000s
user	0m15,796s
sys	0m4,189s

after:

real	0m10,903s
user	0m8,964s
sys	0m1,916s

if vfs didn't call pathauditor at all:

real	0m7,781s
user	0m5,968s
sys	0m1,790s

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

marmoute accepted this revision.May 26 2020, 9:53 AM
marmoute added a subscriber: marmoute.

It would be nice to have performance number in addition to the percentage. However the change looks good as is.

valentin.gatienbaron added a comment.May 26 2020, 11:11 AM

I don't understand. Do you mean have numbers in the code in addition to the commit description? Seems like too much details.

marmoute added a comment.May 26 2020, 11:49 AM
In D8582#128089, @valentin.gatienbaron wrote:

I don't understand. Do you mean have numbers in the code in addition to the commit description? Seems like too much details.

Hum, I got confused, I remember seeing a changeset with percentage speed up only in description. This is -clearly- not this one however.

valentin.gatienbaron added a commit: rHG233ee525dcef: grep: reduce the cost of pathauditor checks when grepping working copy.May 28 2020, 12:23 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG233ee525dcef: grep: reduce the cost of pathauditor checks when grepping working copy (authored by valentin.gatienbaron). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/commands.py (57 lines)
Mmercurial/pathutil.py (14 lines)

Diff 21495

mercurial/commands.py

def prep(ctx, fns): def prep(ctx, fns):
rev = ctx.rev() rev = ctx.rev()
pctx = ctx.p1() pctx = ctx.p1()
matches.setdefault(rev, {}) matches.setdefault(rev, {})
if diff: if diff:
parent = pctx.rev() parent = pctx.rev()
matches.setdefault(parent, {}) matches.setdefault(parent, {})
files = revfiles.setdefault(rev, []) files = revfiles.setdefault(rev, [])
if rev is None:
# in `hg grep pattern`, 2/3 of the time is spent is spent in
# pathauditor checks without this in mozilla-central
contextmanager = repo.wvfs.audit.cached
else:
contextmanager = util.nullcontextmanager
with contextmanager():
for fn in fns: for fn in fns:
# fn might not exist in the revision (could be a file removed by the # fn might not exist in the revision (could be a file removed by
# revision). We could check `fn not in ctx` even when rev is None, # the revision). We could check `fn not in ctx` even when rev is
# but it's less racy to protect againt that in readfile. # None, but it's less racy to protect againt that in readfile.
if rev is not None and fn not in ctx: if rev is not None and fn not in ctx:
continue continue
copy = None copy = None
if follow: if follow:
copy = getrenamed(fn, rev) copy = getrenamed(fn, rev)
if copy: if copy:
copies.setdefault(rev, {})[fn] = copy copies.setdefault(rev, {})[fn] = copy
if fn in skip: if fn in skip:
skip.add(copy) skip.add(copy)
if fn in skip: if fn in skip:
continue continue
files.append(fn) files.append(fn)
if fn not in matches[rev]: if fn not in matches[rev]:
grepbody(fn, rev, readfile(ctx, fn)) grepbody(fn, rev, readfile(ctx, fn))
if diff: if diff:
pfn = copy or fn pfn = copy or fn
if pfn not in matches[parent] and pfn in pctx: if pfn not in matches[parent] and pfn in pctx:
grepbody(pfn, parent, readfile(pctx, pfn)) grepbody(pfn, parent, readfile(pctx, pfn))
ui.pager(b'grep') ui.pager(b'grep')
fm = ui.formatter(b'grep', opts) fm = ui.formatter(b'grep', opts)
for ctx in cmdutil.walkchangerevs(repo, match, opts, prep): for ctx in cmdutil.walkchangerevs(repo, match, opts, prep):
rev = ctx.rev() rev = ctx.rev()
parent = ctx.p1().rev() parent = ctx.p1().rev()
for fn in sorted(revfiles.get(rev, [])): for fn in sorted(revfiles.get(rev, [])):
states = matches[rev][fn] states = matches[rev][fn]

mercurial/pathutil.py

from __future__ import absolute_import from __future__ import absolute_import
import contextlib
import errno import errno
import os import os
import posixpath import posixpath
import stat import stat
from .i18n import _ from .i18n import _
from . import ( from . import (
encoding, encoding,
def check(self, path): def check(self, path):
try: try:
self(path) self(path)
return True return True
except (OSError, error.Abort): except (OSError, error.Abort):
return False return False
@contextlib.contextmanager
def cached(self):
if self._cached:
yield
else:
try:
self._cached = True
yield
finally:
self.audited.clear()
self.auditeddir.clear()
self._cached = False
def canonpath(root, cwd, myname, auditor=None): def canonpath(root, cwd, myname, auditor=None):
'''return the canonical path of myname, given cwd and root '''return the canonical path of myname, given cwd and root
>>> def check(root, cwd, myname): >>> def check(root, cwd, myname):
... a = pathauditor(root, realfs=False) ... a = pathauditor(root, realfs=False)
... try: ... try:
... return canonpath(root, cwd, myname, a) ... return canonpath(root, cwd, myname, a)