This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7492

localrepo: also fastpath access to working copy parents when possible
ClosedPublic

Authored by marmoute on Nov 22 2019, 4:21 AM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG85c4cd73996b: localrepo: also fastpath access to working copy parents when possible
Summary

If the filter level garantee that the workig copy parents will be visible, we
allow fast path access to them. With this change multiple commands can now run
without triggering filtering.

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 created this revision.Nov 22 2019, 4:21 AM
marmoute updated this revision to Diff 18358.Nov 23 2019, 8:03 AM
marmoute updated this revision to Diff 18365.Nov 23 2019, 7:21 PM
marmoute updated this revision to Diff 18382.Nov 24 2019, 6:26 AM
martinvonz added a subscriber: martinvonz.Dec 10 2019, 2:38 AM

Hmm, I thought this entire series had been queued. I'd like to queue it. Can you include perf numbers?

marmoute added a comment.Dec 12 2019, 7:42 AM
In D7492#111595, @martinvonz wrote:

Hmm, I thought this entire series had been queued. I'd like to queue it. Can you include perf numbers?

Now that we have a reference filtered repository, I am planning to use the benchmark suite to gather timing (probably next week). We already track status and diff. addign hg export is easy. However I am not sure what's your criteria for annotate ? Can you clarify them and offer a file to test for each for the reference repositories ?

martinvonz added a comment.Dec 12 2019, 9:31 AM
In D7492#111988, @marmoute wrote:
In D7492#111595, @martinvonz wrote:

Hmm, I thought this entire series had been queued. I'd like to queue it. Can you include perf numbers?

Now that we have a reference filtered repository, I am planning to use the benchmark suite to gather timing (probably next week). We already track status and diff. addign hg export is easy. However I am not sure what's your criteria for annotate ? Can you clarify them and offer a file to test for each for the reference repositories ?

Try the one I used as an example in my patch. I don't think I thought very hard about finding a particularly good example. You just want one that's fast to annotate so the time to load obsmarkers is a significant part of the total time.

pulkit added a subscriber: pulkit.Jan 9 2020, 9:26 AM

How's the perf number mining going? It will be nice to get this pushed before upcoming release candidate.

marmoute added a comment.Jan 16 2020, 5:24 AM

Here is raw perf comparison for status, diff and export. We don't have a benchmark for annottate yet. (not too long to write, but I did not had the time to do that yet)

All benchmarks:

       before           after         ratio
     [8e095512]       [36b2f659]
-       711±0.8ms       60.7±0.2ms     0.09  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
-       712±0.8ms       61.6±0.2ms     0.09  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
-         690±1ms       93.5±0.3ms     0.14  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
-         688±1ms       93.8±0.3ms     0.14  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
-         714±1ms       60.7±0.8ms     0.09  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
-         713±1ms       60.9±0.3ms     0.09  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
-         689±1ms       93.7±0.2ms     0.14  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
-         687±2ms       92.8±0.2ms     0.14  simple_command.read.diff.empty.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
-         799±2ms       98.1±0.6ms     0.12  simple_command.read.export.bare.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
-       800±0.8ms      100.0±0.4ms     0.12  simple_command.read.export.bare.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
-       711±0.9ms        111±0.2ms     0.16  simple_command.read.export.bare.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
-         711±1ms        112±0.3ms     0.16  simple_command.read.export.bare.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
-         760±1ms       59.8±0.1ms     0.08  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
-         763±2ms       62.2±0.3ms     0.08  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
-         689±1ms       93.1±0.3ms     0.14  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
-         688±1ms       94.3±0.3ms     0.14  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 1) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
-         763±1ms       60.1±0.2ms     0.08  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
-         763±1ms       62.1±0.4ms     0.08  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py2.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
-       689±0.8ms       93.2±0.2ms     0.14  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYc-HGWITHRUSTEXTcpython]
-       687±0.9ms       94.1±0.3ms     0.14  simple_command.read.status.wc_clean.default.time_bench('mercurial-filtered-2019-11-22', 'zstd', 'default', True, True, True, True, True, 2) [citrea/virtualenv-py3.7-pyyaml-HGMODULEPOLICYrust+c-HGWITHRUSTEXTcpython]
Alphare updated this revision to Diff 19361.Jan 16 2020, 9:24 AM
pulkit accepted this revision.Jan 17 2020, 9:57 AM

