This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8654

phases: make phase list dense or dictionaries
AbandonedPublic

Authored by joerg.sonnenberger on Jun 23 2020, 8:26 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

When the internal and archived phase was added, allphases became sparse
populated. This dramatically increased the number of lookup operations
for public revisions when they hit phasecache.phase as it it is
iterationing over all phases in the phasesets. Fix this by making the
various data structures either sparse lists or dictionaries, depending
on the purpose. Push the node->revision mapping down into the C module,
no need to do it in Python when it is as much work in the backend.
Document the inconsistency of phaseroots and how it breaks the parent
lookup shortcut.

Overall, this improves a no-change "hg up" for my NetBSD test
repository from 4.7s to 1.3s.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

mjacob added a subscriber: mjacob.Jun 23 2020, 9:23 PM

This is probably not the conceptual feedback you were looking for. I hope it’s still helpful.

mercurial/exchange.py
1031–1032

nit: you could directly iterate on the values

mercurial/exchangev2.py
85

This should probably use itervalues(), but see below.

89–90

If I understand correctly, phases.phasenumber is the same as the key for phase in this for loop. If you use iteritems() above, I think phases.phasenumber is unused.

361

This should probably use itervalues() (not that it makes a difference for the small dict).

mercurial/phases.py
133–137

Is it for backwards compatibility? It wasn’t obvious for me without a comment.

203–204

This line is now outdated.

mercurial/repoview.py
156–157

You could let trackedphaseroots() return only the values directly.

joerg.sonnenberger added inline comments.Jun 24 2020, 6:38 AM
mercurial/exchangev2.py
85

phasenames contains a small number of strings, the copy/reference doesn't matter here.

89–90

Right, possible. This was meant to preserve the original logic for now, but I'll update it for the next iteration.

361

Yeah, I don't think it makes a difference at all and it is more verbose.

mercurial/phases.py
133–137

Yes, both phases can be seen in existing repositories, so the values have to be preserved. Otherwise I would have moved them to 3 and 4, which would have greatly simplified things.

joerg.sonnenberger retitled this revision from phases: make phase list dense or dictionaries [PoC] to phases: make phase list dense or dictionaries.Jun 27 2020, 6:02 PM
joerg.sonnenberger edited the summary of this revision. (Show Details)
joerg.sonnenberger updated this revision to Diff 21721.
joerg.sonnenberger added a comment.Jun 27 2020, 7:34 PM

Further analysis shows that the main issue here is that the topic exchange triggers a rebuild of the "served-topic" branchmap without a real reason. That doesn't mean the change isn't useful in general.

joerg.sonnenberger marked 6 inline comments as done and an inline comment as not done.Jul 2 2020, 2:19 PM

Addressed most of the issues raised, some of them were considered and rejected (re items()).

The evolve follow-up patch can be found in https://foss.heptapod.net/mercurial/evolve/-/commit/17d9e81355294320beef2ae11435485716592146

mercurial/exchangev2.py
85

Simplified to use items(), but I still don't care about the copying here as the dictionary is small.

pulkit added a subscriber: pulkit.Jul 7 2020, 2:00 PM

It's bit hard to review the patch since it seems that it consists multiple changes. I can see following ones:

  1. Change allphases to not contain unrequired phases
  2. Introduce utility function to check public roots
  3. Conversion of some list to dictionary

There might be more.

Can you kindly split them into individual patches?

mercurial/phases.py
131

I think we can keep these two constants and hence leave how internal and archived are computed.

149

no need of this change since allphases won't contain extra phases now. In future, we will only need to update allphases.

joerg.sonnenberger added a comment.Jul 7 2020, 2:48 PM

I can split the patch in some smaller parts. Is the API change itself as goal reasonable?

mercurial/phases.py
149

This change makes it IMO clear what localhiddenphases actually is. I was actually considering spelling them all out explicitly, simply to remove the magic. The HIDEABLE_FLAG doesn't really add value.

joerg.sonnenberger abandoned this revision.Jul 7 2020, 6:39 PM

Superseded by D8694, D8695, D8697 and D8698.

Revision Contents
Changeset List

Diff 21721

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/cext/parsers.c

