This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8125

transactions: convert changes['phases'] to list of ranges
ClosedPublic

Authored by joerg.sonnenberger on Feb 15 2020, 4:31 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHGfdc802f29b2c: transactions: convert changes['phases'] to list of ranges
rHGc74bab41d1c5: transactions: convert changes['phases'] to list of ranges
Summary

Consecutive revisions are often in the same phase, especially public
revisions. This means that a dictionary keyed by the revision for the
phase transitions is highly redundant. Build a list of (range, (old,
new)) entries instead and aggressively merge ranges with the same
transition. For the test case in issue5691, this reduces memory use by
~20MB.

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

marmoute added a subscriber: marmoute.Feb 17 2020, 5:49 PM

I've a couple of high level question:

  • This reduce the memory usage of 20MB compare to what total usage ?
  • Did you asses the performance impact of this? and if so, what is it?
joerg.sonnenberger added a comment.Feb 17 2020, 6:28 PM

I haven't measured run-time impact. The sorting should ensure that the lists are normally kept small, but when many updates apply to very fragmented repositories, it could be worse. 20MB is relative to the 600MB peak RSS from the referenced issue.

durin42 requested changes to this revision.Mar 3 2020, 1:30 PM
durin42 added a subscriber: durin42.

Overall seems fine, some structural nits on the code.

mercurial/phases.py
275

It doesn't look like there's a good reason for this function to be nested like this. Can we move it to module-level with an underscore prefix?

295

here too

This revision now requires changes to proceed.Mar 3 2020, 1:30 PM
joerg.sonnenberger added a commit: rHGc74bab41d1c5: transactions: convert changes['phases'] to list of ranges.Mar 11 2020, 3:16 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGc74bab41d1c5: transactions: convert changes['phases'] to list of ranges (authored by joerg.sonnenberger). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 20748

mercurial/localrepo.py

args.update(bookmarks.preparehookargs(name, old, new)) args.update(bookmarks.preparehookargs(name, old, new))
repo.hook( repo.hook(
b'pretxnclose-bookmark', b'pretxnclose-bookmark',
throw=True, throw=True,
**pycompat.strkwargs(args) **pycompat.strkwargs(args)
) )
if hook.hashook(repo.ui, b'pretxnclose-phase'): if hook.hashook(repo.ui, b'pretxnclose-phase'):
cl = repo.unfiltered().changelog cl = repo.unfiltered().changelog
for rev, (old, new) in tr.changes[b'phases'].items(): for revs, (old, new) in tr.changes[b'phases']:
for rev in revs:
args = tr.hookargs.copy() args = tr.hookargs.copy()
node = hex(cl.node(rev)) node = hex(cl.node(rev))
args.update(phases.preparehookargs(node, old, new)) args.update(phases.preparehookargs(node, old, new))
repo.hook( repo.hook(
b'pretxnclose-phase', b'pretxnclose-phase',
throw=True, throw=True,
**pycompat.strkwargs(args) **pycompat.strkwargs(args)
) )
repo.hook( repo.hook(
b'pretxnclose', throw=True, **pycompat.strkwargs(tr.hookargs) b'pretxnclose', throw=True, **pycompat.strkwargs(tr.hookargs)
) )
def releasefn(tr, success): def releasefn(tr, success):
repo = reporef() repo = reporef()
if repo is None: if repo is None:
self.store.createmode, self.store.createmode,
validator=validate, validator=validate,
releasefn=releasefn, releasefn=releasefn,
checkambigfiles=_cachedfiles, checkambigfiles=_cachedfiles,
name=desc, name=desc,
) )
tr.changes[b'origrepolen'] = len(self) tr.changes[b'origrepolen'] = len(self)
tr.changes[b'obsmarkers'] = set() tr.changes[b'obsmarkers'] = set()
tr.changes[b'phases'] = {} tr.changes[b'phases'] = []
tr.changes[b'bookmarks'] = {} tr.changes[b'bookmarks'] = {}
tr.hookargs[b'txnid'] = txnid tr.hookargs[b'txnid'] = txnid
tr.hookargs[b'txnname'] = desc tr.hookargs[b'txnname'] = desc
# note: writing the fncache only during finalize mean that the file is # note: writing the fncache only during finalize mean that the file is
# outdated when running hooks. As fncache is used for streaming clone, # outdated when running hooks. As fncache is used for streaming clone,
# this is not expected to break anything that happen during the hooks. # this is not expected to break anything that happen during the hooks.
tr.addfinalize(b'flush-fncache', self.store.write) tr.addfinalize(b'flush-fncache', self.store.write)
repo.hook( repo.hook(
b'txnclose-bookmark', b'txnclose-bookmark',
throw=False, throw=False,
**pycompat.strkwargs(args) **pycompat.strkwargs(args)
) )
if hook.hashook(repo.ui, b'txnclose-phase'): if hook.hashook(repo.ui, b'txnclose-phase'):
cl = repo.unfiltered().changelog cl = repo.unfiltered().changelog
phasemv = sorted(tr.changes[b'phases'].items()) phasemv = sorted(
for rev, (old, new) in phasemv: tr.changes[b'phases'], key=lambda r: r[0][0]
)
for revs, (old, new) in phasemv:
for rev in revs:
args = tr.hookargs.copy() args = tr.hookargs.copy()
node = hex(cl.node(rev)) node = hex(cl.node(rev))
args.update(phases.preparehookargs(node, old, new)) args.update(phases.preparehookargs(node, old, new))
repo.hook( repo.hook(
b'txnclose-phase', b'txnclose-phase',
throw=False, throw=False,
**pycompat.strkwargs(args) **pycompat.strkwargs(args)
) )
repo.hook( repo.hook(
b'txnclose', throw=False, **pycompat.strkwargs(hookargs) b'txnclose', throw=False, **pycompat.strkwargs(hookargs)
) )
reporef()._afterlock(hookfunc) reporef()._afterlock(hookfunc)
tr.addfinalize(b'txnclose-hook', txnclosehook) tr.addfinalize(b'txnclose-hook', txnclosehook)

