This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8697

phases: sparsify phase lists
ClosedPublic

Authored by joerg.sonnenberger on Jul 7 2020, 6:38 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGb1e51ef4e536: phases: sparsify phase lists
Summary

When the internal and archived phase was added, allphase became a large,
sparsely populated list. This dramatically increased the number of
lookup operations for public relations in phasecache.phase. As a first
step, define allphases and related lists explicitly to contain only the
actual phases. Make phasenames a dictionary and create corresponding
dictionaries for mapping phase names back to numbers. Adjust various
list to be sparse as well with the exception of phaseroots and phasesets
members of phasecache. Keep those as a separate step as it involves
changes to the C module.

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

pulkit added a subscriber: pulkit.Jul 10 2020, 3:51 PM
pulkit added inline comments.
mercurial/phases.py
152

These all makes me think that we need a class here. What do you think?

joerg.sonnenberger added inline comments.Jul 13 2020, 11:02 AM
mercurial/phases.py
152

I'm not sure if turning the various lists into predicates or so would actually improve anything. I did change this from the original form to make it easier to reason about them, but otherwise this part is a NFC. Anything more would be much more intrusive and possibly also affect 3rd party extensions like evolve with non-trivial follow-up changes. It doesn't seem to be worth the effort.

joerg.sonnenberger added a commit: rHGb1e51ef4e536: phases: sparsify phase lists.Jul 15 2020, 12:14 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGb1e51ef4e536: phases: sparsify phase lists (authored by joerg.sonnenberger). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/bundle2.py (2 lines)
Mmercurial/exchange.py (16 lines)
Mmercurial/exchangev2.py (9 lines)
Mmercurial/phases.py (63 lines)

Diff 21918

mercurial/bundle2.py

phasetonodes = phases.binarydecode(inpart) phasetonodes = phases.binarydecode(inpart)
unfi = op.repo.unfiltered() unfi = op.repo.unfiltered()
cl = unfi.changelog cl = unfi.changelog
phasecache = unfi._phasecache phasecache = unfi._phasecache
msg = ( msg = (
b'remote repository changed while pushing - please try again ' b'remote repository changed while pushing - please try again '
b'(%s is %s expected %s)' b'(%s is %s expected %s)'
) )
for expectedphase, nodes in enumerate(phasetonodes): for expectedphase, nodes in pycompat.iteritems(phasetonodes):
for n in nodes: for n in nodes:
actualphase = phasecache.phase(unfi, cl.rev(n)) actualphase = phasecache.phase(unfi, cl.rev(n))
if actualphase != expectedphase: if actualphase != expectedphase:
finalmsg = msg % ( finalmsg = msg % (
nodemod.short(n), nodemod.short(n),
phases.phasenames[actualphase], phases.phasenames[actualphase],
phases.phasenames[expectedphase], phases.phasenames[expectedphase],
) )

mercurial/exchange.py