{"fm1readmarkers", fm1readmarkers, METH_VARARGS, {"fm1readmarkers", fm1readmarkers, METH_VARARGS,
"parse v1 obsolete markers\n"}, "parse v1 obsolete markers\n"},
{NULL, NULL}}; {NULL, NULL}};
void dirs_module_init(PyObject *mod); void dirs_module_init(PyObject *mod);
void manifest_module_init(PyObject *mod); void manifest_module_init(PyObject *mod);
void revlog_module_init(PyObject *mod); void revlog_module_init(PyObject *mod);
static const int version = 16; static const int version = 17;
static void module_init(PyObject *mod) static void module_init(PyObject *mod)
{ {
PyObject *capsule = NULL; PyObject *capsule = NULL;
PyModule_AddIntConstant(mod, "version", version); PyModule_AddIntConstant(mod, "version", version);
/* This module constant has two purposes. First, it lets us unit test /* This module constant has two purposes. First, it lets us unit test
* the ImportError raised without hard-coding any error text. This * the ImportError raised without hard-coding any error text. This

mercurial/cext/revlog.c

} }
static PyObject *nullentry = NULL; static PyObject *nullentry = NULL;
static const char nullid[20] = {0}; static const char nullid[20] = {0};
static const Py_ssize_t nullrev = -1; static const Py_ssize_t nullrev = -1;
static Py_ssize_t inline_scan(indexObject *self, const char **offsets); static Py_ssize_t inline_scan(indexObject *self, const char **offsets);
static int index_find_node(indexObject *self, const char *node,
Py_ssize_t nodelen);
#if LONG_MAX == 0x7fffffffL #if LONG_MAX == 0x7fffffffL
static const char *const tuple_format = PY23("Kiiiiiis#", "Kiiiiiiy#"); static const char *const tuple_format = PY23("Kiiiiiis#", "Kiiiiiiy#");
#else #else
static const char *const tuple_format = PY23("kiiiiiis#", "kiiiiiiy#"); static const char *const tuple_format = PY23("kiiiiiis#", "kiiiiiiy#");
#endif #endif
/* A RevlogNG v1 index entry is 64 bytes long. */ /* A RevlogNG v1 index entry is 64 bytes long. */
static const long v1_hdrsize = 64; static const long v1_hdrsize = 64;
isfiltered = PyObject_IsTrue(result); isfiltered = PyObject_IsTrue(result);
Py_DECREF(result); Py_DECREF(result);
return isfiltered; return isfiltered;
} else { } else {
return 0; return 0;
} }
} }
static Py_ssize_t add_roots_get_min(indexObject *self, PyObject *list,
Py_ssize_t marker, char *phases)
{
PyObject *iter = NULL;
PyObject *iter_item = NULL;
Py_ssize_t min_idx = index_length(self) + 2;
long iter_item_long;
if (PyList_GET_SIZE(list) != 0) {
iter = PyObject_GetIter(list);
if (iter == NULL)
return -2;
while ((iter_item = PyIter_Next(iter))) {
if (!pylong_to_long(iter_item, &iter_item_long)) {
Py_DECREF(iter_item);
return -2;
}
Py_DECREF(iter_item);
if (iter_item_long < min_idx)
min_idx = iter_item_long;
phases[iter_item_long] = (char)marker;
}
Py_DECREF(iter);
}
return min_idx;
}
static inline void set_phase_from_parents(char *phases, int parent_1, static inline void set_phase_from_parents(char *phases, int parent_1,
int parent_2, Py_ssize_t i) int parent_2, Py_ssize_t i)
{ {
if (parent_1 >= 0 && phases[parent_1] > phases[i]) if (parent_1 >= 0 && phases[parent_1] > phases[i])
phases[i] = phases[parent_1]; phases[i] = phases[parent_1];
if (parent_2 >= 0 && phases[parent_2] > phases[i]) if (parent_2 >= 0 && phases[parent_2] > phases[i])
phases[i] = phases[parent_2]; phases[i] = phases[parent_2];
} }
return reachable; return reachable;
bail: bail:
Py_XDECREF(reachable); Py_XDECREF(reachable);
free(revstates); free(revstates);
free(tovisit); free(tovisit);
return NULL; return NULL;
} }
static int add_roots_get_min(indexObject *self, PyObject *roots, char *phases,
char phase)
{
Py_ssize_t len = index_length(self);
PyObject *iter;
PyObject *item;
PyObject *iterator;
int rev, minrev = -1;
char *node;
if (!PySet_Check(roots))
return -2;
iterator = PyObject_GetIter(roots);
if (iterator == NULL)
return -2;
while ((item = PyIter_Next(iterator))) {
if (node_check(item, &node) == -1)
goto failed;
rev = index_find_node(self, node, 20);
/* null is implicitly public, so negative is invalid */
if (rev < 0 || rev >= len)
goto failed;
phases[rev] = phase;
if (minrev == -1 || minrev > rev)
minrev = rev;
Py_DECREF(item);
}
Py_DECREF(iterator);
return minrev;
failed:
Py_DECREF(iterator);
Py_DECREF(item);
return -2;
}
static PyObject *compute_phases_map_sets(indexObject *self, PyObject *args) static PyObject *compute_phases_map_sets(indexObject *self, PyObject *args)
{ {
PyObject *roots = Py_None; /* 0: public (untracked), 1: draft, 2: secret, 32: archive,
96: internal */
static const char trackedphases[] = {1, 2, 32, 96};
PyObject *ret = NULL; PyObject *ret = NULL;
PyObject *phasessize = NULL; PyObject *roots = Py_None;
PyObject *idx = NULL;
PyObject *pyphase = NULL;
PyObject *pyrev = NULL;
PyObject *phaseroots = NULL; PyObject *phaseroots = NULL;
PyObject *phaseset = NULL; PyObject *phasessize = NULL;
PyObject *phasessetlist = NULL; PyObject *phasesets[4] = {NULL, NULL, NULL, NULL};
PyObject *rev = NULL;
Py_ssize_t len = index_length(self); Py_ssize_t len = index_length(self);
Py_ssize_t numphase = 0; const char *currentphase;
Py_ssize_t minrevallphases = 0;
Py_ssize_t minrevphase = 0;
Py_ssize_t i = 0;
char *phases = NULL; char *phases = NULL;
long phase; int minphaserev = -1, rev, i;
const int numphases = (int)(sizeof(phasesets) / sizeof(phasesets[0]));
if (!PyArg_ParseTuple(args, "O", &roots)) if (!PyArg_ParseTuple(args, "O", &roots))
goto done; return NULL;
if (roots == NULL || !PyList_Check(roots)) { if (roots == NULL || !PyDict_Check(roots)) {
PyErr_SetString(PyExc_TypeError, "roots must be a list"); PyErr_SetString(PyExc_TypeError, "roots must be a dictionary");
goto done; return NULL;
} }
phases = calloc( phases = calloc(len, 1);
len, 1); /* phase per rev: {0: public, 1: draft, 2: secret} */
if (phases == NULL) { if (phases == NULL) {
PyErr_NoMemory(); PyErr_NoMemory();
goto done; return NULL;
} }
/* Put the phase information of all the roots in phases */
numphase = PyList_GET_SIZE(roots) + 1;
minrevallphases = len + 1;
phasessetlist = PyList_New(numphase);
if (phasessetlist == NULL)
goto done;
PyList_SET_ITEM(phasessetlist, 0, Py_None); for (i = 0; i < numphases; ++i) {
Py_INCREF(Py_None); pyphase = PyInt_FromLong(trackedphases[i]);
if (pyphase == NULL)
for (i = 0; i < numphase - 1; i++) {
phaseroots = PyList_GET_ITEM(roots, i);
phaseset = PySet_New(NULL);
if (phaseset == NULL)
goto release; goto release;
PyList_SET_ITEM(phasessetlist, i + 1, phaseset); phaseroots = PyDict_GetItem(roots, pyphase);
if (!PyList_Check(phaseroots)) { Py_DECREF(pyphase);
PyErr_SetString(PyExc_TypeError, if (phaseroots == NULL)
"roots item must be a list"); continue;
rev = add_roots_get_min(self, phaseroots, phases, trackedphases[i]);
phaseroots = NULL;
if (rev == -2)
goto release; goto release;
if (rev != -1 && (minphaserev == -1 || rev < minphaserev))
minphaserev = rev;
} }
minrevphase =
add_roots_get_min(self, phaseroots, i + 1, phases); for (i = 0; i < numphases; ++i) {
if (minrevphase == -2) /* Error from add_roots_get_min */ phasesets[i] = PySet_New(NULL);
if (phasesets[i] == NULL)
goto release; goto release;
minrevallphases = MIN(minrevallphases, minrevphase);
} }
/* Propagate the phase information from the roots to the revs */
if (minrevallphases != -1) { if (minphaserev == -1)
minphaserev = len;
for (rev = minphaserev; rev < len; ++rev) {
int parents[2]; int parents[2];
for (i = minrevallphases; i < len; i++) { /*
if (index_get_parents(self, i, parents, (int)len - 1) < * The parent lookup could be skipped for phaseroots, but
0) * phase --force would historically not recompute them
* correctly, leaving descendents with a lower phase around.
* As such, unconditionally recompute the phase.
*/
if (index_get_parents(self, rev, parents, (int)len - 1) < 0)
goto release;
set_phase_from_parents(phases, parents[0], parents[1], rev);
switch (phases[rev]) {
case 0:
continue;
case 1:
pyphase = phasesets[0];
break;
case 2:
pyphase = phasesets[1];
break;
case 32:
pyphase = phasesets[2];
break;
case 96:
pyphase = phasesets[3];
break;
default:
goto release; goto release;
set_phase_from_parents(phases, parents[0], parents[1],
i);
} }
pyrev = PyInt_FromLong(rev);
if (pyrev == NULL)
goto release;
if (PySet_Add(pyphase, pyrev) == -1) {
Py_DECREF(pyrev);
goto release;
} }
/* Transform phase list to a python list */ Py_DECREF(pyrev);
phasessize = PyInt_FromSsize_t(len); }
if (phasessize == NULL) phaseroots = PyDict_New();
if (phaseroots == NULL)
goto release; goto release;
for (i = 0; i < len; i++) { for (int i = 0; i < numphases; ++i) {
phase = phases[i]; pyphase = PyInt_FromLong(trackedphases[i]);
/* We only store the sets of phase for non public phase, the if (pyphase == NULL)
* public phase is computed as a difference */ goto release;
if (phase != 0) { if (PyDict_SetItem(phaseroots, pyphase, phasesets[i]) == -1) {
phaseset = PyList_GET_ITEM(phasessetlist, phase); Py_DECREF(pyphase);
rev = PyInt_FromSsize_t(i);
if (rev == NULL)
goto release; goto release;
PySet_Add(phaseset, rev);
Py_XDECREF(rev);
} }
Py_DECREF(phasesets[i]);
phasesets[i] = NULL;
} }
ret = PyTuple_Pack(2, phasessize, phasessetlist); phasessize = PyInt_FromSsize_t(len);
if (phasessize == NULL)
goto release;
ret = PyTuple_Pack(2, phasessize, phaseroots);
Py_DECREF(phasessize);
Py_DECREF(phaseroots);
return ret;
release: release:
Py_XDECREF(phasessize); for (i = 0; i < numphases; ++i)
Py_XDECREF(phasessetlist); Py_XDECREF(phasesets[i]);
done: Py_XDECREF(phaseroots);
free(phases); free(phases);
return ret; return NULL;
} }
static PyObject *index_headrevs(indexObject *self, PyObject *args) static PyObject *index_headrevs(indexObject *self, PyObject *args)
{ {
Py_ssize_t i, j, len; Py_ssize_t i, j, len;
char *nothead = NULL; char *nothead = NULL;
PyObject *heads = NULL; PyObject *heads = NULL;
PyObject *filter = NULL; PyObject *filter = NULL;

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()
mjacobUnsubmitted
Done

nit: you could directly iterate on the values

mjacob: nit: you could directly iterate on the values
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():
mjacobUnsubmitted
Done

This should probably use itervalues(), but see below.

mjacob: This should probably use `itervalues()`, but see below.
joerg.sonnenbergerAuthorUnsubmitted
Done

phasenames contains a small number of strings, the copy/reference doesn't matter here.

joerg.sonnenberger: phasenames contains a small number of strings, the copy/reference doesn't matter here.
joerg.sonnenbergerAuthorUnsubmitted
Done

Simplified to use items(), but I still don't care about the copying here as the dictionary is small.

joerg.sonnenberger: Simplified to use items(), but I still don't care about the copying here as the dictionary is…
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],
mjacobUnsubmitted
Done

If I understand correctly, phases.phasenumber is the same as the key for phase in this for loop. If you use iteritems() above, I think phases.phasenumber is unused.

mjacob: If I understand correctly, `phases.phasenumber` is the same as the key for `phase` in this for…
joerg.sonnenbergerAuthorUnsubmitted
Done

Right, possible. This was meant to preserve the original logic for now, but I'll update it for the next iteration.

joerg.sonnenberger: Right, possible. This was meant to preserve the original logic for now, but I'll update it for…
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()}
mjacobUnsubmitted
Done

