bpo-36030: Add an internal _PyTuple_NewUnsafe() function#11927
bpo-36030: Add an internal _PyTuple_NewUnsafe() function#11927sir-sigurd wants to merge 1 commit into
Conversation
e38d1a7 to
80a6c0c
Compare
| } | ||
|
|
||
| PyObject * | ||
| _PyTuple_NewUnsafe(Py_ssize_t size) |
There was a problem hiding this comment.
I'm not sure if _PyTuple_NewNotInitialized is better.
There was a problem hiding this comment.
I suggest the name _PyTuple_NewUninitialized.
|
@sir-sigurd wrote similar change for list: PR #8332. |
| PyObject * | ||
| PyTuple_New(Py_ssize_t size) | ||
| static PyObject * | ||
| tuple_new_internal(Py_ssize_t size, int initialize) |
There was a problem hiding this comment.
Uninitialized tuples must be initialized with Py_SIZE(op) = 0 since they are tracked by the GC (_PyObject_GC_TRACK call below). A GC collection can be triggered while the tuple is filled, whereas tupletraverse() uses Py_SIZE() to iterate on items. If Py_SIZE() isn't 0, tupletraverse() will likely crash.
I suggest the function name "tuple_new_impl".
There was a problem hiding this comment.
Uninitialized tuples must be initialized with Py_SIZE(op) = 0 since they are tracked by the GC (_PyObject_GC_TRACK call below). A GC collection can be triggered while the tuple is filled, whereas tupletraverse() uses Py_SIZE() to iterate on items. If Py_SIZE() isn't 0, tupletraverse() will likely crash.
I don't like setting Py_SIZE(op) = 0 and I see these alternatives:
- if I understand correctly GC collection can be triggered only when some objects are Py_DECREFed, if that's true we can limit usage of proposed function only to the cases when it's quite obvious that there are no Py_DECREFs until tuple is filled (it's done this way in bpo-34151: Improve performance of some list operations #8332), in the current patch there are just 2 not so obvious cases when
PyDict_Next()is used - call
_PyObject_GC_TRACK()in caller when tuple is filled as you proposed in bpo-34151: Improve performance of some list operations #8332 (comment)
There was a problem hiding this comment.
I suggest the function name "tuple_new_impl".
There is already this one:
Lines 696 to 707 in 5105483
| } | ||
|
|
||
| PyObject * | ||
| _PyTuple_NewUnsafe(Py_ssize_t size) |
There was a problem hiding this comment.
I suggest the name _PyTuple_NewUninitialized.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
4 participants
https://bugs.python.org/issue36030
I'm going to update PR using new function in more cases when there is consensus about adding such function.