This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5636

branchmap: updating triggers a write
ClosedPublic

Authored by mjpieters on Jan 21 2019, 1:08 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGeb7ce452e0fb: branchmap: updating triggers a write
Summary

Rather than separate updating and writing, create a subclass that doesn't write
on update. This minimises chances we forget to write out updates somewhere.

This also makes refactoring and improving the branchmap functionality easier.

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

mjpieters created this revision.Jan 21 2019, 1:08 PM
martinvonz added a subscriber: martinvonz.Jan 25 2019, 3:43 PM
martinvonz added inline comments.
mercurial/discovery.py
242–243

Indentation seems bad (seemed bad before too)

mjpieters added inline comments.Jan 26 2019, 8:19 AM
mercurial/discovery.py
242–243

It's a generator expression as single argument. I was deliberately not making style changes in these patches, so I didn't touch the indentation here.

If / when we auto-format code with black or something else, this'll be cleared up then, right?

pulkit accepted this revision.Feb 5 2019, 11:30 AM
Closed by commit rHGeb7ce452e0fb: branchmap: updating triggers a write (authored by mjpieters). · Explain WhyFeb 5 2019, 11:45 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/branchmap.py (15 lines)
Mmercurial/discovery.py (2 lines)

Diff 13807

mercurial/branchmap.py

else: else:
subset = repo.filtered(subsetname) subset = repo.filtered(subsetname)
bcache = subset.branchmap().copy() bcache = subset.branchmap().copy()
extrarevs = subset.changelog.filteredrevs - cl.filteredrevs extrarevs = subset.changelog.filteredrevs - cl.filteredrevs
revs.extend(r for r in extrarevs if r <= bcache.tiprev) revs.extend(r for r in extrarevs if r <= bcache.tiprev)
revs.extend(cl.revs(start=bcache.tiprev + 1)) revs.extend(cl.revs(start=bcache.tiprev + 1))
if revs: if revs:
bcache.update(repo, revs) bcache.update(repo, revs)
bcache.write(repo)
assert bcache.validfor(repo), filtername assert bcache.validfor(repo), filtername
repo._branchcaches[repo.filtername] = bcache repo._branchcaches[repo.filtername] = bcache
def replacecache(repo, bm): def replacecache(repo, bm):
"""Replace the branchmap cache for a repo with a branch mapping. """Replace the branchmap cache for a repo with a branch mapping.
This is likely only called during clone with a branch map from a remote. This is likely only called during clone with a branch map from a remote.
return heads return heads
def iterbranches(self): def iterbranches(self):
for bn, heads in self.iteritems(): for bn, heads in self.iteritems():
yield (bn, heads) + self._branchtip(heads) yield (bn, heads) + self._branchtip(heads)
def copy(self): def copy(self):
"""return an deep copy of the branchcache object""" """return an deep copy of the branchcache object"""
return branchcache(self, self.tipnode, self.tiprev, self.filteredhash, return type(self)(
self, self.tipnode, self.tiprev, self.filteredhash,
self._closednodes) self._closednodes)
def write(self, repo): def write(self, repo):
try: try:
f = repo.cachevfs(self._filename(repo), "w", atomictemp=True) f = repo.cachevfs(self._filename(repo), "w", atomictemp=True)
cachekey = [hex(self.tipnode), '%d' % self.tiprev] cachekey = [hex(self.tipnode), '%d' % self.tiprev]
if self.filteredhash is not None: if self.filteredhash is not None:
cachekey.append(hex(self.filteredhash)) cachekey.append(hex(self.filteredhash))
f.write(" ".join(cachekey) + '\n') f.write(" ".join(cachekey) + '\n')
self.tipnode = cl.node(tiprev) self.tipnode = cl.node(tiprev)
self.tiprev = tiprev self.tiprev = tiprev
self.filteredhash = scmutil.filteredhash(repo, self.tiprev) self.filteredhash = scmutil.filteredhash(repo, self.tiprev)
duration = util.timer() - starttime duration = util.timer() - starttime
repo.ui.log('branchcache', 'updated %s branch cache in %.4f seconds\n', repo.ui.log('branchcache', 'updated %s branch cache in %.4f seconds\n',
repo.filtername, duration) repo.filtername, duration)
self.write(repo)
class remotebranchcache(branchcache):
"""Branchmap info for a remote connection, should not write locally"""
def write(self, repo):
pass
# Revision branch info cache # Revision branch info cache
_rbcversion = '-v1' _rbcversion = '-v1'
_rbcnames = 'rbc-names' + _rbcversion _rbcnames = 'rbc-names' + _rbcversion
_rbcrevs = 'rbc-revs' + _rbcversion _rbcrevs = 'rbc-revs' + _rbcversion
# [4 byte hash prefix][4 byte branch name number with sign bit indicating open] # [4 byte hash prefix][4 byte branch name number with sign bit indicating open]
_rbcrecfmt = '>4sI' _rbcrecfmt = '>4sI'
_rbcrecsize = calcsize(_rbcrecfmt) _rbcrecsize = calcsize(_rbcrecfmt)

mercurial/discovery.py

headssum[branch] = (None, [], []) headssum[branch] = (None, [], [])
# C drop data about untouched branches: # C drop data about untouched branches:
for branch in remotebranches - touchedbranches: for branch in remotebranches - touchedbranches:
del headssum[branch] del headssum[branch]
# D. Update newmap with outgoing changes. # D. Update newmap with outgoing changes.
# This will possibly add new heads and remove existing ones. # This will possibly add new heads and remove existing ones.
newmap = branchmap.branchcache((branch, heads[1]) newmap = branchmap.remotebranchcache((branch, heads[1])
for branch, heads in headssum.iteritems() for branch, heads in headssum.iteritems()
if heads[0] is not None) if heads[0] is not None)
martinvonzUnsubmitted
Not Done

Indentation seems bad (seemed bad before too)

martinvonz: Indentation seems bad (seemed bad before too)
mjpietersAuthorUnsubmitted
Not Done

It's a generator expression as single argument. I was deliberately not making style changes in these patches, so I didn't touch the indentation here.

If / when we auto-format code with black or something else, this'll be cleared up then, right?

mjpieters: It's a generator expression as single argument. I was deliberately not making style changes in…
newmap.update(repo, (ctx.rev() for ctx in missingctx)) newmap.update(repo, (ctx.rev() for ctx in missingctx))
for branch, newheads in newmap.iteritems(): for branch, newheads in newmap.iteritems():
headssum[branch][1][:] = newheads headssum[branch][1][:] = newheads
for branch, items in headssum.iteritems(): for branch, items in headssum.iteritems():
for l in items: for l in items:
if l is not None: if l is not None:
l.sort() l.sort()
headssum[branch] = items + ([],) headssum[branch] = items + ([],)