Skip to content

Commit 7e12faa

Browse files
committed
gh-161226: Copy the tuple in _PyTuple_Resize() if the hash is cached
* Add tuple_resize_inplace() helper function. * Add compute_hash optional parameter to _testcapi._tuple_resize().
1 parent 3945eaf commit 7e12faa

5 files changed

Lines changed: 171 additions & 74 deletions

File tree

‎Doc/c-api/tuple.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,11 @@ Tuple Objects
144144
``*p`` is destroyed. On failure, returns ``-1`` and sets ``*p`` to ``NULL``, and
145145
raises :exc:`MemoryError` or :exc:`SystemError`.
146146
147+
While tuple objects are usually immutable in Python, this special C API
148+
allows mutating a tuple object in-place. The returned tuple object can still
149+
be mutated using :c:func:`PyTuple_SetItem`; except if *newsize* is zero in
150+
which case it returns the immutable empty tuple.
151+
147152
148153
.. _struct-sequence-objects:
149154

‎Lib/test/test_capi/test_tuple.py‎

Lines changed: 60 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import unittest
21
import gc
3-
from sys import getrefcount
2+
import sys
3+
import unittest
44
from test.support import import_helper
55

66
_testcapi = import_helper.import_module('_testcapi')
@@ -131,9 +131,9 @@ def check_tuple_from_pair(self, from_pair):
131131
class Temp:
132132
pass
133133
temp = Temp()
134-
temp_rc = getrefcount(temp)
134+
temp_rc = sys.getrefcount(temp)
135135
self.assertEqual(from_pair(temp, temp), (temp, temp))
136-
self.assertEqual(getrefcount(temp), temp_rc)
136+
self.assertEqual(sys.getrefcount(temp), temp_rc)
137137

138138
self._not_tracked(from_pair(1, 2))
139139
self._not_tracked(from_pair(None, None))
@@ -299,29 +299,64 @@ def test_tuple_set_item(self):
299299

300300
def test__tuple_resize(self):
301301
# Test _PyTuple_Resize()
302-
resize = _testcapi._tuple_resize
302+
_resize = _testcapi._tuple_resize
303303
checknull = _testcapi._check_tuple_item_is_NULL
304304

305-
a = ()
306-
b = resize(a, 0, False)
307-
self.assertEqual(len(a), 0)
308-
self.assertEqual(len(b), 0)
309-
b = resize(a, 2, False)
310-
self.assertEqual(len(a), 0)
311-
self.assertEqual(len(b), 2)
312-
self.assertTrue(checknull(b, 0))
313-
self.assertTrue(checknull(b, 1))
314-
315-
a = ([1], [2], [3])
316-
b = resize(a, 3)
317-
self.assertEqual(b, a)
318-
b = resize(a, 2)
319-
self.assertEqual(b, a[:2])
320-
b = resize(a, 5)
321-
self.assertEqual(len(b), 5)
322-
self.assertEqual(b[:3], a)
323-
self.assertTrue(checknull(b, 3))
324-
self.assertTrue(checknull(b, 4))
305+
def assert_is_fresh_copy(result, refcnt, is_new_obj):
306+
self.assertEqual(refcnt, 1)
307+
self.assertTrue(is_new_obj)
308+
self.assertFalse(sys._is_immortal(result))
309+
310+
def resize(tup, new_size, new=True, compute_hash=False):
311+
if isinstance(tup, tuple):
312+
old_size = len(tup)
313+
else:
314+
old_size = 0
315+
result, refcnt, is_new_obj = _resize(tup, new_size, new,
316+
compute_hash=compute_hash)
317+
if new_size == old_size:
318+
# Return the same object unchanged
319+
self.assertFalse(is_new_obj)
320+
elif old_size == 0:
321+
assert_is_fresh_copy(result, refcnt, is_new_obj)
322+
elif new_size == 0:
323+
# check that the result is the empty bytes string singleton
324+
self.assertEqual(result, ())
325+
self.assertTrue(sys._is_immortal(result))
326+
self.assertTrue(is_new_obj)
327+
elif (not new) or compute_hash:
328+
# Make sure that the result is a fresh object.
329+
assert_is_fresh_copy(result, refcnt, is_new_obj)
330+
else:
331+
# An in-place resize can return the same memory address, or
332+
# not. 'is_new_obj' cannot be tested.
333+
self.assertEqual(refcnt, 1)
334+
self.assertFalse(sys._is_immortal(result))
335+
336+
return result
337+
338+
for compute_hash in True, False:
339+
with self.subTest(compute_hash=compute_hash):
340+
a = ()
341+
b = resize(a, 0, False, compute_hash)
342+
self.assertEqual(len(a), 0)
343+
self.assertEqual(len(b), 0)
344+
b = resize(a, 2, False, compute_hash)
345+
self.assertEqual(len(a), 0)
346+
self.assertEqual(len(b), 2)
347+
self.assertTrue(checknull(b, 0))
348+
self.assertTrue(checknull(b, 1))
349+
350+
a = tuple(object() for _ in range(3))
351+
b = resize(a, 3, True, compute_hash)
352+
self.assertEqual(b, a)
353+
b = resize(a, 2, True, compute_hash)
354+
self.assertEqual(b, a[:2])
355+
b = resize(a, 5, True, compute_hash)
356+
self.assertEqual(len(b), 5)
357+
self.assertEqual(b[:3], a)
358+
self.assertTrue(checknull(b, 3))
359+
self.assertTrue(checknull(b, 4))
325360

