This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5161

branchmap: pass changelog into branchmap functions
ClosedPublic

Authored by indygreg on Oct 19 2018, 12:56 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG47c03042cd1d: branchmap: pass changelog into branchmap functions
Summary

As part of building the branchmap, we loop over revs and call branchmap()
or _branchmap(). Previously, these functions were accessing repo.changelog.
We know from past experience that repo.changelog in loops is bad for
performance.

This commit teaches the branchmap code to pass a changelog instance into
branchmap() and _branchmap() so we don't need to pay this penalty.

On my MBP, this appears to show a speedup on a clone of the
mozilla-unified repo:

$ hg perfbranchmap --clear-revbranch
! base
! wall 21.078160 comb 21.070000 user 20.920000 sys 0.150000 (best of 3)
! wall 20.574682 comb 20.560000 user 20.400000 sys 0.160000 (best of 3)

$ hg perfbranchmap
! base
! wall 4.880413 comb 4.870000 user 4.860000 sys 0.010000 (best of 3)
! wall 4.573968 comb 4.560000 user 4.550000 sys 0.010000 (best of 3)

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Oct 19 2018, 12:56 PM
pulkit accepted this revision.Oct 19 2018, 1:08 PM
pulkit added inline comments.Oct 19 2018, 1:14 PM
mercurial/branchmap.py
407

There is one more call of this function in revset.py:491 which is also inside a for-loop. Doing this same thing there will speed up the branch() revset. If possible, can you have a look before the freeze?

pulkit added inline comments.Oct 19 2018, 1:15 PM
mercurial/branchmap.py
407

Oh, and one more in bundle2.py:1650 which is also inside a for loop!

pulkit added a comment.Oct 19 2018, 1:19 PM

Since these two patches are simple but great speedups, I am queueing them and also I will be sending patches to fix this at other places too if nobody beats me to it.

Closed by commit rHG47c03042cd1d: branchmap: pass changelog into branchmap functions (authored by indygreg). · Explain WhyOct 19 2018, 1:20 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/branchmap.py (22 lines)

Diff 12267

mercurial/branchmap.py

heads missing, this function updates self to be correct. heads missing, this function updates self to be correct.
""" """
starttime = util.timer() starttime = util.timer()
cl = repo.changelog cl = repo.changelog
# collect new branch entries # collect new branch entries
newbranches = {} newbranches = {}
getbranchinfo = repo.revbranchcache().branchinfo getbranchinfo = repo.revbranchcache().branchinfo
for r in revgen: for r in revgen:
branch, closesbranch = getbranchinfo(r) branch, closesbranch = getbranchinfo(r, changelog=cl)
newbranches.setdefault(branch, []).append(r) newbranches.setdefault(branch, []).append(r)
if closesbranch: if closesbranch:
self._closednodes.add(cl.node(r)) self._closednodes.add(cl.node(r))
# fetch current topological heads to speed up filtering # fetch current topological heads to speed up filtering
topoheads = set(cl.headrevs()) topoheads = set(cl.headrevs())
# if older branchheads are reachable from new ones, they aren't # if older branchheads are reachable from new ones, they aren't
def _clear(self): def _clear(self):
self._rbcsnameslen = 0 self._rbcsnameslen = 0
del self._names[:] del self._names[:]
self._rbcnamescount = 0 self._rbcnamescount = 0
self._namesreverse.clear() self._namesreverse.clear()
self._rbcrevslen = len(self._repo.changelog) self._rbcrevslen = len(self._repo.changelog)
self._rbcrevs = bytearray(self._rbcrevslen * _rbcrecsize) self._rbcrevs = bytearray(self._rbcrevslen * _rbcrecsize)
def branchinfo(self, rev): def branchinfo(self, rev, changelog=None):
pulkitUnsubmitted
Not Done

There is one more call of this function in revset.py:491 which is also inside a for-loop. Doing this same thing there will speed up the branch() revset. If possible, can you have a look before the freeze?

pulkit: There is one more call of this function in revset.py:491 which is also inside a for-loop. Doing…
pulkitUnsubmitted
Not Done

Oh, and one more in bundle2.py:1650 which is also inside a for loop!

