revlog: C implementation of delta chain resolution
authorGregory Szorc <gregory.szorc@gmail.com>
Sun, 25 Jun 2017 12:41:34 -0700
changeset 33171 6d678ab1b10d
parent 33166 5c9ad50fd62f
child 33172 0830c841fc7f
revlog: C implementation of delta chain resolution I've seen revlog._deltachain() appear in a number of performance profiles. I suspect there are 2 reasons for this: 1. Delta chain resolution performs many index lookups, thus triggering population of index tuples. Creating possibly tens of thousands of PyObject will have overhead. 2. Delta chain resolution is a tight loop. By moving delta chain resolution to C, we can defer instantiation of full index entry tuples and make the loop faster courtesy of not running in Python. We can measure the impact to delta chain resolution via `hg perflogrevision` using the mozilla-central repo with a recent manifest having delta chain length of 33726: $ hg perfrevlogrevision -m 364895 ! full ! wall 0.367585 comb 0.370000 user 0.340000 sys 0.030000 (best of 27) ! wall 0.357581 comb 0.360000 user 0.350000 sys 0.010000 (best of 28) ! deltachain ! wall 0.010644 comb 0.010000 user 0.010000 sys 0.000000 (best of 270) ! wall 0.000292 comb 0.000000 user 0.000000 sys 0.000000 (best of 8729) $ hg perfrevlogrevision --cache -m 364895 ! deltachain ! wall 0.003904 comb 0.000000 user 0.000000 sys 0.000000 (best of 712) ! wall 0.000284 comb 0.000000 user 0.000000 sys 0.000000 (best of 9926) The first test measures savings from both not instantiating index entries and moving to C. The second test (which doesn't clear the index caches) essentially isolates the benefits of moving from Python to C. It still shows a 13.7x speedup (versus 36.4x). And there are multiple milliseconds of savings within the critical path for resolving revision data. I think that justifies the existence of C code. A more striking example of the benefits of this change can be demonstrated by timing `hg debugdeltachain -m` for the mozilla-central repo: $ time hg debugdeltachain -m > /dev/null before: 1057.4s after: 503.3s PyPy2.7 5.8.0: 220.0s It's worth noting that the C code isn't as optimal as it could be. We're still instantiating a new PyObject for every revision. A future optimization would be to reuse the PyObject on the cached index tuple. We could potentially also get wins by using a memory array of raw integers. There is also room for a delta chain cache on revlog instances. Of course, the best optimization is to implement revlog reading outside of Python so Python doesn't need to be concerned about the relatively expensive index entries and operations on them.
mercurial/cext/revlog.c
mercurial/revlog.py
--- a/mercurial/cext/revlog.c	Wed Jun 28 13:32:36 2017 +0200
+++ b/mercurial/cext/revlog.c	Sun Jun 25 12:41:34 2017 -0700
@@ -816,6 +816,139 @@
 	return NULL;
 }
 
