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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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 12276

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)