pulkit: Oh, and one more in bundle2.py:1650 which is also inside a for loop!
"""Return branch name and close flag for rev, using and updating """Return branch name and close flag for rev, using and updating
persistent cache.""" persistent cache."""
changelog = self._repo.changelog changelog = changelog or self._repo.changelog
rbcrevidx = rev * _rbcrecsize rbcrevidx = rev * _rbcrecsize
# avoid negative index, changelog.read(nullrev) is fast without cache # avoid negative index, changelog.read(nullrev) is fast without cache
if rev == nullrev: if rev == nullrev:
return changelog.branchinfo(rev) return changelog.branchinfo(rev)
# if requested rev isn't allocated, grow and cache the rev info # if requested rev isn't allocated, grow and cache the rev info
if len(self._rbcrevs) < rbcrevidx + _rbcrecsize: if len(self._rbcrevs) < rbcrevidx + _rbcrecsize:
return self._branchinfo(rev) return self._branchinfo(rev, changelog=changelog)
# fast path: extract data from cache, use it if node is matching # fast path: extract data from cache, use it if node is matching
reponode = changelog.node(rev)[:_rbcnodelen] reponode = changelog.node(rev)[:_rbcnodelen]
cachenode, branchidx = unpack_from( cachenode, branchidx = unpack_from(
_rbcrecfmt, util.buffer(self._rbcrevs), rbcrevidx) _rbcrecfmt, util.buffer(self._rbcrevs), rbcrevidx)
close = bool(branchidx & _rbccloseflag) close = bool(branchidx & _rbccloseflag)
if close: if close:
branchidx &= _rbcbranchidxmask branchidx &= _rbcbranchidxmask
# rev/node map has changed, invalidate the cache from here up # rev/node map has changed, invalidate the cache from here up
self._repo.ui.debug("history modification detected - truncating " self._repo.ui.debug("history modification detected - truncating "
"revision branch cache to revision %d\n" % rev) "revision branch cache to revision %d\n" % rev)
truncate = rbcrevidx + _rbcrecsize truncate = rbcrevidx + _rbcrecsize
del self._rbcrevs[truncate:] del self._rbcrevs[truncate:]
self._rbcrevslen = min(self._rbcrevslen, truncate) self._rbcrevslen = min(self._rbcrevslen, truncate)
# fall back to slow path and make sure it will be written to disk # fall back to slow path and make sure it will be written to disk
return self._branchinfo(rev) return self._branchinfo(rev, changelog=changelog)
def _branchinfo(self, rev): def _branchinfo(self, rev, changelog=None):
"""Retrieve branch info from changelog and update _rbcrevs""" """Retrieve branch info from changelog and update _rbcrevs"""
changelog = self._repo.changelog changelog = changelog or self._repo.changelog
b, close = changelog.branchinfo(rev) b, close = changelog.branchinfo(rev)
if b in self._namesreverse: if b in self._namesreverse:
branchidx = self._namesreverse[b] branchidx = self._namesreverse[b]
else: else:
branchidx = len(self._names) branchidx = len(self._names)
self._names.append(b) self._names.append(b)
self._namesreverse[b] = branchidx self._namesreverse[b] = branchidx
reponode = changelog.node(rev) reponode = changelog.node(rev)
if close: if close:
branchidx |= _rbccloseflag branchidx |= _rbccloseflag
self._setcachedata(rev, reponode, branchidx) self._setcachedata(rev, reponode, branchidx, changelog)
return b, close return b, close
def setdata(self, branch, rev, node, close): def setdata(self, branch, rev, node, close):
"""add new data information to the cache""" """add new data information to the cache"""
if branch in self._namesreverse: if branch in self._namesreverse:
branchidx = self._namesreverse[branch] branchidx = self._namesreverse[branch]
else: else:
branchidx = len(self._names) branchidx = len(self._names)
self._names.append(branch) self._names.append(branch)
self._namesreverse[branch] = branchidx self._namesreverse[branch] = branchidx
if close: if close:
branchidx |= _rbccloseflag branchidx |= _rbccloseflag
self._setcachedata(rev, node, branchidx) self._setcachedata(rev, node, branchidx)
# If no cache data were readable (non exists, bad permission, etc) # If no cache data were readable (non exists, bad permission, etc)
# the cache was bypassing itself by setting: # the cache was bypassing itself by setting:
# #
# self.branchinfo = self._branchinfo # self.branchinfo = self._branchinfo
# #
# Since we now have data in the cache, we need to drop this bypassing. # Since we now have data in the cache, we need to drop this bypassing.
if r'branchinfo' in vars(self): if r'branchinfo' in vars(self):
del self.branchinfo del self.branchinfo
def _setcachedata(self, rev, node, branchidx): def _setcachedata(self, rev, node, branchidx, changelog=None):
"""Writes the node's branch data to the in-memory cache data.""" """Writes the node's branch data to the in-memory cache data."""
if rev == nullrev: if rev == nullrev:
return return
changelog = changelog or self._repo.changelog
rbcrevidx = rev * _rbcrecsize rbcrevidx = rev * _rbcrecsize
if len(self._rbcrevs) < rbcrevidx + _rbcrecsize: if len(self._rbcrevs) < rbcrevidx + _rbcrecsize:
self._rbcrevs.extend('\0' * self._rbcrevs.extend('\0' *
(len(self._repo.changelog) * _rbcrecsize - (len(changelog) * _rbcrecsize -
len(self._rbcrevs))) len(self._rbcrevs)))
pack_into(_rbcrecfmt, self._rbcrevs, rbcrevidx, node, branchidx) pack_into(_rbcrecfmt, self._rbcrevs, rbcrevidx, node, branchidx)
self._rbcrevslen = min(self._rbcrevslen, rev) self._rbcrevslen = min(self._rbcrevslen, rev)
tr = self._repo.currenttransaction() tr = self._repo.currenttransaction()
if tr: if tr:
tr.addfinalize('write-revbranchcache', self.write) tr.addfinalize('write-revbranchcache', self.write)