Mercurial > hg

changeset 21809:e250b8300e6e

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
parsers: inline fields of dirstate values in C version Previously, while unpacking the dirstate we'd create 3-4 new CPython objects for most dirstate values: - the state is a single character string, which is pooled by CPython - the mode is a new object if it isn't 0 due to being in the lookup set - the size is a new object if it is greater than 255 - the mtime is a new object if it isn't -1 due to being in the lookup set - the tuple to contain them all In some cases such as regular hg status, we actually look at all the objects. In other cases like hg add, hg status for a subdirectory, or hg status with the third-party hgwatchman enabled, we look at almost none of the objects. This patch eliminates most object creation in these cases by defining a custom C struct that is exposed to Python with an interface similar to a tuple. Only when tuple elements are actually requested are the respective objects created. The gains, where they're expected, are significant. The following tests are run against a working copy with over 270,000 files. parse_dirstate becomes significantly faster: $ hg perfdirstate before: wall 0.186437 comb 0.180000 user 0.160000 sys 0.020000 (best of 35) after: wall 0.093158 comb 0.100000 user 0.090000 sys 0.010000 (best of 95) and as a result, several commands benefit: $ time hg status # with hgwatchman enabled before: 0.42s user 0.14s system 99% cpu 0.563 total after: 0.34s user 0.12s system 99% cpu 0.471 total $ time hg add new-file before: 0.85s user 0.18s system 99% cpu 1.033 total after: 0.76s user 0.17s system 99% cpu 0.931 total There is a slight regression in regular status performance, but this is fixed in an upcoming patch.
author Siddharth Agarwal <sid0@fb.com>
date Tue, 27 May 2014 14:27:41 -0700
parents 7537e57f5dbd
children 4b2ebd3187a1
files mercurial/dirs.c mercurial/dirstate.py mercurial/parsers.c mercurial/pure/parsers.py mercurial/util.h
diffstat 5 files changed, 161 insertions(+), 45 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial/dirs.c	Tue May 27 17:10:28 2014 -0700
+++ b/mercurial/dirs.c	Tue May 27 14:27:41 2014 -0700
@@ -138,25 +138,12 @@
 			return -1;
 		}
 		if (skipchar) {
-			PyObject *st;
-
-			if (!PyTuple_Check(value) ||
-			    PyTuple_GET_SIZE(value) == 0) {
+			if (!dirstate_tuple_check(value)) {
 				PyErr_SetString(PyExc_TypeError,
-						"expected non-empty tuple");
+						"expected a dirstate tuple");
 				return -1;
 			}
-
-			st = PyTuple_GET_ITEM(value, 0);
-
-			if (!PyString_Check(st) || PyString_GET_SIZE(st) == 0) {
-				PyErr_SetString(PyExc_TypeError,
-						"expected non-empty string "
-						"at tuple index 0");
-				return -1;
-			}
-
-			if (PyString_AS_STRING(st)[0] == skipchar)
+			if (((dirstateTupleObject *)value)->state == skipchar)
 				continue;
 		}
 
--- a/mercurial/dirstate.py	Tue May 27 17:10:28 2014 -0700
+++ b/mercurial/dirstate.py	Tue May 27 14:27:41 2014 -0700
@@ -14,9 +14,7 @@
 filecache = scmutil.filecache
 _rangemask = 0x7fffffff
 
-def dirstatetuple(*x):
-    # x is a tuple
-    return x
+dirstatetuple = parsers.dirstatetuple
 
 class repocache(filecache):
     """filecache for files in .hg/"""
--- a/mercurial/parsers.c	Tue May 27 17:10:28 2014 -0700
+++ b/mercurial/parsers.c	Tue May 27 14:27:41 2014 -0700
@@ -153,6 +153,122 @@
 	return NULL;
 }
 
