This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9022

repo: avoid copying/updating a dict on every `repo.__getitem__`
ClosedPublic

Authored by spectral on Sep 15 2020, 6:48 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG4a0ccbecbaa6: repo: avoid copying/updating a dict on every `repo.__getitem__`
Summary

This has some mild performance benefits. I'm looking into a pathological case
where one of our hg log invocations takes several seconds, and according to
hyperfine this reduces the wall time of the entire operation (running in chg)
from:

Time (mean ± σ):      7.390 s ±  0.106 s    [User: 7.058 s, System: 0.271 s]

Range (min … max):    7.300 s …  7.625 s

to:

Time (mean ± σ):      7.046 s ±  0.091 s    [User: 6.714 s, System: 0.279 s]

Range (min … max):    6.916 s …  7.169 s

Note: the log command is slow due to an issue in our custom stuff executing
repo[<arg>] 298,800 times. This performance improvement is likely not
noticeable during normal operation, but I don't feel like it's making the code
more difficult to understand, and every small bit helps.

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.

Revision Contents
Changeset List

PathPackages
Mmercurial/localrepo.py (8 lines)

Diff 22648

mercurial/localrepo.py

nullrev: (nullrev, nullid), nullrev: (nullrev, nullid),
nullid: (nullrev, nullid), nullid: (nullrev, nullid),
} }
@unfilteredpropertycache @unfilteredpropertycache
def _quick_access_changeid_wc(self): def _quick_access_changeid_wc(self):
# also fast path access to the working copy parents # also fast path access to the working copy parents
# however, only do it for filter that ensure wc is visible. # however, only do it for filter that ensure wc is visible.
quick = {} quick = self._quick_access_changeid_null.copy()
cl = self.unfiltered().changelog cl = self.unfiltered().changelog
for node in self.dirstate.parents(): for node in self.dirstate.parents():
if node == nullid: if node == nullid:
continue continue
rev = cl.index.get_rev(node) rev = cl.index.get_rev(node)
if rev is None: if rev is None:
# unknown working copy parent case: # unknown working copy parent case:
# #
@property @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.
""" """
mapping = self._quick_access_changeid_null
if self.filtername in repoview.filter_has_wc: if self.filtername in repoview.filter_has_wc:
mapping = mapping.copy() return self._quick_access_changeid_wc
mapping.update(self._quick_access_changeid_wc) return self._quick_access_changeid_null
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