mercurial/phases.py

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)
headsbyphase[phase].append(node) headsbyphase[phase].append(node)
return headsbyphase return headsbyphase
def _sortedrange_insert(data, idx, rev, t):
merge_before = False
if idx:
r1, t1 = data[idx - 1]
merge_before = r1[-1] + 1 == rev and t1 == t
merge_after = False
if idx < len(data):
r2, t2 = data[idx]
merge_after = r2[0] == rev + 1 and t2 == t
if merge_before and merge_after:
data[idx - 1] = (pycompat.xrange(r1[0], r2[-1] + 1), t)
data.pop(idx)
elif merge_before:
data[idx - 1] = (pycompat.xrange(r1[0], rev + 1), t)
elif merge_after:
data[idx] = (pycompat.xrange(rev, r2[-1] + 1), t)
else:
data.insert(idx, (pycompat.xrange(rev, rev + 1), t))
def _sortedrange_split(data, idx, rev, t):
r1, t1 = data[idx]
if t == t1:
return
t = (t1[0], t[1])
if len(r1) == 1:
data.pop(idx)
_sortedrange_insert(data, idx, rev, t)
elif r1[0] == rev:
data[idx] = (pycompat.xrange(rev + 1, r1[-1] + 1), t1)
_sortedrange_insert(data, idx, rev, t)
elif r1[-1] == rev:
data[idx] = (pycompat.xrange(r1[0], rev), t1)
_sortedrange_insert(data, idx + 1, rev, t)
else:
data[idx : idx + 1] = [
(pycompat.xrange(r1[0], rev), t1),
(pycompat.xrange(rev, rev + 1), t),
(pycompat.xrange(rev + 1, r1[-1] + 1), t1),
]
def _trackphasechange(data, rev, old, new): def _trackphasechange(data, rev, old, new):
"""add a phase move the <data> dictionnary """add a phase move to the <data> list of ranges
If data is None, nothing happens. If data is None, nothing happens.
""" """
if data is None: if data is None:
return return
existing = data.get(rev)
if existing is not None: # If data is empty, create a one-revision range and done
old = existing[0] if not data:
data[rev] = (old, new) data.insert(0, (pycompat.xrange(rev, rev + 1), (old, new)))
return
low = 0
durin42Unsubmitted
Not Done

It doesn't look like there's a good reason for this function to be nested like this. Can we move it to module-level with an underscore prefix?

durin42: It doesn't look like there's a good reason for this function to be nested like this. Can we…
high = len(data)
t = (old, new)
while low < high:
mid = (low + high) // 2
revs = data[mid][0]
if rev in revs:
_sortedrange_split(data, mid, rev, t)
return
if revs[0] == rev + 1:
if mid and data[mid - 1][0][-1] == rev:
_sortedrange_split(data, mid - 1, rev, t)
else:
_sortedrange_insert(data, mid, rev, t)
return
if revs[-1] == rev - 1:
if mid + 1 < len(data) and data[mid + 1][0][0] == rev:
_sortedrange_split(data, mid + 1, rev, t)
durin42Unsubmitted
Not Done

here too