+static inline dirstateTupleObject *make_dirstate_tuple(char state, int mode,
+						       int size, int mtime)
+{
+	dirstateTupleObject *t = PyObject_New(dirstateTupleObject,
+					      &dirstateTupleType);
+	if (!t)
+		return NULL;
+	t->state = state;
+	t->mode = mode;
+	t->size = size;
+	t->mtime = mtime;
+	return t;
+}
+
+static PyObject *dirstate_tuple_new(PyTypeObject *subtype, PyObject *args,
+				    PyObject *kwds)
+{
+	/* We do all the initialization here and not a tp_init function because
+	 * dirstate_tuple is immutable. */
+	dirstateTupleObject *t;
+	char state;
+	int size, mode, mtime;
+	if (!PyArg_ParseTuple(args, "ciii", &state, &mode, &size, &mtime))
+		return NULL;
+
+	t = (dirstateTupleObject *)subtype->tp_alloc(subtype, 1);
+	if (!t)
+		return NULL;
+	t->state = state;
+	t->mode = mode;
+	t->size = size;
+	t->mtime = mtime;
+
+	return (PyObject *)t;
+}
+
+static void dirstate_tuple_dealloc(PyObject *o)
+{
+	PyObject_Del(o);
+}
+
+static Py_ssize_t dirstate_tuple_length(PyObject *o)
+{
+	return 4;
+}
+
+static PyObject *dirstate_tuple_item(PyObject *o, Py_ssize_t i)
+{
+	dirstateTupleObject *t = (dirstateTupleObject *)o;
+	switch (i) {
+	case 0:
+		return PyBytes_FromStringAndSize(&t->state, 1);
+	case 1:
+		return PyInt_FromLong(t->mode);
+	case 2:
+		return PyInt_FromLong(t->size);
+	case 3:
+		return PyInt_FromLong(t->mtime);
+	default:
+		PyErr_SetString(PyExc_IndexError, "index out of range");
+		return NULL;
+	}
+}
+
+static PySequenceMethods dirstate_tuple_sq = {
+	dirstate_tuple_length,     /* sq_length */
+	0,                         /* sq_concat */
+	0,                         /* sq_repeat */
+	dirstate_tuple_item,       /* sq_item */
+	0,                         /* sq_ass_item */
+	0,                         /* sq_contains */
+	0,                         /* sq_inplace_concat */
+	0                          /* sq_inplace_repeat */
+};
+
+PyTypeObject dirstateTupleType = {
+	PyVarObject_HEAD_INIT(NULL, 0)
+	"dirstate_tuple",          /* tp_name */
+	sizeof(dirstateTupleObject),/* tp_basicsize */
+	0,                         /* tp_itemsize */
+	(destructor)dirstate_tuple_dealloc, /* tp_dealloc */
+	0,                         /* tp_print */
+	0,                         /* tp_getattr */
+	0,                         /* tp_setattr */
+	0,                         /* tp_compare */
+	0,                         /* tp_repr */
+	0,                         /* tp_as_number */
+	&dirstate_tuple_sq,        /* tp_as_sequence */
+	0,                         /* tp_as_mapping */
+	0,                         /* tp_hash  */
+	0,                         /* tp_call */
+	0,                         /* tp_str */
+	0,                         /* tp_getattro */
+	0,                         /* tp_setattro */
+	0,                         /* tp_as_buffer */
+	Py_TPFLAGS_DEFAULT,        /* tp_flags */
+	"dirstate tuple",          /* tp_doc */
+	0,                         /* tp_traverse */
+	0,                         /* tp_clear */
+	0,                         /* tp_richcompare */
+	0,                         /* tp_weaklistoffset */
+	0,                         /* tp_iter */
+	0,                         /* tp_iternext */
+	0,                         /* tp_methods */
+	0,                         /* tp_members */
+	0,                         /* tp_getset */
+	0,                         /* tp_base */
+	0,                         /* tp_dict */
+	0,                         /* tp_descr_get */
+	0,                         /* tp_descr_set */
+	0,                         /* tp_dictoffset */
+	0,                         /* tp_init */
+	0,                         /* tp_alloc */
+	dirstate_tuple_new,        /* tp_new */
+};
+
 static PyObject *parse_dirstate(PyObject *self, PyObject *args)
 {
 	PyObject *dmap, *cmap, *parents = NULL, *ret = NULL;
@@ -192,11 +308,8 @@
 			goto quit;
 		}
 