This should probably use itervalues() (not that it makes a difference for the small dict).

mjacob: This should probably use `itervalues()` (not that it makes a difference for the small dict).
joerg.sonnenbergerAuthorUnsubmitted
Done

Yeah, I don't think it makes a difference at all and it is more verbose.

joerg.sonnenberger: Yeah, I don't think it makes a difference at all and it is more verbose.
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
pulkitUnsubmitted
Not Done

I think we can keep these two constants and hence leave how internal and archived are computed.

pulkit: I think we can keep these two constants and hence leave how internal and archived are computed.
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 internal = 96
archived = HIDEABLE_FLAG archived = 32
allphases = list(range(internal + 1)) allphases = [public, draft, secret, archived, internal]
trackedphases = allphases[1:] trackedphases = allphases[draft:]
# record phase names # record phase names
mjacobUnsubmitted
Not Done

Is it for backwards compatibility? It wasn’t obvious for me without a comment.

mjacob: Is it for backwards compatibility? It wasn’t obvious for me without a comment.
joerg.sonnenbergerAuthorUnsubmitted
Not Done

Yes, both phases can be seen in existing repositories, so the values have to be preserved. Otherwise I would have moved them to 3 and 4, which would have greatly simplified things.

joerg.sonnenberger: Yes, both phases can be seen in existing repositories, so the values have to be preserved.
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'
phasenumber = {name: phase for phase, name in phasenames.items()}
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 = tuple(allphases[1:])
remotehiddenphases = tuple(allphases[2:]) remotehiddenphases = tuple(allphases[2:])
localhiddenphases = tuple(p for p in allphases if p & HIDEABLE_FLAG) localhiddenphases = (internal, archived)
pulkitUnsubmitted
Not Done

