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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Revision Contents
Changeset List

PathPackages
Mmercurial/localrepo.py (8 lines)

Diff 22645

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