-		entry = Py_BuildValue("ciii", state, mode, size, mtime);
-		if (!entry)
-			goto quit;
-		PyObject_GC_UnTrack(entry); /* don't waste time with this */
-
+		entry = (PyObject *)make_dirstate_tuple(state, mode, size,
+							mtime);
 		cpos = memchr(cur, 0, flen);
 		if (cpos) {
 			fname = PyBytes_FromStringAndSize(cur, cpos - cur);
@@ -316,33 +429,30 @@
 	p += 20;
 
 	for (pos = 0; PyDict_Next(map, &pos, &k, &v); ) {
+		dirstateTupleObject *tuple;
+		char state;
 		uint32_t mode, size, mtime;
 		Py_ssize_t len, l;
 		PyObject *o;
-		char *s, *t;
+		char *t;
 
-		if (!PyTuple_Check(v) || PyTuple_GET_SIZE(v) != 4) {
-			PyErr_SetString(PyExc_TypeError, "expected a 4-tuple");
+		if (!dirstate_tuple_check(v)) {
+			PyErr_SetString(PyExc_TypeError,
+					"expected a dirstate tuple");
 			goto bail;
 		}
-		o = PyTuple_GET_ITEM(v, 0);
-		if (PyString_AsStringAndSize(o, &s, &l) == -1 || l != 1) {
-			PyErr_SetString(PyExc_TypeError, "expected one byte");
-			goto bail;
-		}
-		*p++ = *s;
-		if (getintat(v, 1, &mode) == -1)
-			goto bail;
-		if (getintat(v, 2, &size) == -1)
-			goto bail;
-		if (getintat(v, 3, &mtime) == -1)
-			goto bail;
-		if (*s == 'n' && mtime == (uint32_t)now) {
+		tuple = (dirstateTupleObject *)v;
+
+		state = tuple->state;
+		mode = tuple->mode;
+		size = tuple->size;
+		mtime = tuple->mtime;
+		if (state == 'n' && mtime == (uint32_t)now) {
 			/* See pure/parsers.py:pack_dirstate for why we do
 			 * this. */
 			mtime = -1;
-			mtime_unset = Py_BuildValue(
-				"ciii", *s, mode, size, mtime);
+			mtime_unset = (PyObject *)make_dirstate_tuple(
+				state, mode, size, mtime);
 			if (!mtime_unset)
 				goto bail;
 			if (PyDict_SetItem(map, k, mtime_unset) == -1)
@@ -350,6 +460,7 @@
 			Py_DECREF(mtime_unset);
 			mtime_unset = NULL;
 		}
+		*p++ = state;
 		putbe32(mode, p);
 		putbe32(size, p + 4);
 		putbe32(mtime, p + 8);
@@ -2021,11 +2132,14 @@
 	dirs_module_init(mod);
 
 	indexType.tp_new = PyType_GenericNew;
-	if (PyType_Ready(&indexType) < 0)
+	if (PyType_Ready(&indexType) < 0 ||
+	    PyType_Ready(&dirstateTupleType) < 0)
 		return;
 	Py_INCREF(&indexType);
-
 	PyModule_AddObject(mod, "index", (PyObject *)&indexType);
+	Py_INCREF(&dirstateTupleType);
+	PyModule_AddObject(mod, "dirstatetuple",
+			   (PyObject *)&dirstateTupleType);
 
 	nullentry = Py_BuildValue("iiiiiiis#", 0, 0, 0,
 				  -1, -1, -1, -1, nullid, 20);
--- a/mercurial/pure/parsers.py	Tue May 27 17:10:28 2014 -0700
+++ b/mercurial/pure/parsers.py	Tue May 27 14:27:41 2014 -0700
@@ -15,6 +15,12 @@
 _decompress = zlib.decompress
 _sha = util.sha1
 
+# Some code below makes tuples directly because it's more convenient. However,
+# code outside this module should always use dirstatetuple.
+def dirstatetuple(*x):
+    # x is a tuple
+    return x
+
 def parse_manifest(mfdict, fdict, lines):
     for l in lines.splitlines():
         f, n = l.split('\0')
@@ -104,7 +110,7 @@
             # dirstate, forcing future 'status' calls to compare the
             # contents of the file if the size is the same. This prevents
             # mistakenly treating such files as clean.
-            e = (e[0], e[1], e[2], -1)
+            e = dirstatetuple(e[0], e[1], e[2], -1)
             dmap[f] = e
 
         if f in copymap:
--- a/mercurial/util.h	Tue May 27 17:10:28 2014 -0700
+++ b/mercurial/util.h	Tue May 27 14:27:41 2014 -0700
@@ -151,6 +151,17 @@
 #define inline __inline
 #endif
 
+typedef struct {
+	PyObject_HEAD
+	char state;
+	int mode;
+	int size;
+	int mtime;
+} dirstateTupleObject;
+
+PyTypeObject dirstateTupleType;
+#define dirstate_tuple_check(op) (Py_TYPE(op) == &dirstateTupleType)
+
 static inline uint32_t getbe32(const char *c)
 {
 	const unsigned char *d = (const unsigned char *)c;