def _pushb2checkphases(pushop, bundler): def _pushb2checkphases(pushop, bundler):
"""insert phase move checking""" """insert phase move checking"""
if not _pushing(pushop) or pushop.force: if not _pushing(pushop) or pushop.force:
return return
b2caps = bundle2.bundle2caps(pushop.remote) b2caps = bundle2.bundle2caps(pushop.remote)
hasphaseheads = b'heads' in b2caps.get(b'phases', ()) hasphaseheads = b'heads' in b2caps.get(b'phases', ())
if pushop.remotephases is not None and hasphaseheads: if pushop.remotephases is not None and hasphaseheads:
# check that the remote phase has not changed # check that the remote phase has not changed
checks = [[] for p in phases.allphases] checks = {p: [] for p in phases.allphases}
checks[phases.public].extend(pushop.remotephases.publicheads) checks[phases.public].extend(pushop.remotephases.publicheads)
checks[phases.draft].extend(pushop.remotephases.draftroots) checks[phases.draft].extend(pushop.remotephases.draftroots)
if any(checks): if any(pycompat.itervalues(checks)):
for nodes in checks: for phase in checks:
nodes.sort() checks[phase].sort()
checkdata = phases.binaryencode(checks) checkdata = phases.binaryencode(checks)
bundler.newpart(b'check:phases', data=checkdata) bundler.newpart(b'check:phases', data=checkdata)
@b2partsgenerator(b'changeset') @b2partsgenerator(b'changeset')
def _pushb2ctx(pushop, bundler): def _pushb2ctx(pushop, bundler):
"""handle changegroup push through bundle2 """handle changegroup push through bundle2
elif haspushkey: elif haspushkey:
return _pushb2phasespushkey(pushop, bundler) return _pushb2phasespushkey(pushop, bundler)
def _pushb2phaseheads(pushop, bundler): def _pushb2phaseheads(pushop, bundler):
"""push phase information through a bundle2 - binary part""" """push phase information through a bundle2 - binary part"""
pushop.stepsdone.add(b'phases') pushop.stepsdone.add(b'phases')
if pushop.outdatedphases: if pushop.outdatedphases:
updates = [[] for p in phases.allphases] updates = {p: [] for p in phases.allphases}
updates[0].extend(h.node() for h in pushop.outdatedphases) updates[0].extend(h.node() for h in pushop.outdatedphases)
phasedata = phases.binaryencode(updates) phasedata = phases.binaryencode(updates)
bundler.newpart(b'phase-heads', data=phasedata) bundler.newpart(b'phase-heads', data=phasedata)
def _pushb2phasespushkey(pushop, bundler): def _pushb2phasespushkey(pushop, bundler):
"""push phase information through a bundle2 - pushkey part""" """push phase information through a bundle2 - pushkey part"""
pushop.stepsdone.add(b'phases') pushop.stepsdone.add(b'phases')
publicheads = headsbyphase.get(phases.public, set()) publicheads = headsbyphase.get(phases.public, set())
revset = b'heads(only(%ln, %ln) and public())' revset = b'heads(only(%ln, %ln) and public())'
extraheads = repo.revs(revset, draftheads, publicheads) extraheads = repo.revs(revset, draftheads, publicheads)
for r in extraheads: for r in extraheads:
headsbyphase[phases.public].add(node(r)) headsbyphase[phases.public].add(node(r))
# transform data in a format used by the encoding function # transform data in a format used by the encoding function
phasemapping = [] phasemapping = {
for phase in phases.allphases: phase: sorted(headsbyphase[phase]) for phase in phases.allphases
phasemapping.append(sorted(headsbyphase[phase])) }
# generate the actual part # generate the actual part
phasedata = phases.binaryencode(phasemapping) phasedata = phases.binaryencode(phasemapping)
bundler.newpart(b'phase-heads', data=phasedata) bundler.newpart(b'phase-heads', data=phasedata)
@getbundle2partsgenerator(b'hgtagsfnodes') @getbundle2partsgenerator(b'hgtagsfnodes')
def _getbundletagsfnodes( def _getbundletagsfnodes(

mercurial/exchangev2.py

# are deferred. Do those now. # are deferred. Do those now.
# Ensure all new changesets are draft by default. If the repo is # Ensure all new changesets are draft by default. If the repo is
# publishing, the phase will be adjusted by the loop below. # publishing, the phase will be adjusted by the loop below.
if csetres[b'added']: if csetres[b'added']:
phases.registernew(repo, tr, phases.draft, csetres[b'added']) phases.registernew(repo, tr, phases.draft, csetres[b'added'])
# And adjust the phase of all changesets accordingly. # And adjust the phase of all changesets accordingly.
for phase in phases.phasenames: for phasenumber, phase in phases.phasenames.items():
if phase == b'secret' or not csetres[b'nodesbyphase'][phase]: if phase == b'secret' or not csetres[b'nodesbyphase'][phase]:
continue continue
phases.advanceboundary( phases.advanceboundary(
repo, repo, tr, phasenumber, csetres[b'nodesbyphase'][phase],
tr,
phases.phasenames.index(phase),
csetres[b'nodesbyphase'][phase],
) )
# Write bookmark updates. # Write bookmark updates.
bookmarks.updatefromremote( bookmarks.updatefromremote(
repo.ui, repo.ui,
repo, repo,
csetres[b'bookmarks'], csetres[b'bookmarks'],
remote.url(), remote.url(),
progress.increment() progress.increment()
revision = cl.changelogrevision(node) revision = cl.changelogrevision(node)
# We need to preserve the mapping of changelog revision to node # We need to preserve the mapping of changelog revision to node
# so we can set the linkrev accordingly when manifests are added. # so we can set the linkrev accordingly when manifests are added.
manifestnodes[cl.rev(node)] = revision.manifest manifestnodes[cl.rev(node)] = revision.manifest
nodesbyphase = {phase: set() for phase in phases.phasenames} nodesbyphase = {phase: set() for phase in phases.phasenames.values()}
remotebookmarks = {} remotebookmarks = {}
# addgroup() expects a 7-tuple describing revisions. This normalizes # addgroup() expects a 7-tuple describing revisions. This normalizes
# the wire data to that format. # the wire data to that format.
# #
# This loop also aggregates non-revision metadata, such as phase # This loop also aggregates non-revision metadata, such as phase
# data. # data.
def iterrevisions(): def iterrevisions():

mercurial/phases.py

pycompat, pycompat,
smartset, smartset,
txnutil, txnutil,
util, util,
) )
_fphasesentry = struct.Struct(b'>i20s') _fphasesentry = struct.Struct(b'>i20s')
INTERNAL_FLAG = 64 # Phases for mercurial internal usage only
HIDEABLE_FLAG = 32 # Phases that are hideable
# record phase index # record phase index
public, draft, secret = range(3) public, draft, secret = range(3)
internal = INTERNAL_FLAG | HIDEABLE_FLAG archived = 32 # non-continuous for compatibility
archived = HIDEABLE_FLAG internal = 96 # non-continuous for compatibility
allphases = list(range(internal + 1)) allphases = (public, draft, secret, archived, internal)
trackedphases = allphases[1:] trackedphases = (draft, secret, archived, internal)
# record phase names # record phase names
cmdphasenames = [b'public', b'draft', b'secret'] # known to `hg phase` command cmdphasenames = [b'public', b'draft', b'secret'] # known to `hg phase` command
phasenames = [None] * len(allphases) phasenames = dict(enumerate(cmdphasenames))
phasenames[: len(cmdphasenames)] = cmdphasenames
phasenames[archived] = b'archived' phasenames[archived] = b'archived'
phasenames[internal] = b'internal' phasenames[internal] = b'internal'
# map phase name to phase number
phasenumber = {name: phase for phase, name in phasenames.items()}
# like phasenumber, but also include maps for the numeric and binary
# phase number to the phase number
phasenumber2 = phasenumber.copy()
phasenumber2.update({phase: phase for phase in phasenames})
phasenumber2.update({b'%i' % phase: phase for phase in phasenames})
# record phase property # record phase property
mutablephases = tuple(allphases[1:]) mutablephases = (draft, secret, archived, internal)
remotehiddenphases = tuple(allphases[2:]) remotehiddenphases = (secret, archived, internal)
localhiddenphases = tuple(p for p in allphases if p & HIDEABLE_FLAG) localhiddenphases = (internal, archived)
pulkitUnsubmitted
Not Done

These all makes me think that we need a class here. What do you think?

pulkit: These all makes me think that we need a class here. What do you think?
joerg.sonnenbergerAuthorUnsubmitted
Done

I'm not sure if turning the various lists into predicates or so would actually improve anything. I did change this from the original form to make it easier to reason about them, but otherwise this part is a NFC. Anything more would be much more intrusive and possibly also affect 3rd party extensions like evolve with non-trivial follow-up changes. It doesn't seem to be worth the effort.

joerg.sonnenberger: I'm not sure if turning the various lists into predicates or so would actually improve anything.
def supportinternal(repo): def supportinternal(repo):
"""True if the internal phase can be used on a repository""" """True if the internal phase can be used on a repository"""
return b'internal-phase' in repo.requirements return b'internal-phase' in repo.requirements
def _readroots(repo, phasedefaults=None): def _readroots(repo, phasedefaults=None):
"""Read phase roots from disk """Read phase roots from disk
phasedefaults is a list of fn(repo, roots) callable, which are phasedefaults is a list of fn(repo, roots) callable, which are
executed if the phase roots file does not exist. When phases are executed if the phase roots file does not exist. When phases are
being initialized on an existing repository, this could be used to being initialized on an existing repository, this could be used to
set selected changesets phase to something else than public. set selected changesets phase to something else than public.
Return (roots, dirty) where dirty is true if roots differ from Return (roots, dirty) where dirty is true if roots differ from
what is being stored. what is being stored.
""" """
repo = repo.unfiltered() repo = repo.unfiltered()
dirty = False dirty = False
roots = [set() for i in allphases] roots = [set() for i in range(max(allphases) + 1)]
try: try:
f, pending = txnutil.trypending(repo.root, repo.svfs, b'phaseroots') f, pending = txnutil.trypending(repo.root, repo.svfs, b'phaseroots')
try: try:
for line in f: for line in f:
phase, nh = line.split() phase, nh = line.split()
roots[int(phase)].add(bin(nh)) roots[int(phase)].add(bin(nh))
finally: finally:
f.close() f.close()
except IOError as inst: except IOError as inst:
if inst.errno != errno.ENOENT: if inst.errno != errno.ENOENT:
raise raise
if phasedefaults: if phasedefaults:
for f in phasedefaults: for f in phasedefaults:
roots = f(repo, roots) roots = f(repo, roots)
dirty = True dirty = True
return roots, dirty return roots, dirty
def binaryencode(phasemapping): def binaryencode(phasemapping):
"""encode a 'phase -> nodes' mapping into a binary stream """encode a 'phase -> nodes' mapping into a binary stream
Since phases are integer the mapping is actually a python list: The revision lists are encoded as (phase, root) pairs.
[[PUBLIC_HEADS], [DRAFTS_HEADS], [SECRET_HEADS]]
""" """
binarydata = [] binarydata = []
for phase, nodes in enumerate(phasemapping): for phase, nodes in pycompat.iteritems(phasemapping):
for head in nodes: for head in nodes:
binarydata.append(_fphasesentry.pack(phase, head)) binarydata.append(_fphasesentry.pack(phase, head))
return b''.join(binarydata) return b''.join(binarydata)
def binarydecode(stream): def binarydecode(stream):
"""decode a binary stream into a 'phase -> nodes' mapping """decode a binary stream into a 'phase -> nodes' mapping
Since phases are integer the mapping is actually a python list.""" The (phase, root) pairs are turned back into a dictionary with
headsbyphase = [[] for i in allphases] the phase as index and the aggregated roots of that phase as value."""
headsbyphase = {i: [] for i in allphases}
entrysize = _fphasesentry.size entrysize = _fphasesentry.size
while True: while True:
entry = stream.read(entrysize) entry = stream.read(entrysize)
if len(entry) < entrysize: if len(entry) < entrysize:
if entry: if entry:
raise error.Abort(_(b'bad phase-heads stream')) raise error.Abort(_(b'bad phase-heads stream'))
break break
phase, node = _fphasesentry.unpack(entry) phase, node = _fphasesentry.unpack(entry)
def _getphaserevsnative(self, repo): def _getphaserevsnative(self, repo):
repo = repo.unfiltered() repo = repo.unfiltered()
nativeroots = [] nativeroots = []
for phase in trackedphases: for phase in trackedphases:
nativeroots.append( nativeroots.append(
pycompat.maplist(repo.changelog.rev, self.phaseroots[phase]) pycompat.maplist(repo.changelog.rev, self.phaseroots[phase])
) )
return repo.changelog.computephases(nativeroots) revslen, phasesets = repo.changelog.computephases(nativeroots)
phasesets2 = [set() for phase in range(max(allphases) + 1)]
for phase, phaseset in zip(allphases, phasesets):
phasesets2[phase] = phaseset
return revslen, phasesets2
def _computephaserevspure(self, repo): def _computephaserevspure(self, repo):
repo = repo.unfiltered() repo = repo.unfiltered()
cl = repo.changelog cl = repo.changelog
self._phasesets = [set() for phase in allphases] self._phasesets = [set() for phase in range(max(allphases) + 1)]
lowerroots = set() lowerroots = set()
for phase in reversed(trackedphases): for phase in reversed(trackedphases):
roots = pycompat.maplist(cl.rev, self.phaseroots[phase]) roots = pycompat.maplist(cl.rev, self.phaseroots[phase])
if roots: if roots:
ps = set(cl.descendants(roots)) ps = set(cl.descendants(roots))
for root in roots: for root in roots:
ps.add(root) ps.add(root)
ps.difference_update(lowerroots) ps.difference_update(lowerroots)
phasetracking = None phasetracking = None
else: else:
phasetracking = tr.changes.get(b'phases') phasetracking = tr.changes.get(b'phases')
repo = repo.unfiltered() repo = repo.unfiltered()
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 pycompat.xrange(targetphase + 1, len(allphases)): 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 = [ nodes = [
n for n in nodes if self.phase(repo, repo[n].rev()) >= phase n for n in nodes if self.phase(repo, repo[n].rev()) >= phase
] ]
if not nodes: if not nodes:
break # no roots to move anymore break # no roots to move anymore
olds = self.phaseroots[phase] olds = self.phaseroots[phase]
def subsetphaseheads(repo, subset): def subsetphaseheads(repo, subset):
"""Finds the phase heads for a subset of a history """Finds the phase heads for a subset of a history
Returns a list indexed by phase number where each item is a list of phase Returns a list indexed by phase number where each item is a list of phase
head nodes. head nodes.
""" """
cl = repo.changelog cl = repo.changelog
headsbyphase = [[] for i in allphases] headsbyphase = {i: [] for i in allphases}
# No need to keep track of secret phase; any heads in the subset that # No need to keep track of secret phase; any heads in the subset that
# are not mentioned are implicitly secret. # are not mentioned are implicitly secret.
for phase in allphases[:secret]: for phase in allphases[:secret]:
revset = b"heads(%%ln & %s())" % phasenames[phase] revset = b"heads(%%ln & %s())" % phasenames[phase]
headsbyphase[phase] = [cl.node(r) for r in repo.revs(revset, subset)] headsbyphase[phase] = [cl.node(r) for r in repo.revs(revset, subset)]
return headsbyphase return headsbyphase
def newcommitphase(ui): def newcommitphase(ui):
"""helper to get the target phase of new commit """helper to get the target phase of new commit
Handle all possible values for the phases.new-commit options. Handle all possible values for the phases.new-commit options.
""" """
v = ui.config(b'phases', b'new-commit') v = ui.config(b'phases', b'new-commit')
try: try:
return phasenames.index(v) return phasenumber2[v]
except ValueError: except KeyError:
try: raise error.ConfigError(
return int(v) _(b"phases.new-commit: not a valid phase name ('%s')") % v
except ValueError: )
msg = _(b"phases.new-commit: not a valid phase name ('%s')")
raise error.ConfigError(msg % v)
def hassecret(repo): def hassecret(repo):
"""utility function that check if a repo have any secret changeset.""" """utility function that check if a repo have any secret changeset."""
return bool(repo._phasecache.phaseroots[secret]) return bool(repo._phasecache.phaseroots[secret])
def preparehookargs(node, old, new): def preparehookargs(node, old, new):
if old is None: if old is None:
old = b'' old = b''
else: else:
old = phasenames[old] old = phasenames[old]
return {b'node': node, b'oldphase': old, b'phase': phasenames[new]} return {b'node': node, b'oldphase': old, b'phase': phasenames[new]}