From 2fa0b0d756ec0ad5bd66be48dcf6411e879ffb73 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 27 Aug 2021 06:03:46 +0300 Subject: [PATCH 1/6] bpo-45026: More compact range iterator --- Lib/test/test_range.py | 18 ++++++++++ Objects/rangeobject.c | 77 ++++++++++++++++++++---------------------- 2 files changed, 55 insertions(+), 40 deletions(-) diff --git a/Lib/test/test_range.py b/Lib/test/test_range.py index 107c0e2e11c7ce..9cc864af874633 100644 --- a/Lib/test/test_range.py +++ b/Lib/test/test_range.py @@ -421,6 +421,24 @@ def test_large_exhausted_iterator_pickling(self): self.assertEqual(list(i), []) self.assertEqual(list(i2), []) + def test_iterator_unpickle_compat(self): + testcases = [ + b'c__builtin__\niter\n(c__builtin__\nxrange\n(I10\nI20\nI2\ntRtRI2\nb.', + b'c__builtin__\niter\n(c__builtin__\nxrange\n(K\nK\x14K\x02tRtRK\x02b.', + b'\x80\x02c__builtin__\niter\nc__builtin__\nxrange\nK\nK\x14K\x02\x87R\x85RK\x02b.', + b'\x80\x03cbuiltins\niter\ncbuiltins\nrange\nK\nK\x14K\x02\x87R\x85RK\x02b.', + b'\x80\x04\x951\x00\x00\x00\x00\x00\x00\x00\x8c\x08builtins\x8c\x04iter\x93\x8c\x08builtins\x8c\x05range\x93K\nK\x14K\x02\x87R\x85RK\x02b.', + + b'c__builtin__\niter\n(c__builtin__\nxrange\n(L-36893488147419103232L\nI20\nI2\ntRtRL18446744073709551623L\nb.', + b'c__builtin__\niter\n(c__builtin__\nxrange\n(L-36893488147419103232L\nK\x14K\x02tRtRL18446744073709551623L\nb.', + b'\x80\x02c__builtin__\niter\nc__builtin__\nxrange\n\x8a\t\x00\x00\x00\x00\x00\x00\x00\x00\xfeK\x14K\x02\x87R\x85R\x8a\t\x07\x00\x00\x00\x00\x00\x00\x00\x01b.', + b'\x80\x03cbuiltins\niter\ncbuiltins\nrange\n\x8a\t\x00\x00\x00\x00\x00\x00\x00\x00\xfeK\x14K\x02\x87R\x85R\x8a\t\x07\x00\x00\x00\x00\x00\x00\x00\x01b.', + b'\x80\x04\x95C\x00\x00\x00\x00\x00\x00\x00\x8c\x08builtins\x8c\x04iter\x93\x8c\x08builtins\x8c\x05range\x93\x8a\t\x00\x00\x00\x00\x00\x00\x00\x00\xfeK\x14K\x02\x87R\x85R\x8a\t\x07\x00\x00\x00\x00\x00\x00\x00\x01b.', + ] + for t in testcases: + it = pickle.loads(t) + self.assertEqual(list(it), [14, 16, 18]) + def test_odd_bug(self): # This used to raise a "SystemError: NULL result without error" # because the range validation step was eating the exception diff --git a/Objects/rangeobject.c b/Objects/rangeobject.c index 3e05707b1cee69..f74c83522ec9de 100644 --- a/Objects/rangeobject.c +++ b/Objects/rangeobject.c @@ -766,7 +766,6 @@ PyTypeObject PyRange_Type = { typedef struct { PyObject_HEAD - long index; long start; long step; long len; @@ -775,18 +774,19 @@ typedef struct { static PyObject * rangeiter_next(rangeiterobject *r) { - if (r->index < r->len) - /* cast to unsigned to avoid possible signed overflow - in intermediate calculations. */ - return PyLong_FromLong((long)(r->start + - (unsigned long)(r->index++) * r->step)); + if (r->len > 0) { + long result = r->start; + r->start += r->step; + r->len--; + return PyLong_FromLong(result); + } return NULL; } static PyObject * rangeiter_len(rangeiterobject *r, PyObject *Py_UNUSED(ignored)) { - return PyLong_FromLong(r->len - r->index); + return PyLong_FromLong(r->len); } PyDoc_STRVAR(length_hint_doc, @@ -813,8 +813,8 @@ rangeiter_reduce(rangeiterobject *r, PyObject *Py_UNUSED(ignored)) if (range == NULL) goto err; /* return the result */ - return Py_BuildValue("N(N)i", _PyEval_GetBuiltinId(&PyId_iter), - range, r->index); + return Py_BuildValue("N(N)O", _PyEval_GetBuiltinId(&PyId_iter), + range, Py_None); err: Py_XDECREF(start); Py_XDECREF(stop); @@ -833,7 +833,8 @@ rangeiter_setstate(rangeiterobject *r, PyObject *state) index = 0; else if (index > r->len) index = r->len; /* exhausted iterator */ - r->index = index; + r->start += index * r->step; + r->len -= index; Py_RETURN_NONE; } @@ -931,13 +932,11 @@ fast_range_iter(long start, long stop, long step) return NULL; } it->len = (long)ulen; - it->index = 0; return (PyObject *)it; } typedef struct { PyObject_HEAD - PyObject *index; PyObject *start; PyObject *step; PyObject *len; @@ -946,7 +945,8 @@ typedef struct { static PyObject * longrangeiter_len(longrangeiterobject *r, PyObject *no_args) { - return PyNumber_Subtract(r->len, r->index); + Py_INCREF(r->len); + return r->len; } static PyObject * @@ -976,7 +976,7 @@ longrangeiter_reduce(longrangeiterobject *r, PyObject *Py_UNUSED(ignored)) /* return the result */ return Py_BuildValue("N(N)O", _PyEval_GetBuiltinId(&PyId_iter), - range, r->index); + range, Py_None); } static PyObject * @@ -999,8 +999,18 @@ longrangeiter_setstate(longrangeiterobject *r, PyObject *state) if (cmp > 0) state = r->len; } - Py_INCREF(state); - Py_XSETREF(r->index, state); + PyObject *new_len = PyNumber_Subtract(r->len, state); + if (new_len == NULL) + return NULL; + Py_SETREF(r->len, new_len); + PyObject *product = PyNumber_Multiply(state, r->step); + if (product == NULL) + return NULL; + PyObject *new_start = PyNumber_Add(r->start, product); + Py_DECREF(product); + if (new_start == NULL) + return NULL; + Py_SETREF(r->start, new_start); Py_RETURN_NONE; } @@ -1017,7 +1027,6 @@ static PyMethodDef longrangeiter_methods[] = { static void longrangeiter_dealloc(longrangeiterobject *r) { - Py_XDECREF(r->index); Py_XDECREF(r->start); Py_XDECREF(r->step); Py_XDECREF(r->len); @@ -1027,29 +1036,21 @@ longrangeiter_dealloc(longrangeiterobject *r) static PyObject * longrangeiter_next(longrangeiterobject *r) { - PyObject *product, *new_index, *result; - if (PyObject_RichCompareBool(r->index, r->len, Py_LT) != 1) + if (PyObject_RichCompareBool(r->len, _PyLong_GetZero(), Py_GT) != 1) return NULL; - new_index = PyNumber_Add(r->index, _PyLong_GetOne()); - if (!new_index) + PyObject *new_start = PyNumber_Add(r->start, r->step); + if (new_start == NULL) { return NULL; - - product = PyNumber_Multiply(r->index, r->step); - if (!product) { - Py_DECREF(new_index); - return NULL; - } - - result = PyNumber_Add(r->start, product); - Py_DECREF(product); - if (result) { - Py_SETREF(r->index, new_index); } - else { - Py_DECREF(new_index); + PyObject *new_len = PyNumber_Subtract(r->len, _PyLong_GetOne()); + if (new_len == NULL) { + Py_DECREF(new_start); + return NULL; } - + PyObject *result = r->start; + r->start = new_start; + Py_SETREF(r->len, new_len); return result; } @@ -1128,11 +1129,9 @@ range_iter(PyObject *seq) it->start = r->start; it->step = r->step; it->len = r->length; - it->index = _PyLong_GetZero(); Py_INCREF(it->start); Py_INCREF(it->step); Py_INCREF(it->len); - Py_INCREF(it->index); return (PyObject *)it; } @@ -1210,7 +1209,7 @@ range_reverse(PyObject *seq, PyObject *Py_UNUSED(ignored)) it = PyObject_New(longrangeiterobject, &PyLongRangeIter_Type); if (it == NULL) return NULL; - it->index = it->start = it->step = NULL; + it->start = it->step = NULL; /* start + (len - 1) * step */ it->len = range->length; @@ -1235,8 +1234,6 @@ range_reverse(PyObject *seq, PyObject *Py_UNUSED(ignored)) if (!it->step) goto create_failure; - it->index = _PyLong_GetZero(); - Py_INCREF(it->index); return (PyObject *)it; create_failure: From 1cd01387ed050ecd71fee1196bb66a93d796abdb Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 27 Aug 2021 06:35:33 +0300 Subject: [PATCH 2/6] Fix sizeof test. --- Lib/test/test_sys.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index dba4928ec261ac..d99190ca96b1c5 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1336,7 +1336,8 @@ def delx(self): del self.__x # PyCapsule # XXX # rangeiterator - check(iter(range(1)), size('4l')) + check(iter(range(1)), size('3l')) + check(iter(range(2**65)), size('3P')) # reverse check(reversed(''), size('nP')) # range From 3133f664f1cd7c04ae60c7ca51b6ea3224024ac9 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 29 Aug 2021 15:23:51 +0300 Subject: [PATCH 3/6] Add explicit tests for __setstate__. --- Lib/test/test_range.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Lib/test/test_range.py b/Lib/test/test_range.py index c94dba799adedc..177890b0f7e728 100644 --- a/Lib/test/test_range.py +++ b/Lib/test/test_range.py @@ -450,6 +450,20 @@ def test_iterator_unpickle_compat(self): it = pickle.loads(t) self.assertEqual(list(it), [14, 16, 18]) + def test_iterator_setstate(self): + it = iter(range(10, 20, 2)) + it.__setstate__(2) + self.assertEqual(list(it), [14, 16, 18]) + it = reversed(range(10, 20, 2)) + it.__setstate__(3) + self.assertEqual(list(it), [12, 10]) + it = iter(range(-2**65, 20, 2)) + it.__setstate__(2**64 + 7) + self.assertEqual(list(it), [14, 16, 18]) + it = reversed(range(10, 2**65, 2)) + it.__setstate__(2**64 - 7) + self.assertEqual(list(it), [12, 10]) + def test_odd_bug(self): # This used to raise a "SystemError: NULL result without error" # because the range validation step was eating the exception From 7a42474da799a5b75ac661247c817771c18ee5a3 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 29 Aug 2021 15:55:28 +0300 Subject: [PATCH 4/6] Add a NEWS entry. --- .../Core and Builtins/2021-08-29-15-55-19.bpo-45026.z7nTA3.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-08-29-15-55-19.bpo-45026.z7nTA3.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-08-29-15-55-19.bpo-45026.z7nTA3.rst b/Misc/NEWS.d/next/Core and Builtins/2021-08-29-15-55-19.bpo-45026.z7nTA3.rst new file mode 100644 index 00000000000000..481ab53e4f5197 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-08-29-15-55-19.bpo-45026.z7nTA3.rst @@ -0,0 +1,3 @@ +Optimize the :class:`range` object iterator. It is now smaller, faster +iteration of ranges containing large numbers. Smaller pickles, faster +unpickling. From 10822c03ddf124c7e99f820366531d7344713bcc Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 27 Nov 2022 17:29:53 +0200 Subject: [PATCH 5/6] Microoptimize small range iteration. --- Objects/rangeobject.c | 2 +- Python/bytecodes.c | 2 +- Python/generated_cases.c.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Objects/rangeobject.c b/Objects/rangeobject.c index 037e1abdc4888f..67a7201976f8f0 100644 --- a/Objects/rangeobject.c +++ b/Objects/rangeobject.c @@ -758,7 +758,7 @@ rangeiter_next(_PyRangeIterObject *r) { if (r->len > 0) { long result = r->start; - r->start += r->step; + r->start = result + r->step; r->len--; return PyLong_FromLong(result); } diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 5d956feb46bdff..41dd1acc937d71 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2627,7 +2627,7 @@ dummy_func( } else { long value = r->start; - r->start += r->step; + r->start = value + r->step; r->len--; if (_PyLong_AssignValue(&GETLOCAL(_Py_OPARG(next)), value) < 0) { goto error; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 108a7d2ab4b75e..3af60b83d84e70 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2645,7 +2645,7 @@ } else { long value = r->start; - r->start += r->step; + r->start = value + r->step; r->len--; if (_PyLong_AssignValue(&GETLOCAL(_Py_OPARG(next)), value) < 0) { goto error; From 050df748edb52cd95aab0dada8fe99d7a13055f0 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 30 Nov 2022 10:49:26 +0200 Subject: [PATCH 6/6] Set start and len simultaneously in longrangeiter_setstate. --- Objects/rangeobject.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Objects/rangeobject.c b/Objects/rangeobject.c index 67a7201976f8f0..992e7c079ded54 100644 --- a/Objects/rangeobject.c +++ b/Objects/rangeobject.c @@ -971,10 +971,6 @@ longrangeiter_setstate(longrangeiterobject *r, PyObject *state) if (cmp > 0) state = r->len; } - PyObject *new_len = PyNumber_Subtract(r->len, state); - if (new_len == NULL) - return NULL; - Py_SETREF(r->len, new_len); PyObject *product = PyNumber_Multiply(state, r->step); if (product == NULL) return NULL; @@ -982,7 +978,15 @@ longrangeiter_setstate(longrangeiterobject *r, PyObject *state) Py_DECREF(product); if (new_start == NULL) return NULL; - Py_SETREF(r->start, new_start); + PyObject *new_len = PyNumber_Subtract(r->len, state); + if (new_len == NULL) { + Py_DECREF(new_start); + return NULL; + } + PyObject *tmp = r->start; + r->start = new_start; + Py_SETREF(r->len, new_len); + Py_DECREF(tmp); Py_RETURN_NONE; }