This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5170

branchmap: pass changelog into revbranchcache.branchinfo()
AbandonedPublic

Authored by pulkit on Oct 19 2018, 1:38 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

We call branchinfo() function into for-loops and each branchinfo() calls try to
access repo.changelog.

As D5161 introduced the functionality to pass a chnagelog into that function to
prevent accessing repo.changelog in loops, let's use that.

Timings of hg perfrevset 'branch("tip")' on hg-committed before and after:

Before
! wall 0.133497 comb 0.140000 user 0.130000 sys 0.010000 (best of 64)
! wall 0.139459 comb 0.150000 user 0.150000 sys 0.000000 (best of 57)

After
! wall 0.106518 comb 0.110000 user 0.100000 sys 0.010000 (best of 77)
! wall 0.106157 comb 0.110000 user 0.100000 sys 0.010000 (best of 83)

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Oct 19 2018, 1:38 PM
yuja added a subscriber: yuja.Oct 20 2018, 3:19 AM
! wall 0.133497 comb 0.140000 user 0.130000 sys 0.010000 (best of 64)
! wall 0.139459 comb 0.150000 user 0.150000 sys 0.000000 (best of 57)
After
! wall 0.106518 comb 0.110000 user 0.100000 sys 0.010000 (best of 77)
! wall 0.106157 comb 0.110000 user 0.100000 sys 0.010000 (best of 83)

I'm surprised that unfilteredrepo.changelog is also slow.

  • a/mercurial/localrepo.py

+++ b/mercurial/localrepo.py
@@ -2037,8 +2037,9 @@

if full:
    rbc = self.revbranchcache()

+ cl = self.changelog

Perhaps it has to be self.unfiltered().changelog because revbranchcache is
initialized with the unfiltered repo.

for r in self.changelog:
  • rbc.branchinfo(r)

+ rbc.branchinfo(r, changelog=cl)

Can't we just make revbranchcache keep repo.changelog reference? It doesn't
make sense to pass in repo.changelog as an argument, whereas the repo is
held by revbranchcache.

yuja added a comment.Oct 20 2018, 3:36 AM
rbc = self.revbranchcache()

+ cl = self.changelog

Perhaps it has to be self.unfiltered().changelog because revbranchcache is
initialized with the unfiltered repo.

Never mind. It doesn't matter so long as the passed rev isn't filtered.

indygreg added a subscriber: indygreg.Nov 13 2018, 8:15 PM

I think this patch can be abandoned because of other work late in the 4.8 cycle?

pulkit abandoned this revision.Nov 14 2018, 5:40 AM
In D5170#78533, @indygreg wrote:

I think this patch can be abandoned because of other work late in the 4.8 cycle?

Yep!

Revision Contents
Changeset List

PathPackages
Mmercurial/bundle2.py (2 lines)
Mmercurial/localrepo.py (3 lines)
Mmercurial/revset.py (3 lines)

Diff 12278

mercurial/bundle2.py

def addpartrevbranchcache(repo, bundler, outgoing): def addpartrevbranchcache(repo, bundler, outgoing):
# we include the rev branch cache for the bundle changeset # we include the rev branch cache for the bundle changeset
# (as an optional parts) # (as an optional parts)
cache = repo.revbranchcache() cache = repo.revbranchcache()
cl = repo.unfiltered().changelog cl = repo.unfiltered().changelog
branchesdata = collections.defaultdict(lambda: (set(), set())) branchesdata = collections.defaultdict(lambda: (set(), set()))
for node in outgoing.missing: for node in outgoing.missing:
branch, close = cache.branchinfo(cl.rev(node)) branch, close = cache.branchinfo(cl.rev(node), changelog=cl)
branchesdata[branch][close].add(node) branchesdata[branch][close].add(node)
def generate(): def generate():
for branch, (nodes, closed) in sorted(branchesdata.items()): for branch, (nodes, closed) in sorted(branchesdata.items()):
utf8branch = encoding.fromlocal(branch) utf8branch = encoding.fromlocal(branch)
yield rbcstruct.pack(len(utf8branch), len(nodes), len(closed)) yield rbcstruct.pack(len(utf8branch), len(nodes), len(closed))
yield utf8branch yield utf8branch
for n in sorted(nodes): for n in sorted(nodes):

mercurial/localrepo.py

if tr is None or tr.changes['origrepolen'] < len(self): if tr is None or tr.changes['origrepolen'] < len(self):
# updating the unfiltered branchmap should refresh all the others, # updating the unfiltered branchmap should refresh all the others,
self.ui.debug('updating the branch cache\n') self.ui.debug('updating the branch cache\n')
branchmap.updatecache(self.filtered('served')) branchmap.updatecache(self.filtered('served'))
if full: if full:
rbc = self.revbranchcache() rbc = self.revbranchcache()
cl = self.changelog
for r in self.changelog: for r in self.changelog:
rbc.branchinfo(r) rbc.branchinfo(r, changelog=cl)
rbc.write() rbc.write()
# ensure the working copy parents are in the manifestfulltextcache # ensure the working copy parents are in the manifestfulltextcache
for ctx in self['.'].parents(): for ctx in self['.'].parents():
ctx.manifest() # accessing the manifest is enough ctx.manifest() # accessing the manifest is enough
def invalidatecaches(self): def invalidatecaches(self):

mercurial/revset.py

""" """
All changesets belonging to the given branch or the branches of the given All changesets belonging to the given branch or the branches of the given
changesets. changesets.
Pattern matching is supported for `string`. See Pattern matching is supported for `string`. See
:hg:`help revisions.patterns`. :hg:`help revisions.patterns`.
""" """
getbi = repo.revbranchcache().branchinfo getbi = repo.revbranchcache().branchinfo
cl = repo.changelog
def getbranch(r): def getbranch(r):
try: try:
return getbi(r)[0] return getbi(r, changelog=cl)[0]
except error.WdirUnsupported: except error.WdirUnsupported:
return repo[r].branch() return repo[r].branch()
try: try:
b = getstring(x, '') b = getstring(x, '')
except error.ParseError: except error.ParseError:
# not a string, but another revspec, e.g. tip() # not a string, but another revspec, e.g. tip()
pass pass