bpo-36551: Optimize list comprehensions with preallocate size and protect against overflow#12718
bpo-36551: Optimize list comprehensions with preallocate size and protect against overflow#12718tonybaloney wants to merge 12 commits intopython:masterfrom
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
tonybaloney
force-pushed
the
list_comp_opt
branch
from
bd5e8b6 to
22debe6
Compare
|
|
||
| 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).
| } | ||
|
|
||
| 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 |
5 participants
https://bugs.python.org/issue36551