+static inline int index_baserev(indexObject *self, int rev)
+{
+	const char *data;
+
+	if (rev >= self->length - 1) {
+		PyObject *tuple = PyList_GET_ITEM(self->added,
+			rev - self->length + 1);
+		return (int)PyInt_AS_LONG(PyTuple_GET_ITEM(tuple, 3));
+	}
+	else {
+		data = index_deref(self, rev);
+		if (data == NULL) {
+			return -2;
+		}
+
+		return getbe32(data + 16);
+	}
+}
+
+static PyObject *index_deltachain(indexObject *self, PyObject *args)
+{
+	int rev, generaldelta;
+	PyObject *stoparg;
+	int stoprev, iterrev, baserev = -1;
+	int stopped;
+	PyObject *chain = NULL, *value = NULL, *result = NULL;
+	const Py_ssize_t length = index_length(self);
+
+	if (!PyArg_ParseTuple(args, "iOi", &rev, &stoparg, &generaldelta)) {
+		return NULL;
+	}
+
+	if (PyInt_Check(stoparg)) {
+		stoprev = (int)PyInt_AsLong(stoparg);
+		if (stoprev == -1 && PyErr_Occurred()) {
+			return NULL;
+		}
+	}
+	else if (stoparg == Py_None) {
+		stoprev = -2;
+	}
+	else {
+		PyErr_SetString(PyExc_ValueError,
+			"stoprev must be integer or None");
+		return NULL;
+	}
+
+	if (rev < 0 || rev >= length - 1) {
+		PyErr_SetString(PyExc_ValueError, "revlog index out of range");
+		return NULL;
+	}
+
+	chain = PyList_New(0);
+	if (chain == NULL) {
+		return NULL;
+	}
+
+	baserev = index_baserev(self, rev);
+
+	/* This should never happen. */
+	if (baserev == -2) {
+		PyErr_SetString(PyExc_IndexError, "unable to resolve data");
+		goto bail;
+	}
+
+	iterrev = rev;
+
+	while (iterrev != baserev && iterrev != stoprev) {
+		value = PyInt_FromLong(iterrev);
+		if (value == NULL) {
+			goto bail;
+		}
+		if (PyList_Append(chain, value)) {
+			Py_DECREF(value);
+			goto bail;
+		}
+		Py_DECREF(value);
+
+		if (generaldelta) {
+			iterrev = baserev;
+		}
+		else {
+			iterrev--;
+		}
+
+		if (iterrev < 0) {
+			break;
+		}
+
+		if (iterrev >= length - 1) {
+			PyErr_SetString(PyExc_IndexError, "revision outside index");
+			return NULL;
+		}
+
+		baserev = index_baserev(self, iterrev);
+
+		/* This should never happen. */
+		if (baserev == -2) {
+			PyErr_SetString(PyExc_IndexError, "unable to resolve data");
+			goto bail;
+		}
+	}
+
+	if (iterrev == stoprev) {
+		stopped = 1;
+	}
+	else {
+		value = PyInt_FromLong(iterrev);
+		if (value == NULL) {
+			goto bail;
+		}
+		if (PyList_Append(chain, value)) {
+			Py_DECREF(value);
+			goto bail;
+		}
+		Py_DECREF(value);
+
+		stopped = 0;
+	}
+
+	if (PyList_Reverse(chain)) {
+		goto bail;
+	}
+
+	result = Py_BuildValue("OO", chain, stopped ? Py_True : Py_False);
+	Py_DECREF(chain);
+	return result;
+
+bail:
+	Py_DECREF(chain);
+	return NULL;
+}
+
 static inline int nt_level(const char *node, Py_ssize_t level)
 {
 	int v = node[level>>1];
@@ -1828,6 +1961,8 @@
 	 "get head revisions"}, /* Can do filtering since 3.2 */
 	{"headrevsfiltered", (PyCFunction)index_headrevs, METH_VARARGS,
 	 "get filtered head revisions"}, /* Can always do filtering */
+	{"deltachain", (PyCFunction)index_deltachain, METH_VARARGS,
+	 "determine revisions with deltas to reconstruct fulltext"},
 	{"insert", (PyCFunction)index_insert, METH_VARARGS,
 	 "insert an index entry"},
 	{"partialmatch", (PyCFunction)index_partialmatch, METH_VARARGS,
--- a/mercurial/revlog.py	Wed Jun 28 13:32:36 2017 +0200
+++ b/mercurial/revlog.py	Sun Jun 25 12:41:34 2017 -0700
@@ -570,6 +570,12 @@
         revs in ascending order and ``stopped`` is a bool indicating whether
         ``stoprev`` was hit.
         """
+        # Try C implementation.
+        try:
+            return self.index.deltachain(rev, stoprev, self._generaldelta)
+        except AttributeError:
+            pass
+
         chain = []
 
         # Alias to prevent attribute lookup in tight loop.