durin42: here too
else:
_sortedrange_insert(data, mid + 1, rev, t)
return
if revs[0] > rev:
high = mid
else:
low = mid + 1
if low == len(data):
data.append((pycompat.xrange(rev, rev + 1), t))
return
r1, t1 = data[low]
if r1[0] > rev:
data.insert(low, (pycompat.xrange(rev, rev + 1), t))
else:
data.insert(low + 1, (pycompat.xrange(rev, rev + 1), t))
class phasecache(object): class phasecache(object):
def __init__(self, repo, phasedefaults, _load=True): def __init__(self, repo, phasedefaults, _load=True):
if _load: if _load:
# Cheap trick to allow shallow-copy without copy module # Cheap trick to allow shallow-copy without copy module
self.phaseroots, self.dirty = _readroots(repo, phasedefaults) self.phaseroots, self.dirty = _readroots(repo, phasedefaults)
self._loadedrevslen = 0 self._loadedrevslen = 0
def registernew(self, repo, tr, targetphase, nodes): def registernew(self, repo, tr, targetphase, nodes):
repo = repo.unfiltered() repo = repo.unfiltered()
self._retractboundary(repo, tr, targetphase, nodes) self._retractboundary(repo, tr, targetphase, nodes)
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
for n in nodes: revs = [torev(node) for node in nodes]
rev = torev(n) revs.sort()
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, 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.
# find the phase of the affected revision # find the phase of the affected revision
for phase in pycompat.xrange(targetphase, -1, -1): for phase in pycompat.xrange(targetphase, -1, -1):
if phase: if phase:
roots = oldroots[phase] roots = oldroots[phase]
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 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):
# 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 targetphase in (archived, internal) and not supportinternal(repo): if targetphase in (archived, internal) and not supportinternal(repo):
name = phasenames[targetphase] name = phasenames[targetphase]

mercurial/scmutil.py

repo.ui.status(msg % len(extinctadded)) repo.ui.status(msg % len(extinctadded))
@reportsummary @reportsummary
def reportphasechanges(repo, tr): def reportphasechanges(repo, tr):
"""Report statistics of phase changes for changesets pre-existing """Report statistics of phase changes for changesets pre-existing
pull/unbundle. pull/unbundle.
""" """
origrepolen = tr.changes.get(b'origrepolen', len(repo)) origrepolen = tr.changes.get(b'origrepolen', len(repo))
phasetracking = tr.changes.get(b'phases', {}) published = []
if not phasetracking: for revs, (old, new) in tr.changes.get(b'phases', []):
return if new != phases.public:
published = [ continue
rev published.extend(rev for rev in revs if rev < origrepolen)
for rev, (old, new) in pycompat.iteritems(phasetracking)
if new == phases.public and rev < origrepolen
]
if not published: if not published:
return return
msg = _(b'%d local changesets published\n') msg = _(b'%d local changesets published\n')
if as_validator: if as_validator:
msg = _(b'%d local changesets will be published\n') msg = _(b'%d local changesets will be published\n')
repo.ui.status(msg % len(published)) repo.ui.status(msg % len(published))

tests/testlib/ext-phase-report.py

# tiny extension to report phase changes during transaction # tiny extension to report phase changes during transaction
from __future__ import absolute_import from __future__ import absolute_import
def reposetup(ui, repo): def reposetup(ui, repo):
def reportphasemove(tr): def reportphasemove(tr):
for rev, move in sorted(tr.changes[b'phases'].items()): for revs, move in sorted(tr.changes[b"phases"], key=lambda r: r[0][0]):
for rev in revs:
if move[0] is None: if move[0] is None:
ui.write( ui.write(
( (
b'test-debug-phase: new rev %d: x -> %d\n' b'test-debug-phase: new rev %d: x -> %d\n'
% (rev, move[1]) % (rev, move[1])
) )
) )
else: else:
ui.write( ui.write(
( (
b'test-debug-phase: move rev %d: %d -> %d\n' b'test-debug-phase: move rev %d: %d -> %d\n'
% (rev, move[0], move[1]) % (rev, move[0], move[1])
) )
) )
class reportphaserepo(repo.__class__): class reportphaserepo(repo.__class__):
def transaction(self, *args, **kwargs): def transaction(self, *args, **kwargs):
tr = super(reportphaserepo, self).transaction(*args, **kwargs) tr = super(reportphaserepo, self).transaction(*args, **kwargs)
tr.addpostclose(b'report-phase', reportphasemove) tr.addpostclose(b'report-phase', reportphasemove)
return tr return tr
repo.__class__ = reportphaserepo repo.__class__ = reportphaserepo