no need of this change since allphases won't contain extra phases now. In future, we will only need to update allphases.

pulkit: no need of this change since `allphases` won't contain extra phases now. In future, we will…
joerg.sonnenbergerAuthorUnsubmitted
Done

This change makes it IMO clear what localhiddenphases actually is. I was actually considering spelling them all out explicitly, simply to remove the magic. The HIDEABLE_FLAG doesn't really add value.

joerg.sonnenberger: This change makes it IMO clear what localhiddenphases actually is. I was actually considering…
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 = {i: set() for i in allphases}
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
mjacobUnsubmitted
Done

This line is now outdated.

mjacob: This line is now outdated.
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)
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
self._phasesets = None self._phasesets = None
self.filterunknown(repo) self.filterunknown(repo)
self.opener = repo.svfs self.opener = repo.svfs
def hasnonpublicphases(self, repo):
"""detect if there are revisions with non-public phase"""
repo = repo.unfiltered()
cl = repo.changelog
if len(cl) >= self._loadedrevslen:
self.invalidate()
self.loadphaserevs(repo)
return any(
revs
for phase, revs in pycompat.iteritems(self.phaseroots)
if phase != public
)
def nonpublicphaseroots(self, repo):
"""returns the roots of all non-public phases
The roots are not minimized, so if the secret revisions are
descendants of draft revisions, their roots will still be present.
"""
repo = repo.unfiltered()
cl = repo.changelog
if len(cl) >= self._loadedrevslen:
self.invalidate()
self.loadphaserevs(repo)
return set().union(
*[
revs
for phase, revs in pycompat.iteritems(self.phaseroots)
if phase != public
]
)
def getrevset(self, repo, phases, subset=None): def getrevset(self, repo, phases, subset=None):
"""return a smartset for the given phases""" """return a smartset for the given phases"""
self.loadphaserevs(repo) # ensure phase's sets are loaded self.loadphaserevs(repo) # ensure phase's sets are loaded
phases = set(phases) phases = set(phases)
publicphase = public in phases publicphase = public in phases
if publicphase: if publicphase:
# In this case, phases keeps all the *other* phases. # In this case, phases keeps all the *other* phases.
if not revs: if not revs:
return subset return subset
return subset.filter(lambda r: r not in revs) return subset.filter(lambda r: r not in revs)
def copy(self): def copy(self):
# Shallow copy meant to ensure isolation in # Shallow copy meant to ensure isolation in
# advance/retractboundary(), nothing more. # advance/retractboundary(), nothing more.
ph = self.__class__(None, None, _load=False) ph = self.__class__(None, None, _load=False)
ph.phaseroots = self.phaseroots[:] ph.phaseroots = self.phaseroots.copy()
ph.dirty = self.dirty ph.dirty = self.dirty
ph.opener = self.opener ph.opener = self.opener
ph._loadedrevslen = self._loadedrevslen ph._loadedrevslen = self._loadedrevslen
ph._phasesets = self._phasesets ph._phasesets = self._phasesets
return ph return ph
def replace(self, phcache): def replace(self, phcache):
"""replace all values in 'self' with content of phcache""" """replace all values in 'self' with content of phcache"""
for a in ( for a in (
b'phaseroots', b'phaseroots',
b'dirty', b'dirty',
b'opener', b'opener',
b'_loadedrevslen', b'_loadedrevslen',
b'_phasesets', b'_phasesets',
): ):
setattr(self, a, getattr(phcache, a)) setattr(self, a, getattr(phcache, a))
def _getphaserevsnative(self, repo): def _getphaserevsnative(self, repo):
repo = repo.unfiltered() repo = repo.unfiltered()
nativeroots = [] return repo.changelog.computephases(self.phaseroots)
for phase in trackedphases:
nativeroots.append(
pycompat.maplist(repo.changelog.rev, self.phaseroots[phase])
)
return repo.changelog.computephases(nativeroots)
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 = {phase: set() for phase in allphases}
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)
return return
f = self.opener(b'phaseroots', b'w', atomictemp=True, checkambig=True) f = self.opener(b'phaseroots', b'w', atomictemp=True, checkambig=True)
try: try:
self._write(f) self._write(f)
finally: finally:
f.close() f.close()
def _write(self, fp): def _write(self, fp):
for phase, roots in enumerate(self.phaseroots): for phase, roots in pycompat.iteritems(self.phaseroots):
for h in sorted(roots): for h in sorted(roots):
fp.write(b'%i %s\n' % (phase, hex(h))) fp.write(b'%i %s\n' % (phase, hex(h)))
self.dirty = False self.dirty = False
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
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]
if not dryrun: if not dryrun:
# declare deleted root in the target phase # declare deleted root in the target phase
if targetphase != 0: if targetphase != 0:
self._retractboundary(repo, tr, targetphase, delroots) self._retractboundary(repo, tr, targetphase, delroots)
repo.invalidatevolatilesets() repo.invalidatevolatilesets()
return changes return changes
def retractboundary(self, repo, tr, targetphase, nodes): def retractboundary(self, repo, tr, targetphase, nodes):
oldroots = self.phaseroots[: targetphase + 1] oldroots = {
phase: revs
for phase, revs in pycompat.iteritems(self.phaseroots)
if phase <= targetphase
}
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()
if ( if (
self._retractboundary(repo, tr, targetphase, nodes) self._retractboundary(repo, tr, targetphase, nodes)
and phasetracking is not None and phasetracking is not None
): ):
# find the affected revisions # find the affected revisions
new = self.phaseroots[targetphase] new = self.phaseroots[targetphase]
old = oldroots[targetphase] old = oldroots[targetphase]
affected = set(repo.revs(b'(%ln::) - (%ln::)', new, old)) affected = set(repo.revs(b'(%ln::) - (%ln::)', new, old))
# 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.get(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 sorted(revs): for r in sorted(revs):
_trackphasechange(phasetracking, r, phase, targetphase) _trackphasechange(phasetracking, r, phase, targetphase)
repo.invalidatevolatilesets() repo.invalidatevolatilesets()
def filterunknown(self, repo): def filterunknown(self, repo):
"""remove unknown nodes from the phase boundary """remove unknown nodes from the phase boundary
Nothing is lost as unknown nodes only hold data for their descendants. Nothing is lost as unknown nodes only hold data for their descendants.
""" """
filtered = False filtered = False
has_node = repo.changelog.index.has_node # to filter unknown nodes has_node = repo.changelog.index.has_node # to filter unknown nodes
for phase, nodes in enumerate(self.phaseroots): for phase, nodes in pycompat.iteritems(self.phaseroots):
missing = sorted(node for node in nodes if not has_node(node)) missing = sorted(node for node in nodes if not has_node(node))
if missing: if missing:
for mnode in missing: for mnode in missing:
repo.ui.debug( repo.ui.debug(
b'removing unknown node %s from %i-phase boundary\n' b'removing unknown node %s from %i-phase boundary\n'
% (short(mnode), phase) % (short(mnode), phase)
) )
nodes.symmetric_difference_update(missing) nodes.symmetric_difference_update(missing)
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 updatephases(repo, trgetter, headsbyphase): def updatephases(repo, trgetter, headsbyphase):
"""Updates the repo with the given phase heads""" """Updates the repo with the given phase heads"""
# Now advance phase boundaries of all but secret phase # Now advance phase boundaries of all phases
# #
# run the update (and fetch transaction) only if there are actually things # run the update (and fetch transaction) only if there are actually things
# to update. This avoid creating empty transaction during no-op operation. # to update. This avoid creating empty transaction during no-op operation.
for phase in allphases[:-1]: for phase in allphases:
revset = b'%ln - _phase(%s)' revset = b'%ln - _phase(%s)'
heads = [c.node() for c in repo.set(revset, headsbyphase[phase], phase)] heads = [c.node() for c in repo.set(revset, headsbyphase[phase], phase)]
if heads: if heads:
advanceboundary(repo, trgetter(), phase, heads) advanceboundary(repo, trgetter(), phase, heads)
def analyzeremotephases(repo, subset, roots): def analyzeremotephases(repo, subset, roots):
"""Compute phases heads and root in a subset of node from root dict """Compute phases heads and root in a subset of node from root dict
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[2]) 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]}

