This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9232

phases: allow registration and boundary advancement with revision sets
ClosedPublic

Authored by joerg.sonnenberger on Oct 20 2020, 11:33 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG09735cde6275: phases: allow registration and boundary advancement with revision sets
Summary

The core internals either use revision sets already or can trivially use
them. Use the new interface in cg1unpacker.apply to avoid materializing
the list of all new nodes as it is normally just a revision range. This
avoids about 67 Bytes / changeset on AMD64 in peak RSS.

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

mharbison72 added a subscriber: mharbison72.Oct 22 2020, 2:29 PM
mharbison72 added inline comments.
mercurial/phases.py
513–515

Doesn't this sort of thing get flagged by pyflakes or whatever for being a mutable argument? I thought I remembered a series that took out stuff like this, defaulting to None and creating an empty list locally if needed. It looks like there are a few other cases below.

joerg.sonnenberger added inline comments.Oct 22 2020, 3:49 PM
mercurial/phases.py
513–515

It doesn't flag it here. I'm not sure if it detects that revs is not mutated.

joerg.sonnenberger retitled this revision from phases: allow registration and boundary advancement with revsion sets to phases: allow registration and boundary advancement with revision sets.Oct 23 2020, 2:34 PM
joerg.sonnenberger updated this revision to Diff 23298.
joerg.sonnenberger added a commit: rHG09735cde6275: phases: allow registration and boundary advancement with revision sets.Oct 28 2020, 10:00 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG09735cde6275: phases: allow registration and boundary advancement with revision sets (authored by joerg.sonnenberger). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/changegroup.py (21 lines)
Mmercurial/phases.py (41 lines)

Diff 23343

mercurial/changegroup.py

