# HG changeset patch
# User Gregory Szorc <gregory.sz...@gmail.com>
# Date 1489467613 25200
#      Mon Mar 13 22:00:13 2017 -0700
# Node ID beda0762692f4252b049472b7d653abc5e809151
# Parent  b4159d4c3683f03b6962991f318b080d849d6ba3
[RFC] parsers: inline fm1readmarker() into Obsmarker1_FromData()

As promised by the previous commit.

Some moderate refactoring was performed to eliminate the various
PyObject variables and to assign directly into the returned
value.

We see another speedup on `hg perfloadmarkers`:

! result: 130341
! wall 0.085715 comb 0.080000 user 0.060000 sys 0.020000 (best of 100
! wall 0.077677 comb 0.080000 user 0.060000 sys 0.020000 (best of 100)

That is likely due eliminating the temporary tuple instance.

diff --git a/mercurial/parsers.c b/mercurial/parsers.c
--- a/mercurial/parsers.c
+++ b/mercurial/parsers.c
@@ -2698,24 +2698,27 @@ static void obsmarker1_dealloc(obsmarker
        PyObject_Del(self);
 }
 
-static PyObject *fm1readmarker(const char *databegin, const char *dataend,
-                              uint32_t *msize);
-
 /**
  * Construct an obsmarker1Object from raw data.
  */
 static obsmarker1Object * Obsmarker1_FromData(const char *databegin,
                                              const char *dataend,
-                                             uint32_t *msize) {
-
-       PyObject *tuple;
+                                             uint32_t *msize)
+{
        obsmarker1Object *result = NULL;
-
-       /* For now, we call the existing function to parse to a tuple
-          and convert to our new type. */
-       tuple = fm1readmarker(databegin, dataend, msize);
-       if (tuple == NULL) {
-               return NULL;
+       const char *data = databegin;
+       const char *meta;
+
+       double mtime;
+       int16_t tz;
+       uint16_t flags;
+       unsigned char nsuccs, nparents, nmetadata;
+       Py_ssize_t hashwidth = 20;
+       PyObject *mtimeo, *tzo;
+       int i;
+
+       if (data + FM1_HEADER_SIZE > dataend) {
+               goto overflow;
        }
 
        result = (obsmarker1Object *)PyObject_CallObject(
@@ -2724,24 +2727,126 @@ static obsmarker1Object * Obsmarker1_Fro
                goto bail;
        }
 
-       result->precursors = PyTuple_GET_ITEM(tuple, 0);
-       Py_INCREF(result->precursors);
-       result->successors = PyTuple_GET_ITEM(tuple, 1);
-       Py_INCREF(result->successors);
-       result->flags = PyTuple_GET_ITEM(tuple, 2);
-       Py_INCREF(result->flags);
-       result->metadata = PyTuple_GET_ITEM(tuple, 3);
-       Py_INCREF(result->metadata);
-       result->time = PyTuple_GET_ITEM(tuple, 4);
-       Py_INCREF(result->time);
-       result->parents = PyTuple_GET_ITEM(tuple, 5);
-       Py_INCREF(result->parents);
-
-       Py_CLEAR(tuple);
-
+       *msize = getbe32(data);
+       data += 4;
+       mtime = getbefloat64(data);
+       data += 8;
+       tz = getbeint16(data);
+       data += 2;
+       flags = getbeuint16(data);
+       data += 2;
+
+       result->flags = PyLong_FromUnsignedLong(flags);
+       if (result->flags == NULL) {
+               Py_CLEAR(result);
+               goto bail;
+       }
+
+       result->time = PyTuple_New(2);
+       if (result->time == NULL) {
+               Py_CLEAR(result);
+               goto bail;
+       }
+
+       mtimeo = PyFloat_FromDouble(mtime);
+       tzo = PyLong_FromUnsignedLong(tz * 60);
+       if (!mtimeo || !tzo) {
+               Py_XDECREF(mtimeo);
+               Py_XDECREF(tzo);
+               Py_CLEAR(result);
+               goto bail;
+       }
+
+       PyTuple_SET_ITEM(result->time, 0, mtimeo);
+       PyTuple_SET_ITEM(result->time, 1, tzo);
+
+       if (flags & USING_SHA_256) {
+               hashwidth = 32;
+       }
+
+       nsuccs = (unsigned char)(*data++);
+       nparents = (unsigned char)(*data++);
+       nmetadata = (unsigned char)(*data++);
+
+       if (databegin + *msize > dataend) {
+               goto overflow;
+       }
+       dataend = databegin + *msize;  /* narrow down to marker size */
+
+       if (data + hashwidth > dataend) {
+               goto overflow;
+       }
+
+       result->precursors = PyBytes_FromStringAndSize(data, hashwidth);
+       data += hashwidth;
+       if (result->precursors == NULL) {
+               Py_CLEAR(result);
+               goto bail;
+       }
+
+       if (data + nsuccs * hashwidth > dataend) {
+               goto overflow;
+       }
+       result->successors = readshas(data, nsuccs, hashwidth);
+       if (result->successors == NULL) {
+               Py_CLEAR(result);
+               goto bail;
+       }
+       data += nsuccs * hashwidth;
+
+       if (nparents == 1 || nparents == 2) {
+               if (data + nparents * hashwidth > dataend) {
+                       goto overflow;
+               }
+               result->parents = readshas(data, nparents, hashwidth);
+               if (result->parents == NULL) {
+                       Py_CLEAR(result);
+                       goto bail;
+               }
+               data += nparents * hashwidth;
+       } else {
+               result->parents = Py_None;
+               Py_INCREF(Py_None);
+       }
+
+       if (data + 2 * nmetadata > dataend) {
+               goto overflow;
+       }
+       meta = data + (2 * nmetadata);
+       result->metadata = PyTuple_New(nmetadata);
+       if (result->metadata == NULL) {
+               Py_CLEAR(result);
+               goto bail;
+       }
+       for (i = 0; i < nmetadata; i++) {
+               PyObject *tmp, *left = NULL, *right = NULL;
+               Py_ssize_t leftsize = (unsigned char)(*data++);
+               Py_ssize_t rightsize = (unsigned char)(*data++);
+               if (meta + leftsize + rightsize > dataend) {
+                       goto overflow;
+               }
+               left = PyBytes_FromStringAndSize(meta, leftsize);
+               meta += leftsize;
+               right = PyBytes_FromStringAndSize(meta, rightsize);
+               meta += rightsize;
+               tmp = PyTuple_New(2);
+               if (!left || !right || !tmp) {
+                       Py_XDECREF(left);
+                       Py_XDECREF(right);
+                       Py_XDECREF(tmp);
+                       goto bail;
+               }
+               PyTuple_SET_ITEM(tmp, 0, left);
+               PyTuple_SET_ITEM(tmp, 1, right);
+               PyTuple_SET_ITEM(result->metadata, i, tmp);
+       }
+
+       goto bail;
+
+overflow:
+       Py_CLEAR(result);
+       PyErr_SetString(PyExc_ValueError, "overflow in obsstore");
 bail:
-       Py_XDECREF(tuple);
-
        return result;
 }
 
@@ -2854,125 +2959,6 @@ PyTypeObject obsmarker1Type = {
        PyType_GenericNew, /* tp_new */
 };
 
-static PyObject *fm1readmarker(const char *databegin, const char *dataend,
-                              uint32_t *msize)
-{
-       const char *data = databegin;
-       const char *meta;
-
-       double mtime;
-       int16_t tz;
-       uint16_t flags;
-       unsigned char nsuccs, nparents, nmetadata;
-       Py_ssize_t hashwidth = 20;
-
-       PyObject *prec = NULL, *parents = NULL, *succs = NULL;
-       PyObject *metadata = NULL, *ret = NULL;
-       int i;
-
-       if (data + FM1_HEADER_SIZE > dataend) {
-               goto overflow;
-       }
-
-       *msize = getbe32(data);
-       data += 4;
-       mtime = getbefloat64(data);
-       data += 8;
-       tz = getbeint16(data);
-       data += 2;
-       flags = getbeuint16(data);
-       data += 2;
-
-       if (flags & USING_SHA_256) {
-               hashwidth = 32;
-       }
-
-       nsuccs = (unsigned char)(*data++);
-       nparents = (unsigned char)(*data++);
-       nmetadata = (unsigned char)(*data++);
-
-       if (databegin + *msize > dataend) {
-               goto overflow;
-       }
-       dataend = databegin + *msize;  /* narrow down to marker size */
-
-       if (data + hashwidth > dataend) {
-               goto overflow;
-       }
-       prec = PyBytes_FromStringAndSize(data, hashwidth);
-       data += hashwidth;
-       if (prec == NULL) {
-               goto bail;
-       }
-
-       if (data + nsuccs * hashwidth > dataend) {
-               goto overflow;
-       }
-       succs = readshas(data, nsuccs, hashwidth);
-       if (succs == NULL) {
-               goto bail;
-       }
-       data += nsuccs * hashwidth;
-
-       if (nparents == 1 || nparents == 2) {
-               if (data + nparents * hashwidth > dataend) {
-                       goto overflow;
-               }
-               parents = readshas(data, nparents, hashwidth);
-               if (parents == NULL) {
-                       goto bail;
-               }
-               data += nparents * hashwidth;
-       } else {
-               parents = Py_None;
-               Py_INCREF(parents);
-       }
-
-       if (data + 2 * nmetadata > dataend) {
-               goto overflow;
-       }
-       meta = data + (2 * nmetadata);
-       metadata = PyTuple_New(nmetadata);
-       if (metadata == NULL) {
-               goto bail;
-       }
-       for (i = 0; i < nmetadata; i++) {
-               PyObject *tmp, *left = NULL, *right = NULL;
-               Py_ssize_t leftsize = (unsigned char)(*data++);
-               Py_ssize_t rightsize = (unsigned char)(*data++);
-               if (meta + leftsize + rightsize > dataend) {
-                       goto overflow;
-               }
-               left = PyBytes_FromStringAndSize(meta, leftsize);
-               meta += leftsize;
-               right = PyBytes_FromStringAndSize(meta, rightsize);
-               meta += rightsize;
-               tmp = PyTuple_New(2);
-               if (!left || !right || !tmp) {
-                       Py_XDECREF(left);
-                       Py_XDECREF(right);
-                       Py_XDECREF(tmp);
-                       goto bail;
-               }
-               PyTuple_SET_ITEM(tmp, 0, left);
-               PyTuple_SET_ITEM(tmp, 1, right);
-               PyTuple_SET_ITEM(metadata, i, tmp);
-       }
-       ret = Py_BuildValue("(OOHO(di)O)", prec, succs, flags,
-                           metadata, mtime, (int)tz * 60, parents);
-       goto bail;  /* return successfully */
-
-overflow:
-       PyErr_SetString(PyExc_ValueError, "overflow in obsstore");
-bail:
-       Py_XDECREF(prec);
-       Py_XDECREF(succs);
-       Py_XDECREF(metadata);
-       Py_XDECREF(parents);
-       return ret;
-}
-
-
 static PyObject *fm1readmarkers(PyObject *self, PyObject *args) {
        const char *data, *dataend;
        int datalen;
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to