326361
a = ()
327362
self.assertRaises(MemoryError, resize, a, PY_SSIZE_T_MAX)
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:c:func:`_PyTuple_Resize` now returns a copy if the hash value was already
2+
computed. Patch by Victor Stinner.

‎Modules/_testcapi/tuple.c‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,17 @@ tuple_set_item(PyObject *Py_UNUSED(module), PyObject *args)
6666
}
6767

6868
static PyObject *
69-
_tuple_resize(PyObject *Py_UNUSED(module), PyObject *args)
69+
_tuple_resize(PyObject *Py_UNUSED(module), PyObject *args, PyObject *kwargs)
7070
{
71+
static char *kwlist[] = {"obj", "newsize", "new", "compute_hash", NULL};
7172
PyObject *tup;
7273
Py_ssize_t newsize;
7374
int new = 1;
74-
if (!PyArg_ParseTuple(args, "On|p", &tup, &newsize, &new)) {
75+
int compute_hash = 0;
76+
77+
if (!PyArg_ParseTupleAndKeywords(args, kwargs,
78+
"On|pi", kwlist,
79+
&tup, &newsize, &new, &compute_hash)) {
7580
return NULL;
7681
}
7782
if (new) {
@@ -84,12 +89,24 @@ _tuple_resize(PyObject *Py_UNUSED(module), PyObject *args)
8489
NULLABLE(tup);
8590
Py_XINCREF(tup);
8691
}
92+
93+
if (compute_hash) {
94+
if (PyObject_Hash(tup) == -1) {
95+
Py_DECREF(tup);
96+
return NULL;
97+
}
98+
}
99+
100+
PyObject *old_tup = tup;
87101
int r = _PyTuple_Resize(&tup, newsize);
88102
if (r == -1) {
89103
assert(tup == NULL);
90104
return NULL;
91105
}
92-
return tup;
106+
assert(tup != NULL);
107+
108+
Py_ssize_t refcnt = Py_REFCNT(tup);
109+
return Py_BuildValue("Nnp", tup, refcnt, tup != old_tup);
93110
}
94111

95112
static PyObject *
@@ -135,7 +152,7 @@ static PyMethodDef test_methods[] = {
135152
{"tuple_get_size", tuple_get_size, METH_O},
136153
{"tuple_get_item", tuple_get_item, METH_VARARGS},
137154
{"tuple_set_item", tuple_set_item, METH_VARARGS},
138-
{"_tuple_resize", _tuple_resize, METH_VARARGS},
155+
{"_tuple_resize", _PyCFunction_CAST(_tuple_resize), METH_VARARGS | METH_KEYWORDS},
139156
{"_check_tuple_item_is_NULL", _check_tuple_item_is_NULL, METH_VARARGS},
140157
{"tuple_fromarray", tuple_fromarray, METH_VARARGS},
141158
{NULL},

‎Objects/tupleobject.c‎

Lines changed: 83 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,49 +1015,12 @@ PyTypeObject PyTuple_Type = {
10151015
._tp_iteritem = tuple_iteritem,
10161016
};
10171017

1018-
/* The following function breaks the notion that tuples are immutable:
1019-
it changes the size of a tuple. We get away with this only if there
1020-
is only one module referencing the object. You can also think of it
1021-
as creating a new tuple object and destroying the old one, only more
1022-
efficiently. In any case, don't use this if the tuple may already be
1023-
known to some other part of the code. */
10241018

1025-
int
1026-
_PyTuple_Resize(PyObject **pv, Py_ssize_t newsize)
1019+
static int
1020+
tuple_resize_inplace(PyObject **pv, Py_ssize_t newsize)
10271021
{
1028-
PyTupleObject *v;
1029-
PyTupleObject *sv;
1030-
Py_ssize_t i;
1031-
Py_ssize_t oldsize;
1022+
PyTupleObject *v = (PyTupleObject *) *pv;
10321023

1033-
v = (PyTupleObject *) *pv;
1034-
if (v == NULL || !Py_IS_TYPE(v, &PyTuple_Type) || newsize < 0 ||
1035-
(Py_SIZE(v) != 0 && !_PyObject_IsUniquelyReferenced(*pv))) {
1036-
*pv = 0;
1037-
Py_XDECREF(v);
1038-
PyErr_BadInternalCall();
1039-
return -1;
1040-
}
1041-
1042-
oldsize = Py_SIZE(v);
1043-
if (oldsize == newsize) {
1044-
return 0;
1045-
}
1046-
if (newsize == 0) {
1047-
Py_DECREF(v);
1048-
*pv = tuple_get_empty();
1049-
return 0;
1050-
}
1051-
if (oldsize == 0) {
1052-
#ifdef Py_DEBUG
1053-
assert(v == &_Py_SINGLETON(tuple_empty));
1054-
#endif
1055-
/* The empty tuple is statically allocated so we never
1056-
resize it in-place. */
1057-
Py_DECREF(v);
1058-
*pv = PyTuple_New(newsize);
1059-
return *pv == NULL ? -1 : 0;
1060-
}
10611024
/* Check for overflow */
10621025
if ((size_t)newsize > ((size_t)PY_SSIZE_T_MAX - (sizeof(PyTupleObject) -
10631026
sizeof(PyObject *))) / sizeof(PyObject *)) {
@@ -1074,34 +1037,109 @@ _PyTuple_Resize(PyObject **pv, Py_ssize_t newsize)
10741037
_Py_ForgetReference((PyObject *) v);
10751038
#endif
10761039
/* DECREF items deleted by shrinkage */
1077-
for (i = newsize; i < oldsize; i++) {
1040+
Py_ssize_t oldsize = Py_SIZE(v);
1041+
for (Py_ssize_t i = newsize; i < oldsize; i++) {
10781042
Py_CLEAR(v->ob_item[i]);
10791043
}
10801044
_PyReftracerTrack((PyObject *)v, PyRefTracer_DESTROY);
1081-
sv = PyObject_GC_Resize(PyTupleObject, v, newsize);
1045+
1046+
PyTupleObject *sv = PyObject_GC_Resize(PyTupleObject, v, newsize);
10821047
if (sv == NULL) {
10831048
*pv = NULL;
10841049
#ifdef Py_REF_DEBUG
10851050
_Py_DecRefTotal(_PyThreadState_GET());
10861051
#endif
10871052
/* The failed reallocation leaves the original items owned by v. */
1088-
i = oldsize;
1053+
Py_ssize_t i = oldsize;
10891054
while (--i >= 0) {
10901055
Py_XDECREF(v->ob_item[i]);
10911056
}
10921057
PyObject_GC_Del(v);
10931058
return -1;
10941059
}
1060+
10951061
_Py_NewReferenceNoTotal((PyObject *) sv);
10961062
/* Zero out items added by growing */
1097-
if (newsize > oldsize)
1063+
if (newsize > oldsize) {
10981064
memset(&sv->ob_item[oldsize], 0,
1099-
sizeof(*sv->ob_item) * (newsize - oldsize));
1065+
sizeof(sv->ob_item[0]) * (newsize - oldsize));
1066+
}
1067+
11001068
*pv = (PyObject *) sv;
11011069
_PyObject_GC_TRACK(sv);
11021070
return 0;
11031071
}
11041072

1073+
1074+
/* The following function breaks the notion that tuples are immutable:
1075+
it changes the size of a tuple. We get away with this only if there
1076+
is only one module referencing the object. You can also think of it
1077+
as creating a new tuple object and destroying the old one, only more
1078+
efficiently. In any case, don't use this if the tuple may already be
1079+
known to some other part of the code. */
1080+
1081+
int
1082+
_PyTuple_Resize(PyObject **pv, Py_ssize_t newsize)
1083+
{
1084+
PyTupleObject *v = (PyTupleObject *) *pv;
1085+
if (v == NULL || !Py_IS_TYPE(v, &PyTuple_Type) || newsize < 0 ||
1086+
(Py_SIZE(v) != 0 && !_PyObject_IsUniquelyReferenced(*pv))) {
1087+
*pv = 0;
1088+
Py_XDECREF(v);
1089+
PyErr_BadInternalCall();
1090+
return -1;
1091+
}
1092+
1093+
Py_ssize_t oldsize = Py_SIZE(v);
1094+
if (oldsize == newsize) {
1095+
return 0;
1096+
}
1097+
1098+
if (newsize == 0) {
1099+
*pv = tuple_get_empty();
1100+
Py_DECREF(v);
1101+
return 0;
1102+
}
1103+
1104+
if (oldsize == 0) {
1105+
#ifdef Py_DEBUG
1106+
assert(v == &_Py_SINGLETON(tuple_empty));
1107+
#endif
1108+
/* The empty tuple is statically allocated so we never
1109+
resize it in-place. */
1110+
*pv = PyTuple_New(newsize);
1111+
Py_DECREF(v);
1112+
return *pv == NULL ? -1 : 0;
1113+
}
1114+
1115+
// gh-161226: Return a copy if the hash value was already computed
1116+
if (v->ob_hash != -1) {
1117+
PyTupleObject *new_tuple = tuple_alloc(newsize);
1118+
if (new_tuple == NULL) {
1119+
*pv = NULL;
1120+
Py_DECREF(v);
1121+
return -1;
1122+
}
1123+
// Copy items
1124+
Py_ssize_t copy = Py_MIN(oldsize, newsize);
1125+
for (Py_ssize_t i = 0; i < copy; i++) {
1126+
new_tuple->ob_item[i] = Py_NewRef(v->ob_item[i]);
1127+
}
1128+
if (newsize > oldsize) {
1129+
// Set new items to NULL
1130+
memset(&new_tuple->ob_item[oldsize], 0,
1131+
(newsize - oldsize) * sizeof(new_tuple->ob_item[0]));
1132+
}
1133+
_PyObject_GC_TRACK(new_tuple);
1134+
1135+
*pv = _PyObject_CAST(new_tuple);
1136+
Py_DECREF(v);
1137+
return 0;
1138+
}
1139+
1140+
return tuple_resize_inplace(pv, newsize);
1141+
}
1142+
11051143
/*********************** Tuple Iterator **************************/
11061144

11071145
#define _PyTupleIterObject_CAST(op) ((_PyTupleIterObject *)(op))

0 commit comments

Comments
 (0)