Amending the perf numbers in commit message and fixed some spellings.

This revision is now accepted and ready to land.Jan 17 2020, 9:57 AM
marmoute added a comment.Jan 17 2020, 10:00 AM

Thank you, much appreciated.

Revision Contents
Changeset List

Diff 19409

mercurial/context.py

def markcommitted(self, node): def markcommitted(self, node):
with self._repo.dirstate.parentchange(): with self._repo.dirstate.parentchange():
for f in self.modified() + self.added(): for f in self.modified() + self.added():
self._repo.dirstate.normal(f) self._repo.dirstate.normal(f)
for f in self.removed(): for f in self.removed():
self._repo.dirstate.drop(f) self._repo.dirstate.drop(f)
self._repo.dirstate.setparents(node) self._repo.dirstate.setparents(node)
self._repo._quick_access_changeid_invalidate()
# write changes out explicitly, because nesting wlock at # write changes out explicitly, because nesting wlock at
# runtime may prevent 'wlock.release()' in 'repo.commit()' # runtime may prevent 'wlock.release()' in 'repo.commit()'
# from immediately doing so for subsequent changing files # from immediately doing so for subsequent changing files
self._repo.dirstate.write(self._repo.currenttransaction()) self._repo.dirstate.write(self._repo.currenttransaction())
sparse.aftercommit(self._repo, node) sparse.aftercommit(self._repo, node)

mercurial/localrepo.py

return matchmod.intersectmatchers(match, nm) return matchmod.intersectmatchers(match, nm)
return matchmod.intersectmatchers(match, self._narrowmatch) return matchmod.intersectmatchers(match, self._narrowmatch)
return self._narrowmatch return self._narrowmatch
def setnarrowpats(self, newincludes, newexcludes): def setnarrowpats(self, newincludes, newexcludes):
narrowspec.save(self, newincludes, newexcludes) narrowspec.save(self, newincludes, newexcludes)
self.invalidate(clearfilecache=True) self.invalidate(clearfilecache=True)
@util.propertycache @unfilteredpropertycache
def _quick_access_changeid_null(self):
return {
b'null': (nullrev, nullid),
nullrev: (nullrev, nullid),
nullid: (nullrev, nullid),
}
@unfilteredpropertycache
def _quick_access_changeid_wc(self):
# also fast path access to the working copy parents
# however, only do it for filter that ensure wc is visible.
quick = {}
cl = self.unfiltered().changelog
for node in self.dirstate.parents():
if node == nullid:
continue
rev = cl.index.get_rev(node)
if rev is None:
# unknown working copy parent case:
#
# skip the fast path and let higher code deal with it
continue
pair = (rev, node)
quick[rev] = pair
quick[node] = pair
return quick
@unfilteredmethod
def _quick_access_changeid_invalidate(self):
if '_quick_access_changeid_wc' in vars(self):
del self.__dict__['_quick_access_changeid_wc']
@property
def _quick_access_changeid(self): def _quick_access_changeid(self):
"""an helper dictionnary for __getitem__ calls """an helper dictionnary for __getitem__ calls
This contains a list of symbol we can recognise right away without This contains a list of symbol we can recognise right away without
further processing. further processing.
""" """
return { mapping = self._quick_access_changeid_null
b'null': (nullrev, nullid), if self.filtername in repoview.filter_has_wc:
nullrev: (nullrev, nullid), mapping = mapping.copy()
nullid: (nullrev, nullid), mapping.update(self._quick_access_changeid_wc)
} return mapping
def __getitem__(self, changeid): def __getitem__(self, changeid):
# dealing with special cases # dealing with special cases
if changeid is None: if changeid is None:
return context.workingctx(self) return context.workingctx(self)
if isinstance(changeid, context.basectx): if isinstance(changeid, context.basectx):
return changeid return changeid
return b'store' return b'store'
return None return None
def wjoin(self, f, *insidef): def wjoin(self, f, *insidef):
return self.vfs.reljoin(self.root, f, *insidef) return self.vfs.reljoin(self.root, f, *insidef)
def setparents(self, p1, p2=nullid): def setparents(self, p1, p2=nullid):
self[None].setparents(p1, p2) self[None].setparents(p1, p2)
self._quick_access_changeid_invalidate()
def filectx(self, path, changeid=None, fileid=None, changectx=None): def filectx(self, path, changeid=None, fileid=None, changectx=None):
"""changeid must be a changeset revision, if specified. """changeid must be a changeset revision, if specified.
fileid can be a file revision or node.""" fileid can be a file revision or node."""
return context.filectx( return context.filectx(
self, path, changeid, fileid, changectx=changectx self, path, changeid, fileid, changectx=changectx
) )
self._branchcaches.clear() self._branchcaches.clear()
self.invalidatevolatilesets() self.invalidatevolatilesets()
self._sparsesignaturecache.clear() self._sparsesignaturecache.clear()
def invalidatevolatilesets(self): def invalidatevolatilesets(self):
self.filteredrevcache.clear() self.filteredrevcache.clear()
obsolete.clearobscaches(self) obsolete.clearobscaches(self)
self._quick_access_changeid_invalidate()
def invalidatedirstate(self): def invalidatedirstate(self):
'''Invalidates the dirstate, causing the next call to dirstate '''Invalidates the dirstate, causing the next call to dirstate
to check if it was modified since the last time it was read, to check if it was modified since the last time it was read,
rereading it if it has. rereading it if it has.
This is different to dirstate.invalidate() that it doesn't always This is different to dirstate.invalidate() that it doesn't always
rereads the dirstate. Use dirstate.invalidate() if you want to rereads the dirstate. Use dirstate.invalidate() if you want to

