From 9f1e266e906c5711c1c0c0a266ba5dcd8114ca98 Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Fri, 4 Jan 2019 13:44:39 -0700 Subject: [PATCH 1/6] Operator argument parsing for the normal case --- Modules/_operator.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Modules/_operator.c b/Modules/_operator.c index 3bf8c1276d7bed..ff446877e45d37 100644 --- a/Modules/_operator.c +++ b/Modules/_operator.c @@ -993,10 +993,16 @@ itemgetter_call(itemgetterobject *ig, PyObject *args, PyObject *kw) PyObject *obj, *result; Py_ssize_t i, nitems=ig->nitems; - if (!_PyArg_NoKeywords("itemgetter", kw)) - return NULL; - if (!PyArg_UnpackTuple(args, "itemgetter", 1, 1, &obj)) - return NULL; + assert(PyTuple_CheckExact(args)); + if (kw == NULL && PyTuple_GET_SIZE(args) == 1) { + obj = PyTuple_GET_ITEM(args, 0); + } + else { + if (!_PyArg_NoKeywords("itemgetter", kw)) + return NULL; + if (!PyArg_UnpackTuple(args, "itemgetter", 1, 1, &obj)) + return NULL; + } if (nitems == 1) return PyObject_GetItem(obj, ig->item); From c014a05c5259cc1deeace29648aef715969a2f8c Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Fri, 4 Jan 2019 14:39:16 -0700 Subject: [PATCH 2/6] Fast path for tuples with non-negative integer indicies --- Modules/_operator.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/Modules/_operator.c b/Modules/_operator.c index ff446877e45d37..7846fef4f6c6e4 100644 --- a/Modules/_operator.c +++ b/Modules/_operator.c @@ -937,6 +937,7 @@ typedef struct { PyObject_HEAD Py_ssize_t nitems; PyObject *item; + Py_ssize_t index; // -1 unless *item* is a single non-negative integer index } itemgetterobject; static PyTypeObject itemgetter_type; @@ -948,6 +949,7 @@ itemgetter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) itemgetterobject *ig; PyObject *item; Py_ssize_t nitems; + Py_ssize_t index; if (!_PyArg_NoKeywords("itemgetter", kwds)) return NULL; @@ -967,6 +969,16 @@ itemgetter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) Py_INCREF(item); ig->item = item; ig->nitems = nitems; + ig->index = -1; + if (PyLong_CheckExact(item)) { + index = PyLong_AsSsize_t(item); + if (index < 0) { + // Only optimize non-negative values that fit in a Py_ssize_t + PyErr_Clear(); + } else { + ig->index = index; + } + } PyObject_GC_Track(ig); return (PyObject *)ig; @@ -1003,8 +1015,16 @@ itemgetter_call(itemgetterobject *ig, PyObject *args, PyObject *kw) if (!PyArg_UnpackTuple(args, "itemgetter", 1, 1, &obj)) return NULL; } - if (nitems == 1) + if (nitems == 1) { + if (ig->index >= 0 + && PyTuple_CheckExact(obj) + && ig->index < PyTuple_GET_SIZE(obj)) { + result = PyTuple_GET_ITEM(obj, ig->index); + Py_INCREF(result); + return result; + } return PyObject_GetItem(obj, ig->item); + } assert(PyTuple_Check(ig->item)); assert(PyTuple_GET_SIZE(ig->item) == nitems); From d00fd55045269b444a7766f9323cd6124fd2cbe6 Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Fri, 4 Jan 2019 22:18:39 -0700 Subject: [PATCH 3/6] Add blurb --- .../next/Library/2019-01-04-22-18-25.bpo-35664.Z-Gyyj.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2019-01-04-22-18-25.bpo-35664.Z-Gyyj.rst diff --git a/Misc/NEWS.d/next/Library/2019-01-04-22-18-25.bpo-35664.Z-Gyyj.rst b/Misc/NEWS.d/next/Library/2019-01-04-22-18-25.bpo-35664.Z-Gyyj.rst new file mode 100644 index 00000000000000..f4acc5ae57e7e7 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-01-04-22-18-25.bpo-35664.Z-Gyyj.rst @@ -0,0 +1,4 @@ +Improve operator.itemgetter() performance by 33% with optimized argument +handling and with adding a fast path for the common case of a single +non-negative integer index into a tuple (which is the typical use case in +the standard library). From e9e2fa91aea04513b80c429e597934fea62a66ec Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Sat, 5 Jan 2019 23:55:38 -0700 Subject: [PATCH 4/6] PEP 7 nits --- Modules/_operator.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Modules/_operator.c b/Modules/_operator.c index 7846fef4f6c6e4..6e2919b44c58a7 100644 --- a/Modules/_operator.c +++ b/Modules/_operator.c @@ -975,7 +975,8 @@ itemgetter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (index < 0) { // Only optimize non-negative values that fit in a Py_ssize_t PyErr_Clear(); - } else { + } + else { ig->index = index; } } @@ -1018,7 +1019,8 @@ itemgetter_call(itemgetterobject *ig, PyObject *args, PyObject *kw) if (nitems == 1) { if (ig->index >= 0 && PyTuple_CheckExact(obj) - && ig->index < PyTuple_GET_SIZE(obj)) { + && ig->index < PyTuple_GET_SIZE(obj)) + { result = PyTuple_GET_ITEM(obj, ig->index); Py_INCREF(result); return result; From e7c33f1d10ca76b70ad6dd5aedf1471d4527e66e Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Sun, 6 Jan 2019 00:08:44 -0700 Subject: [PATCH 5/6] Add more tests --- Lib/test/test_operator.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Lib/test/test_operator.py b/Lib/test/test_operator.py index 6254091e78fec4..f46d94a226717a 100644 --- a/Lib/test/test_operator.py +++ b/Lib/test/test_operator.py @@ -401,6 +401,19 @@ def __getitem__(self, name): self.assertEqual(operator.itemgetter(2,10,5)(data), ('2', '10', '5')) self.assertRaises(TypeError, operator.itemgetter(2, 'x', 5), data) + # interesting indices + t = tuple('abcde') + self.assertEqual(operator.itemgetter(-1)(t), 'e') + self.assertEqual(operator.itemgetter(slice(2, 4))(t), ('c', 'd')) + + # interesting sequences + class T(tuple): + 'Tuple subclass' + pass + self.assertEqual(operator.itemgetter(0)(T('abc')), 'a') + self.assertEqual(operator.itemgetter(0)(['a', 'b', 'c']), 'a') + self.assertEqual(operator.itemgetter(0)(range(100, 200)), 100) + def test_methodcaller(self): operator = self.module self.assertRaises(TypeError, operator.methodcaller) From d70baef0cdaca013cb33e15c7c77807fabf4af0a Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Sun, 6 Jan 2019 00:31:29 -0700 Subject: [PATCH 6/6] Expand comment since the review didn't understand previous comment --- Modules/_operator.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Modules/_operator.c b/Modules/_operator.c index 6e2919b44c58a7..d6c6a18d81b449 100644 --- a/Modules/_operator.c +++ b/Modules/_operator.c @@ -973,7 +973,11 @@ itemgetter_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (PyLong_CheckExact(item)) { index = PyLong_AsSsize_t(item); if (index < 0) { - // Only optimize non-negative values that fit in a Py_ssize_t + /* If we get here, then either the index conversion failed + * due to being out of range, or the index was a negative + * integer. Either way, we clear any possible exception + * and fall back to the slow path, where ig->index is -1. + */ PyErr_Clear(); } else {