Skip to content

Commit 65e1cea

Browse files
committed
Convert to rich comparisons:
- sort's docompare() calls RichCompare(Py_LT). - list_contains(), list_index(), listcount(), listremove() call RichCompare(Py_EQ). - Get rid of list_compare(), in favor of new list_richcompare(). The latter does some nice shortcuts, like when == or != is requested, it first compares the lengths for trivial accept/reject. Then it goes over the items until it finds an index where the items differe; then it does more shortcut magic to minimize the number of additional comparisons. - Aligned the comments for large struct initializers.
1 parent 7d64577 commit 65e1cea

File tree

1 file changed

+163
-90
lines changed

1 file changed

+163
-90
lines changed

Objects/listobject.c

Lines changed: 163 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -245,19 +245,6 @@ list_repr(PyListObject *v)
245245
return s;
246246
}
247247

248-
static int
249-
list_compare(PyListObject *v, PyListObject *w)
250-
{
251-
int i;
252-
253-
for (i = 0; i < v->ob_size && i < w->ob_size; i++) {
254-
int cmp = PyObject_Compare(v->ob_item[i], w->ob_item[i]);
255-
if (cmp != 0)
256-
return cmp;
257-
}
258-
return v->ob_size - w->ob_size;
259-
}
260-
261248
static int
262249
list_length(PyListObject *a)
263250
{
@@ -269,13 +256,14 @@ list_length(PyListObject *a)
269256
static int
270257
list_contains(PyListObject *a, PyObject *el)
271258
{
272-
int i, cmp;
259+
int i;
273260

274261
for (i = 0; i < a->ob_size; ++i) {
275-
cmp = PyObject_Compare(el, PyList_GET_ITEM(a, i));
276-
if (cmp == 0)
262+
int cmp = PyObject_RichCompareBool(el, PyList_GET_ITEM(a, i),
263+
Py_EQ);
264+
if (cmp > 0)
277265
return 1;
278-
if (PyErr_Occurred())
266+
else if (cmp < 0)
279267
return -1;
280268
}
281269
return 0;
@@ -725,10 +713,16 @@ docompare(PyObject *x, PyObject *y, PyObject *compare)
725713
int i;
726714

727715
if (compare == NULL) {
728-
i = PyObject_Compare(x, y);
729-
if (i && PyErr_Occurred())
730-
i = CMPERROR;
731-
return i;
716+
/* NOTE: we rely on the fact here that the sorting algorithm
717+
only ever checks whether k<0, i.e., whether x<y. So we
718+
invoke the rich comparison function with Py_LT ('<'), and
719+
return -1 when it returns true and 0 when it returns
720+
false. */
721+
i = PyObject_RichCompareBool(x, y, Py_LT);
722+
if (i < 0)
723+
return CMPERROR;
724+
else
725+
return -i;
732726
}
733727

734728
args = Py_BuildValue("(OO)", x, y);
@@ -1344,9 +1338,10 @@ listindex(PyListObject *self, PyObject *args)
13441338
if (!PyArg_ParseTuple_Compat1(args, "O:index", &v))
13451339
return NULL;
13461340
for (i = 0; i < self->ob_size; i++) {
1347-
if (PyObject_Compare(self->ob_item[i], v) == 0)
1341+
int cmp = PyObject_RichCompareBool(self->ob_item[i], v, Py_EQ);
1342+
if (cmp > 0)
13481343
return PyInt_FromLong((long)i);
1349-
if (PyErr_Occurred())
1344+
else if (cmp < 0)
13501345
return NULL;
13511346
}
13521347
PyErr_SetString(PyExc_ValueError, "list.index(x): x not in list");
@@ -1363,9 +1358,10 @@ listcount(PyListObject *self, PyObject *args)
13631358
if (!PyArg_ParseTuple_Compat1(args, "O:count", &v))
13641359
return NULL;
13651360
for (i = 0; i < self->ob_size; i++) {
1366-
if (PyObject_Compare(self->ob_item[i], v) == 0)
1361+
int cmp = PyObject_RichCompareBool(self->ob_item[i], v, Py_EQ);
1362+
if (cmp > 0)
13671363
count++;
1368-
if (PyErr_Occurred())
1364+
else if (cmp < 0)
13691365
return NULL;
13701366
}
13711367
return PyInt_FromLong((long)count);
@@ -1380,14 +1376,15 @@ listremove(PyListObject *self, PyObject *args)
13801376
if (!PyArg_ParseTuple_Compat1(args, "O:remove", &v))
13811377
return NULL;
13821378
for (i = 0; i < self->ob_size; i++) {
1383-
if (PyObject_Compare(self->ob_item[i], v) == 0) {
1379+
int cmp = PyObject_RichCompareBool(self->ob_item[i], v, Py_EQ);
1380+
if (cmp > 0) {
13841381
if (list_ass_slice(self, i, i+1,
13851382
(PyObject *)NULL) != 0)
13861383
return NULL;
13871384
Py_INCREF(Py_None);
13881385
return Py_None;
13891386
}
1390-
if (PyErr_Occurred())
1387+
else if (cmp < 0)
13911388
return NULL;
13921389
}
13931390
PyErr_SetString(PyExc_ValueError, "list.remove(x): x not in list");
@@ -1418,6 +1415,78 @@ list_clear(PyListObject *lp)
14181415
return 0;
14191416
}
14201417

1418+
static PyObject *
1419+
list_richcompare(PyObject *v, PyObject *w, int op)
1420+
{
1421+
PyListObject *vl, *wl;
1422+
int i;
1423+
1424+
if (!PyList_Check(v) || !PyList_Check(w)) {
1425+
Py_INCREF(Py_NotImplemented);
1426+
return Py_NotImplemented;
1427+
}
1428+
1429+
vl = (PyListObject *)v;
1430+
wl = (PyListObject *)w;
1431+
1432+
if (vl->ob_size != wl->ob_size && (op == Py_EQ || op == Py_NE)) {
1433+
/* Shortcut: if the lengths differ, the lists differ */
1434+
PyObject *res;
1435+
if (op == Py_EQ)
1436+
res = Py_False;
1437+
else
1438+
res = Py_True;
1439+
Py_INCREF(res);
1440+
return res;
1441+
}
1442+
1443+
/* Search for the first index where items are different */
1444+
for (i = 0; i < vl->ob_size && i < wl->ob_size; i++) {
1445+
int k = PyObject_RichCompareBool(vl->ob_item[i],
1446+
wl->ob_item[i], Py_EQ);
1447+
if (k < 0)
1448+
return NULL;
1449+
if (!k)
1450+
break;
1451+
}
1452+
1453+
if (i >= vl->ob_size || i >= wl->ob_size) {
1454+
/* No more items to compare -- compare sizes */
1455+
int vs = vl->ob_size;
1456+
int ws = wl->ob_size;
1457+
int cmp;
1458+
PyObject *res;
1459+
switch (op) {
1460+
case Py_LT: cmp = vs < ws; break;
1461+
case Py_LE: cmp = ws <= ws; break;
1462+
case Py_EQ: cmp = vs == ws; break;
1463+
case Py_NE: cmp = vs != ws; break;
1464+
case Py_GT: cmp = vs > ws; break;
1465+
case Py_GE: cmp = vs >= ws; break;
1466+
default: return NULL; /* cannot happen */
1467+
}
1468+
if (cmp)
1469+
res = Py_True;
1470+
else
1471+
res = Py_False;
1472+
Py_INCREF(res);
1473+
return res;
1474+
}
1475+
1476+
/* We have an item that differs -- shortcuts for EQ/NE */
1477+
if (op == Py_EQ) {
1478+
Py_INCREF(Py_False);
1479+
return Py_False;
1480+
}
1481+
if (op == Py_NE) {
1482+
Py_INCREF(Py_True);
1483+
return Py_True;
1484+
}
1485+
1486+
/* Compare the final item again using the proper operator */
1487+
return PyObject_RichCompare(vl->ob_item[i], wl->ob_item[i], op);
1488+
}
1489+
14211490
static char append_doc[] =
14221491
"L.append(object) -- append object to end";
14231492
static char extend_doc[] =
@@ -1438,15 +1507,15 @@ static char sort_doc[] =
14381507
"L.sort([cmpfunc]) -- sort *IN PLACE*; if given, cmpfunc(x, y) -> -1, 0, 1";
14391508

14401509
static PyMethodDef list_methods[] = {
1441-
{"append", (PyCFunction)listappend, METH_VARARGS, append_doc},
1442-
{"insert", (PyCFunction)listinsert, METH_VARARGS, insert_doc},
1443-
{"extend", (PyCFunction)listextend, METH_VARARGS, extend_doc},
1444-
{"pop", (PyCFunction)listpop, METH_VARARGS, pop_doc},
1445-
{"remove", (PyCFunction)listremove, METH_VARARGS, remove_doc},
1446-
{"index", (PyCFunction)listindex, METH_VARARGS, index_doc},
1447-
{"count", (PyCFunction)listcount, METH_VARARGS, count_doc},
1510+
{"append", (PyCFunction)listappend, METH_VARARGS, append_doc},
1511+
{"insert", (PyCFunction)listinsert, METH_VARARGS, insert_doc},
1512+
{"extend", (PyCFunction)listextend, METH_VARARGS, extend_doc},
1513+
{"pop", (PyCFunction)listpop, METH_VARARGS, pop_doc},
1514+
{"remove", (PyCFunction)listremove, METH_VARARGS, remove_doc},
1515+
{"index", (PyCFunction)listindex, METH_VARARGS, index_doc},
1516+
{"count", (PyCFunction)listcount, METH_VARARGS, count_doc},
14481517
{"reverse", (PyCFunction)listreverse, METH_VARARGS, reverse_doc},
1449-
{"sort", (PyCFunction)listsort, METH_VARARGS, sort_doc},
1518+
{"sort", (PyCFunction)listsort, METH_VARARGS, sort_doc},
14501519
{NULL, NULL} /* sentinel */
14511520
};
14521521

@@ -1457,16 +1526,16 @@ list_getattr(PyListObject *f, char *name)
14571526
}
14581527

14591528
static PySequenceMethods list_as_sequence = {
1460-
(inquiry)list_length, /*sq_length*/
1461-
(binaryfunc)list_concat, /*sq_concat*/
1462-
(intargfunc)list_repeat, /*sq_repeat*/
1463-
(intargfunc)list_item, /*sq_item*/
1464-
(intintargfunc)list_slice, /*sq_slice*/
1465-
(intobjargproc)list_ass_item, /*sq_ass_item*/
1466-
(intintobjargproc)list_ass_slice, /*sq_ass_slice*/
1467-
(objobjproc)list_contains, /*sq_contains*/
1468-
(binaryfunc)list_inplace_concat, /*sq_inplace_concat*/
1469-
(intargfunc)list_inplace_repeat, /*sq_inplace_repeat*/
1529+
(inquiry)list_length, /* sq_length */
1530+
(binaryfunc)list_concat, /* sq_concat */
1531+
(intargfunc)list_repeat, /* sq_repeat */
1532+
(intargfunc)list_item, /* sq_item */
1533+
(intintargfunc)list_slice, /* sq_slice */
1534+
(intobjargproc)list_ass_item, /* sq_ass_item */
1535+
(intintobjargproc)list_ass_slice, /* sq_ass_slice */
1536+
(objobjproc)list_contains, /* sq_contains */
1537+
(binaryfunc)list_inplace_concat, /* sq_inplace_concat */
1538+
(intargfunc)list_inplace_repeat, /* sq_inplace_repeat */
14701539
};
14711540

14721541
PyTypeObject PyList_Type = {
@@ -1475,25 +1544,26 @@ PyTypeObject PyList_Type = {
14751544
"list",
14761545
sizeof(PyListObject) + PyGC_HEAD_SIZE,
14771546
0,
1478-
(destructor)list_dealloc, /*tp_dealloc*/
1479-
(printfunc)list_print, /*tp_print*/
1480-
(getattrfunc)list_getattr, /*tp_getattr*/
1481-
0, /*tp_setattr*/
1482-
(cmpfunc)list_compare, /*tp_compare*/
1483-
(reprfunc)list_repr, /*tp_repr*/
1484-
0, /*tp_as_number*/
1485-
&list_as_sequence, /*tp_as_sequence*/
1486-
0, /*tp_as_mapping*/
1487-
0, /*tp_hash*/
1488-
0, /*tp_call*/
1489-
0, /*tp_str*/
1490-
0, /*tp_getattro*/
1491-
0, /*tp_setattro*/
1492-
0, /*tp_as_buffer*/
1493-
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_GC, /*tp_flags*/
1494-
0, /* tp_doc */
1495-
(traverseproc)list_traverse, /* tp_traverse */
1496-
(inquiry)list_clear, /* tp_clear */
1547+
(destructor)list_dealloc, /* tp_dealloc */
1548+
(printfunc)list_print, /* tp_print */
1549+
(getattrfunc)list_getattr, /* tp_getattr */
1550+
0, /* tp_setattr */
1551+
0, /* tp_compare */
1552+
(reprfunc)list_repr, /* tp_repr */
1553+
0, /* tp_as_number */
1554+
&list_as_sequence, /* tp_as_sequence */
1555+
0, /* tp_as_mapping */
1556+
0, /* tp_hash */
1557+
0, /* tp_call */
1558+
0, /* tp_str */
1559+
0, /* tp_getattro */
1560+
0, /* tp_setattro */
1561+
0, /* tp_as_buffer */
1562+
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_GC, /* tp_flags */
1563+
0, /* tp_doc */
1564+
(traverseproc)list_traverse, /* tp_traverse */
1565+
(inquiry)list_clear, /* tp_clear */
1566+
list_richcompare, /* tp_richcompare */
14971567
};
14981568

14991569

@@ -1536,14 +1606,14 @@ immutable_list_ass(void)
15361606
}
15371607

15381608
static PySequenceMethods immutable_list_as_sequence = {
1539-
(inquiry)list_length, /*sq_length*/
1540-
(binaryfunc)list_concat, /*sq_concat*/
1541-
(intargfunc)list_repeat, /*sq_repeat*/
1542-
(intargfunc)list_item, /*sq_item*/
1543-
(intintargfunc)list_slice, /*sq_slice*/
1544-
(intobjargproc)immutable_list_ass, /*sq_ass_item*/
1545-
(intintobjargproc)immutable_list_ass, /*sq_ass_slice*/
1546-
(objobjproc)list_contains, /*sq_contains*/
1609+
(inquiry)list_length, /* sq_length */
1610+
(binaryfunc)list_concat, /* sq_concat */
1611+
(intargfunc)list_repeat, /* sq_repeat */
1612+
(intargfunc)list_item, /* sq_item */
1613+
(intintargfunc)list_slice, /* sq_slice */
1614+
(intobjargproc)immutable_list_ass, /* sq_ass_item */
1615+
(intintobjargproc)immutable_list_ass, /* sq_ass_slice */
1616+
(objobjproc)list_contains, /* sq_contains */
15471617
};
15481618

15491619
static PyTypeObject immutable_list_type = {
@@ -1552,22 +1622,25 @@ static PyTypeObject immutable_list_type = {
15521622
"list (immutable, during sort)",
15531623
sizeof(PyListObject) + PyGC_HEAD_SIZE,
15541624
0,
1555-
0, /*tp_dealloc*/ /* Cannot happen */
1556-
(printfunc)list_print, /*tp_print*/
1557-
(getattrfunc)immutable_list_getattr, /*tp_getattr*/
1558-
0, /*tp_setattr*/
1559-
0, /*tp_compare*/ /* Won't be called */
1560-
(reprfunc)list_repr, /*tp_repr*/
1561-
0, /*tp_as_number*/
1562-
&immutable_list_as_sequence, /*tp_as_sequence*/
1563-
0, /*tp_as_mapping*/
1564-
0, /*tp_hash*/
1565-
0, /*tp_call*/
1566-
0, /*tp_str*/
1567-
0, /*tp_getattro*/
1568-
0, /*tp_setattro*/
1569-
0, /*tp_as_buffer*/
1570-
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_GC, /*tp_flags*/
1571-
0, /* tp_doc */
1572-
(traverseproc)list_traverse, /* tp_traverse */
1625+
0, /* Cannot happen */ /* tp_dealloc */
1626+
(printfunc)list_print, /* tp_print */
1627+
(getattrfunc)immutable_list_getattr, /* tp_getattr */
1628+
0, /* tp_setattr */
1629+
0, /* Won't be called */ /* tp_compare */
1630+
(reprfunc)list_repr, /* tp_repr */
1631+
0, /* tp_as_number */
1632+
&immutable_list_as_sequence, /* tp_as_sequence */
1633+
0, /* tp_as_mapping */
1634+
0, /* tp_hash */
1635+
0, /* tp_call */
1636+
0, /* tp_str */
1637+
0, /* tp_getattro */
1638+
0, /* tp_setattro */
1639+
0, /* tp_as_buffer */
1640+
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_GC, /* tp_flags */
1641+
0, /* tp_doc */
1642+
(traverseproc)list_traverse, /* tp_traverse */
1643+
0, /* tp_clear */
1644+
list_richcompare, /* tp_richcompare */
1645+
/* NOTE: This is *not* the standard list_type struct! */
15731646
};

0 commit comments

Comments
 (0)