mercurial/scmutil.py

(dst, oldcopies.get(src, src)) (dst, oldcopies.get(src, src))
for dst, src in pycompat.iteritems(oldcopies) for dst, src in pycompat.iteritems(oldcopies)
) )
# Adjust the dirstate copies # Adjust the dirstate copies
for dst, src in pycompat.iteritems(copies): for dst, src in pycompat.iteritems(copies):
if src not in newctx or dst in newctx or ds[dst] != b'a': if src not in newctx or dst in newctx or ds[dst] != b'a':
src = None src = None
ds.copy(src, dst) ds.copy(src, dst)
repo._quick_access_changeid_invalidate()
def writerequires(opener, requirements): def writerequires(opener, requirements):
with opener(b'requires', b'w', atomictemp=True) as fp: with opener(b'requires', b'w', atomictemp=True) as fp:
for r in sorted(requirements): for r in sorted(requirements):
fp.write(b"%s\n" % r) fp.write(b"%s\n" % r)

tests/test-repo-filters-tiptoe.t

Getting status of null Getting status of null
$ hg status --change null $ hg status --change null
Getting status of working copy Getting status of working copy
$ hg status $ hg status
debug.filters: computing revision filter for "visible"
M c M c
A d A d
R a R a
! b ! b
Getting data about the working copy parent Getting data about the working copy parent
$ hg log -r '.' -T "{node}\n{date}\n" $ hg log -r '.' -T "{node}\n{date}\n"
debug.filters: computing revision filter for "visible" debug.filters: computing revision filter for "visible"
c2932ca7786be30b67154d541a8764fae5532261 c2932ca7786be30b67154d541a8764fae5532261
0.00 0.00
Getting working copy diff Getting working copy diff
$ hg diff $ hg diff
debug.filters: computing revision filter for "visible"
diff -r c2932ca7786be30b67154d541a8764fae5532261 a diff -r c2932ca7786be30b67154d541a8764fae5532261 a
--- a/a Thu Jan 01 00:00:00 1970 +0000 --- a/a Thu Jan 01 00:00:00 1970 +0000
+++ /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +0,0 @@ @@ -1,1 +0,0 @@
-a -a
diff -r c2932ca7786be30b67154d541a8764fae5532261 c diff -r c2932ca7786be30b67154d541a8764fae5532261 c
--- a/c Thu Jan 01 00:00:00 1970 +0000 --- a/c Thu Jan 01 00:00:00 1970 +0000
+++ b/c Thu Jan 01 00:00:00 1970 +0000 +++ b/c Thu Jan 01 00:00:00 1970 +0000