bpo-36551: Optimize list comprehensions with preallocate size and protect against overflow #12718
Conversation
…ith a preallocated memory size.
…o 'ifs' within the comprehension. Effectively preallocating the list to the length of the iterator
| Py_ssize_t size = PyObject_LengthHint(target, 2); | ||
| Py_DECREF(target); | ||
| if (size < 0) | ||
| goto error; |
There was a problem hiding this comment.
IMHO this should raise a ValueError or something other than OverflowError with an error message for an average Python user.
There was a problem hiding this comment.
Have raised that separately in #12720 as it's out-of-scope for this change and impact other things
|
If you are adding a new error condition, I think you should also try to add a test for it. Assuming the with self.assertRaises(OverflowError):
a = [x for x in range(2**256)]Ideally you'd come up with something a bit more clever since the failure mode of that one is apparently that the process consumes all available memory and then crashes, but I can't think of any at the moment. |
Replace LengthHint for HasLen and Object_Length
bd5e8b6 to
22debe6
Compare
| @@ -123,6 +123,13 @@ | |||
| >>> test_func() | |||
| [2, 2, 2, 2, 2] | |||
|
|
|||
| Verify that an overflow error is raised for listcomps with very-large iterators | |||
|
|
|||
| >>> [y for y in range(2**256)] # doctest: +IGNORE_EXCEPTION_DETAIL | |||
There was a problem hiding this comment.
It's really weird to me that this module is tested entirely with doctests (though I know that's not part of this PR).
| @@ -215,6 +215,12 @@ list_new_prealloc(Py_ssize_t size) | |||
| return (PyObject *) op; | |||
| } | |||
|
|
|||
| PyObject * | |||
| _PyList_NewPrealloc(Py_ssize_t size) | |||
There was a problem hiding this comment.
Instead of adding a new function _PyList_NewPrealloc with identical functionality as list_new_prealloc, why not just rename list_new_prealloc to _PyList_NewPrealloc?
|
What happens if the |
|
Issue was rejected, closing PR |
https://bugs.python.org/issue36551