progress = repo.ui.makeprogress( progress = repo.ui.makeprogress(
_(b'changesets'), unit=_(b'chunks'), total=expectedtotal _(b'changesets'), unit=_(b'chunks'), total=expectedtotal
) )
self.callback = progress.increment self.callback = progress.increment
efilesset = set() efilesset = set()
cgnodes = [] cgnodes = []
def onchangelog(cl, node):
efilesset.update(cl.readfiles(node))
cgnodes.append(node)
def ondupchangelog(cl, node): def ondupchangelog(cl, node):
if cl.rev(node) < clstart:
cgnodes.append(node) cgnodes.append(node)
def onchangelog(cl, node):
efilesset.update(cl.readfiles(node))
self.changelogheader() self.changelogheader()
deltas = self.deltaiter() deltas = self.deltaiter()
if not cl.addgroup( if not cl.addgroup(
deltas, deltas,
csmap, csmap,
trp, trp,
addrevisioncb=onchangelog, addrevisioncb=onchangelog,
duplicaterevisioncb=ondupchangelog, duplicaterevisioncb=ondupchangelog,
needfiles = {} needfiles = {}
if repo.ui.configbool(b'server', b'validate'): if repo.ui.configbool(b'server', b'validate'):
cl = repo.changelog cl = repo.changelog
ml = repo.manifestlog ml = repo.manifestlog
# validate incoming csets have their manifests # validate incoming csets have their manifests
for cset in pycompat.xrange(clstart, clend): for cset in pycompat.xrange(clstart, clend):
mfnode = cl.changelogrevision(cset).manifest mfnode = cl.changelogrevision(cset).manifest
mfest = ml[mfnode].readdelta() mfest = ml[mfnode].readdelta()
# store file cgnodes we must see # store file nodes we must see
for f, n in pycompat.iteritems(mfest): for f, n in pycompat.iteritems(mfest):
needfiles.setdefault(f, set()).add(n) needfiles.setdefault(f, set()).add(n)
# process the files # process the files
repo.ui.status(_(b"adding file changes\n")) repo.ui.status(_(b"adding file changes\n"))
newrevs, newfiles = _addchangegroupfiles( newrevs, newfiles = _addchangegroupfiles(
repo, self, revmap, trp, efiles, needfiles repo, self, revmap, trp, efiles, needfiles
) )
hookargs[b'node'] = hex(cl.node(clstart)) hookargs[b'node'] = hex(cl.node(clstart))
hookargs[b'node_last'] = hex(cl.node(clend - 1)) hookargs[b'node_last'] = hex(cl.node(clend - 1))
repo.hook( repo.hook(
b'pretxnchangegroup', b'pretxnchangegroup',
throw=True, throw=True,
**pycompat.strkwargs(hookargs) **pycompat.strkwargs(hookargs)
) )
added = [cl.node(r) for r in pycompat.xrange(clstart, clend)] added = pycompat.xrange(clstart, clend)
phaseall = None phaseall = None
if srctype in (b'push', b'serve'): if srctype in (b'push', b'serve'):
# Old servers can not push the boundary themselves. # Old servers can not push the boundary themselves.
# New servers won't push the boundary if changeset already # New servers won't push the boundary if changeset already
# exists locally as secret # exists locally as secret
# #
# We should not use added here but the list of all change in # We should not use added here but the list of all change in
# the bundle # the bundle
if repo.publishing(): if repo.publishing():
targetphase = phaseall = phases.public targetphase = phaseall = phases.public
else: else:
# closer target phase computation # closer target phase computation
# Those changesets have been pushed from the # Those changesets have been pushed from the
# outside, their phases are going to be pushed # outside, their phases are going to be pushed
# alongside. Therefor `targetphase` is # alongside. Therefor `targetphase` is
# ignored. # ignored.
targetphase = phaseall = phases.draft targetphase = phaseall = phases.draft
if added: if added:
phases.registernew(repo, tr, targetphase, added) phases.registernew(repo, tr, targetphase, [], revs=added)
if phaseall is not None: if phaseall is not None:
phases.advanceboundary(repo, tr, phaseall, cgnodes) phases.advanceboundary(repo, tr, phaseall, cgnodes, revs=added)
cgnodes = []
if changesets > 0: if changesets > 0:
def runhooks(unused_success): def runhooks(unused_success):
# These hooks run when the lock releases, not when the # These hooks run when the lock releases, not when the
# transaction closes. So it's possible for the changelog # transaction closes. So it's possible for the changelog
# to have changed since we last saw it. # to have changed since we last saw it.
if clstart >= len(repo): if clstart >= len(repo):
return return
repo.hook(b"changegroup", **pycompat.strkwargs(hookargs)) repo.hook(b"changegroup", **pycompat.strkwargs(hookargs))
for n in added: for rev in added:
args = hookargs.copy() args = hookargs.copy()
args[b'node'] = hex(n) args[b'node'] = hex(cl.node(rev))
del args[b'node_last'] del args[b'node_last']
repo.hook(b"incoming", **pycompat.strkwargs(args)) repo.hook(b"incoming", **pycompat.strkwargs(args))
newheads = [h for h in repo.heads() if h not in oldheads] newheads = [h for h in repo.heads() if h not in oldheads]
repo.ui.log( repo.ui.log(
b"incoming", b"incoming",
b"%d incoming changes - new heads: %s\n", b"%d incoming changes - new heads: %s\n",
len(added), len(added),

mercurial/phases.py

def _updateroots(self, phase, newroots, tr): def _updateroots(self, phase, newroots, tr):
self.phaseroots[phase] = newroots self.phaseroots[phase] = newroots
self.invalidate() self.invalidate()
self.dirty = True self.dirty = True
tr.addfilegenerator(b'phase', (b'phaseroots',), self._write) tr.addfilegenerator(b'phase', (b'phaseroots',), self._write)
tr.hookargs[b'phases_moved'] = b'1' tr.hookargs[b'phases_moved'] = b'1'
def registernew(self, repo, tr, targetphase, nodes): def registernew(self, repo, tr, targetphase, nodes, revs=None):
if revs is None:
revs = []
mharbison72Unsubmitted
Not Done

Doesn't this sort of thing get flagged by pyflakes or whatever for being a mutable argument? I thought I remembered a series that took out stuff like this, defaulting to None and creating an empty list locally if needed. It looks like there are a few other cases below.

mharbison72: Doesn't this sort of thing get flagged by pyflakes or whatever for being a mutable argument? I…
joerg.sonnenbergerAuthorUnsubmitted
Not Done

It doesn't flag it here. I'm not sure if it detects that revs is not mutated.

joerg.sonnenberger: It doesn't flag it here. I'm not sure if it detects that `revs` is not mutated.
repo = repo.unfiltered() repo = repo.unfiltered()
self._retractboundary(repo, tr, targetphase, nodes) self._retractboundary(repo, tr, targetphase, nodes, revs=revs)
if tr is not None and b'phases' in tr.changes: if tr is not None and b'phases' in tr.changes:
phasetracking = tr.changes[b'phases'] phasetracking = tr.changes[b'phases']
torev = repo.changelog.rev torev = repo.changelog.rev
phase = self.phase phase = self.phase
revs = [torev(node) for node in nodes] revs = [torev(node) for node in nodes] + sorted(revs)
revs.sort() revs.sort()
for rev in revs: for rev in revs:
revphase = phase(repo, rev) revphase = phase(repo, rev)
_trackphasechange(phasetracking, rev, None, revphase) _trackphasechange(phasetracking, rev, None, revphase)
repo.invalidatevolatilesets() repo.invalidatevolatilesets()
def advanceboundary(self, repo, tr, targetphase, nodes, dryrun=None): def advanceboundary(
self, repo, tr, targetphase, nodes, revs=None, dryrun=None
):
"""Set all 'nodes' to phase 'targetphase' """Set all 'nodes' to phase 'targetphase'
Nodes with a phase lower than 'targetphase' are not affected. Nodes with a phase lower than 'targetphase' are not affected.
If dryrun is True, no actions will be performed If dryrun is True, no actions will be performed
Returns a set of revs whose phase is changed or should be changed Returns a set of revs whose phase is changed or should be changed
""" """
# Be careful to preserve shallow-copied values: do not update # Be careful to preserve shallow-copied values: do not update
# phaseroots values, replace them. # phaseroots values, replace them.
if revs is None:
revs = []
if tr is None: if tr is None:
phasetracking = None phasetracking = None
else: else:
phasetracking = tr.changes.get(b'phases') phasetracking = tr.changes.get(b'phases')
repo = repo.unfiltered() repo = repo.unfiltered()
revs = [repo[n].rev() for n in nodes] + [r for r in revs]
changes = set() # set of revisions to be changed changes = set() # set of revisions to be changed
delroots = [] # set of root deleted by this path delroots = [] # set of root deleted by this path
for phase in (phase for phase in allphases if phase > targetphase): for phase in (phase for phase in allphases if phase > targetphase):
# filter nodes that are not in a compatible phase already # filter nodes that are not in a compatible phase already
nodes = [ revs = [rev for rev in revs if self.phase(repo, rev) >= phase]
n for n in nodes if self.phase(repo, repo[n].rev()) >= phase if not revs:
]
if not nodes:
break # no roots to move anymore break # no roots to move anymore
olds = self.phaseroots[phase] olds = self.phaseroots[phase]
affected = repo.revs(b'%ln::%ln', olds, nodes) affected = repo.revs(b'%ln::%ld', olds, revs)
changes.update(affected) changes.update(affected)
if dryrun: if dryrun:
continue continue
for r in affected: for r in affected:
_trackphasechange( _trackphasechange(
phasetracking, r, self.phase(repo, r), targetphase phasetracking, r, self.phase(repo, r), targetphase
) )
revs = set(repo.revs(b'%ln::%ld', roots, affected)) revs = set(repo.revs(b'%ln::%ld', roots, affected))
affected -= revs affected -= revs
else: # public phase else: # public phase
revs = affected revs = affected
for r in sorted(revs): for r in sorted(revs):
_trackphasechange(phasetracking, r, phase, targetphase) _trackphasechange(phasetracking, r, phase, targetphase)
repo.invalidatevolatilesets() repo.invalidatevolatilesets()
def _retractboundary(self, repo, tr, targetphase, nodes): def _retractboundary(self, repo, tr, targetphase, nodes, revs=None):
# Be careful to preserve shallow-copied values: do not update # Be careful to preserve shallow-copied values: do not update
# phaseroots values, replace them. # phaseroots values, replace them.
if revs is None:
revs = []
if targetphase in (archived, internal) and not supportinternal(repo): if targetphase in (archived, internal) and not supportinternal(repo):
name = phasenames[targetphase] name = phasenames[targetphase]
msg = b'this repository does not support the %s phase' % name msg = b'this repository does not support the %s phase' % name
raise error.ProgrammingError(msg) raise error.ProgrammingError(msg)
repo = repo.unfiltered() repo = repo.unfiltered()
torev = repo.changelog.rev torev = repo.changelog.rev
tonode = repo.changelog.node tonode = repo.changelog.node
currentroots = {torev(node) for node in self.phaseroots[targetphase]} currentroots = {torev(node) for node in self.phaseroots[targetphase]}
finalroots = oldroots = set(currentroots) finalroots = oldroots = set(currentroots)
newroots = [torev(node) for node in nodes] newroots = [torev(node) for node in nodes] + [r for r in revs]
newroots = [ newroots = [
rev for rev in newroots if self.phase(repo, rev) < targetphase rev for rev in newroots if self.phase(repo, rev) < targetphase
] ]
if newroots: if newroots:
if nullrev in newroots: if nullrev in newroots:
raise error.Abort(_(b'cannot change null revision phase')) raise error.Abort(_(b'cannot change null revision phase'))
currentroots.update(newroots) currentroots.update(newroots)
# #
# The other caller is __init__ that have no _phasesets initialized # The other caller is __init__ that have no _phasesets initialized
# anyway. If this change we should consider adding a dedicated # anyway. If this change we should consider adding a dedicated
# "destroyed" function to phasecache or a proper cache key mechanism # "destroyed" function to phasecache or a proper cache key mechanism
# (see branchmap one) # (see branchmap one)
self.invalidate() self.invalidate()
def advanceboundary(repo, tr, targetphase, nodes, dryrun=None): def advanceboundary(repo, tr, targetphase, nodes, revs=None, dryrun=None):
"""Add nodes to a phase changing other nodes phases if necessary. """Add nodes to a phase changing other nodes phases if necessary.
This function move boundary *forward* this means that all nodes This function move boundary *forward* this means that all nodes
are set in the target phase or kept in a *lower* phase. are set in the target phase or kept in a *lower* phase.
Simplify boundary to contains phase roots only. Simplify boundary to contains phase roots only.
If dryrun is True, no actions will be performed If dryrun is True, no actions will be performed
Returns a set of revs whose phase is changed or should be changed Returns a set of revs whose phase is changed or should be changed
""" """
if revs is None:
revs = []
phcache = repo._phasecache.copy() phcache = repo._phasecache.copy()
changes = phcache.advanceboundary( changes = phcache.advanceboundary(
repo, tr, targetphase, nodes, dryrun=dryrun repo, tr, targetphase, nodes, revs=revs, dryrun=dryrun
) )
if not dryrun: if not dryrun:
repo._phasecache.replace(phcache) repo._phasecache.replace(phcache)
return changes return changes
def retractboundary(repo, tr, targetphase, nodes): def retractboundary(repo, tr, targetphase, nodes):
"""Set nodes back to a phase changing other nodes phases if """Set nodes back to a phase changing other nodes phases if
necessary. necessary.
This function move boundary *backward* this means that all nodes This function move boundary *backward* this means that all nodes
are set in the target phase or kept in a *higher* phase. are set in the target phase or kept in a *higher* phase.
Simplify boundary to contains phase roots only.""" Simplify boundary to contains phase roots only."""
phcache = repo._phasecache.copy() phcache = repo._phasecache.copy()
phcache.retractboundary(repo, tr, targetphase, nodes) phcache.retractboundary(repo, tr, targetphase, nodes)
repo._phasecache.replace(phcache) repo._phasecache.replace(phcache)
def registernew(repo, tr, targetphase, nodes): def registernew(repo, tr, targetphase, nodes, revs=None):
"""register a new revision and its phase """register a new revision and its phase
Code adding revisions to the repository should use this function to Code adding revisions to the repository should use this function to
set new changeset in their target phase (or higher). set new changeset in their target phase (or higher).
""" """
if revs is None:
revs = []
phcache = repo._phasecache.copy() phcache = repo._phasecache.copy()
phcache.registernew(repo, tr, targetphase, nodes) phcache.registernew(repo, tr, targetphase, nodes, revs=revs)
repo._phasecache.replace(phcache) repo._phasecache.replace(phcache)
def listphases(repo): def listphases(repo):
"""List phases root for serialization over pushkey""" """List phases root for serialization over pushkey"""
# Use ordered dictionary so behavior is deterministic. # Use ordered dictionary so behavior is deterministic.
keys = util.sortdict() keys = util.sortdict()
value = b'%i' % draft value = b'%i' % draft