[PATCH 1 of 2 obs-speedup] obsolete: add C implementation of _addsuccessors
Augie Fackler
raf at durin42.com
Mon Aug 24 13:52:01 CDT 2015
On Mon, Aug 24, 2015 at 11:46:16AM -0700, Sean Farley wrote:
>
> Augie Fackler <raf at durin42.com> writes:
>
> > # HG changeset patch
> > # User Augie Fackler <augie at google.com>
> > # Date 1440439282 14400
> > # Mon Aug 24 14:01:22 2015 -0400
> > # Node ID 3aecfdbfe3ca54134cdb0e98944e8fca53bd46f1
> > # Parent 207e95720308518aa40c2a91ce20fdf52dfa8550
> > obsolete: add C implementation of _addsuccessors
> >
> > Before, best of 10 runs:
> > hg sl > /dev/null 0.75s user 0.07s system 99% cpu 0.828 total
> >
> > After, best of 10 runs:
> > ./hg sl > /dev/null 0.65s user 0.09s system 99% cpu 0.744 total
> >
> > where
> > alias.sl=log -Gr smart -Tsl
> > revsetalias.smart=(parents(not public()) or not public() or . or head()) and (not obsolete() or unstable()^)
> >
> > It's a straight-line port to C code, but still a 10% win on walltime
> > for the command. I can live with that.
> >
> > diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
> > --- a/mercurial/obsolete.py
> > +++ b/mercurial/obsolete.py
> > @@ -470,10 +470,16 @@ class marker(object):
> > return self._data[2]
> >
> > @util.nogc
> > -def _addsuccessors(successors, markers):
> > +def _addsuccessors_pure(successors, markers):
> > for mark in markers:
> > successors.setdefault(mark[0], set()).add(mark)
> >
> > +def _addsuccessors(successors, markers):
> > + global _addsuccessors
> > + fn = getattr(parsers, 'addsuccessors', _addsuccessors_pure)
> > + _addsuccessors = fn
> > + return fn(successors, markers)
> > +
>
> Nit: is this how we do C function stuff? With globals? Honestly asking
> here.
It might be too clever, but basically I'm causing the loader stub to
disappear once we know (at runtime) which implementation we're going
to use.
>
> > @util.nogc
> > def _addprecursors(precursors, markers):
> > for mark in markers:
> > diff --git a/mercurial/parsers.c b/mercurial/parsers.c
> > --- a/mercurial/parsers.c
> > +++ b/mercurial/parsers.c
> > @@ -2700,6 +2700,46 @@ bail:
> > return NULL;
> > }
> >
> > +static PyObject *addsuccessors(PyObject *self, PyObject *args) {
> > + Py_ssize_t i, len;
> > + PyObject *succs = NULL, *marks = NULL;
> > +
> > + if (!PyArg_ParseTuple(args, "O!O!", &PyDict_Type, &succs,
> > + &PyList_Type, &marks)) {
>
> Minor nit: if my calculations are correct, this line has one too many
> tabs.
My emacs with vaguely-kernel-ish settings produced this. My personal
opinion is that this line should have exactly one tab and then spaces,
but I'm in the minority here.
>
> > + return NULL;
> > + }
> > + len = PyList_Size(marks);
> > + for (i = 0; i < len; i++) {
> > + PyObject *mark, *key, *dest;
> > + mark = PyList_GetItem(marks, i);
> > + if (!mark) {
> > + return NULL;
> > + }
> > + key = PyTuple_GetItem(mark, 0);
> > + if (!key) {
> > + return NULL;
> > + }
> > + if (PyDict_Contains(succs, key)) {
> > + dest = PyDict_GetItem(succs, key);
> > + } else {
> > + int r;
> > + dest = PySet_New(NULL);
> > + if (!dest)
> > + return NULL;
> > + r = PyDict_SetItem(succs, key, dest);
> > + Py_DECREF(dest);
> > + if (r)
> > + return NULL;
> > + }
> > + if (!dest)
> > + return NULL;
> > + if (PySet_Add(dest, mark))
> > + return NULL;
> > + }
> > + Py_INCREF(Py_None);
> > + return Py_None;
> > +}
> > +
> > static char parsers_doc[] = "Efficient content parsing.";
> >
> > PyObject *encodedir(PyObject *self, PyObject *args);
> > @@ -2722,6 +2762,8 @@ static PyMethodDef methods[] = {
> > {"lowerencode", lowerencode, METH_VARARGS, "lower-encode a path\n"},
> > {"fm1readmarkers", fm1readmarkers, METH_VARARGS,
> > "parse v1 obsolete markers\n"},
> > + {"addsuccessors", addsuccessors, METH_VARARGS,
> > + "add successors to a dict\n"},
> > {NULL, NULL}
> > };
> >
> > diff --git a/mercurial/revset.py b/mercurial/revset.py
> > --- a/mercurial/revset.py
> > +++ b/mercurial/revset.py
> > @@ -2945,6 +2945,7 @@ class abstractsmartset(object):
> >
> > This is part of the mandatory API for smartset."""
> > c = other.__contains__
> > + # this is slow?
> > return self.filter(lambda r: not c(r), cache=False)
> >
> > def filter(self, condition, cache=True):
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at selenic.com
> > https://selenic.com/mailman/listinfo/mercurial-devel
>
More information about the Mercurial-devel
mailing list