mercurial/policy.py

# keep in sync with "version" in C modules # keep in sync with "version" in C modules
_cextversions = { _cextversions = {
('cext', 'base85'): 1, ('cext', 'base85'): 1,
('cext', 'bdiff'): 3, ('cext', 'bdiff'): 3,
('cext', 'mpatch'): 1, ('cext', 'mpatch'): 1,
('cext', 'osutil'): 4, ('cext', 'osutil'): 4,
('cext', 'parsers'): 16, ('cext', 'parsers'): 17,
} }
# map import request to other package or module # map import request to other package or module
_modredirects = { _modredirects = {
('cext', 'charencode'): ('cext', 'parsers'), ('cext', 'charencode'): ('cext', 'parsers'),
('cffi', 'base85'): ('pure', 'base85'), ('cffi', 'base85'): ('pure', 'base85'),
('cffi', 'charencode'): ('pure', 'charencode'), ('cffi', 'charencode'): ('pure', 'charencode'),
('cffi', 'parsers'): ('pure', 'parsers'), ('cffi', 'parsers'): ('pure', 'parsers'),

mercurial/repoview.py

return frozenset(hiddens | secrets) return frozenset(hiddens | secrets)
else: else:
return hiddens return hiddens
def computemutable(repo, visibilityexceptions=None): def computemutable(repo, visibilityexceptions=None):
assert not repo.changelog.filteredrevs assert not repo.changelog.filteredrevs
# fast check to avoid revset call on huge repo # fast check to avoid revset call on huge repo
if any(repo._phasecache.phaseroots[1:]): if repo._phasecache.hasnonpublicphases(repo):
getphase = repo._phasecache.phase getphase = repo._phasecache.phase
maymutable = filterrevs(repo, b'base') maymutable = filterrevs(repo, b'base')
return frozenset(r for r in maymutable if getphase(repo, r)) return frozenset(r for r in maymutable if getphase(repo, r))
return frozenset() return frozenset()
def computeimpactable(repo, visibilityexceptions=None): def computeimpactable(repo, visibilityexceptions=None):
"""Everything impactable by mutable revision """Everything impactable by mutable revision
The immutable filter still have some chance to get invalidated. This will The immutable filter still have some chance to get invalidated. This will
happen when: happen when:
- you garbage collect hidden changeset, - you garbage collect hidden changeset,
- public phase is moved backward, - public phase is moved backward,
- something is changed in the filtering (this could be fixed) - something is changed in the filtering (this could be fixed)
This filter out any mutable changeset and any public changeset that may be This filter out any mutable changeset and any public changeset that may be
impacted by something happening to a mutable revision. impacted by something happening to a mutable revision.
This is achieved by filtered everything with a revision number egal or This is achieved by filtered everything with a revision number egal or
higher than the first mutable changeset is filtered.""" higher than the first mutable changeset is filtered."""
assert not repo.changelog.filteredrevs assert not repo.changelog.filteredrevs
cl = repo.changelog cl = repo.changelog
firstmutable = len(cl) firstmutable = len(cl)
for roots in repo._phasecache.phaseroots[1:]: roots = repo._phasecache.nonpublicphaseroots(repo)
mjacobUnsubmitted
Done

You could let trackedphaseroots() return only the values directly.

mjacob: You could let `trackedphaseroots()` return only the values directly.
if roots: if roots:
firstmutable = min(firstmutable, min(cl.rev(r) for r in roots)) firstmutable = min(firstmutable, min(cl.rev(r) for r in roots))
# protect from nullrev root # protect from nullrev root
firstmutable = max(0, firstmutable) firstmutable = max(0, firstmutable)
return frozenset(pycompat.xrange(firstmutable, len(cl))) return frozenset(pycompat.xrange(firstmutable, len(cl)))
# function to compute filtered set # function to compute filtered set
# #
# When adding a new filter you MUST update the table at: # When adding a new filter you MUST update the table at:

tests/test-parseindex.t

> from __future__ import print_function > from __future__ import print_function
> import sys > import sys
> from mercurial import changelog, pycompat, vfs > from mercurial import changelog, pycompat, vfs
> cl = changelog.changelog(vfs.vfs(pycompat.fsencode(sys.argv[1]))) > cl = changelog.changelog(vfs.vfs(pycompat.fsencode(sys.argv[1])))
> n0, n1 = cl.node(0), cl.node(1) > n0, n1 = cl.node(0), cl.node(1)
> ops = [ > ops = [
> ('reachableroots', > ('reachableroots',
> lambda: cl.index.reachableroots2(0, [1], [0], False)), > lambda: cl.index.reachableroots2(0, [1], [0], False)),
> ('compute_phases_map_sets', lambda: cl.computephases([[0], []])), > ('compute_phases_map_sets', lambda: cl.computephases({1: {cl.node(0)}})),
> ('index_headrevs', lambda: cl.headrevs()), > ('index_headrevs', lambda: cl.headrevs()),
> ('find_gca_candidates', lambda: cl.commonancestorsheads(n0, n1)), > ('find_gca_candidates', lambda: cl.commonancestorsheads(n0, n1)),
> ('find_deepest', lambda: cl.ancestor(n0, n1)), > ('find_deepest', lambda: cl.ancestor(n0, n1)),
> ] > ]
> for l, f in ops: > for l, f in ops:
> print(l + ':', end=' ') > print(l + ':', end=' ')
> try: > try:
> f() > f()