From 556c016d34dc96ded9920c7e33a61903aa824ad3 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 24 Feb 2024 15:32:03 +0900 Subject: [PATCH 1/9] gh-112087: Use QSBR technique for list_new/clear for free-thread --- Lib/test/test_list.py | 3 +- Objects/listobject.c | 171 ++++++++++++++++++++++++++++-------------- 2 files changed, 116 insertions(+), 58 deletions(-) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index 2969c6e2f98a23..22b09b36d78eee 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -1,6 +1,6 @@ import sys from test import list_tests -from test.support import cpython_only +from test.support import cpython_only, Py_GIL_DISABLED import pickle import unittest @@ -230,6 +230,7 @@ def __eq__(self, other): self.assertFalse(list3 == list4) @cpython_only + @unittest.skipIf(Py_GIL_DISABLED, 'Only for the default build') def test_preallocation(self): iterable = [0] * 10 iter_size = sys.getsizeof(iterable) diff --git a/Objects/listobject.c b/Objects/listobject.c index 2bb7d4ec342451..169003833b2f61 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -31,6 +31,91 @@ get_list_freelist(void) } #endif +#ifdef Py_GIL_DISABLED +static size_t +list_good_size(Py_ssize_t size) +{ + // 4, 8, 16, 24, 32, 40, 48, 64, 80, ... + // NOTE: we add one here so that the rounding accounts for the "allocated" + size_t reqsize = (size_t)size + 1; + if (reqsize <= 4) { + reqsize = 4; + } + else if (reqsize <= 48) { + reqsize = (reqsize + 7) & ~7; + } + else { + reqsize = (reqsize + 15) & ~15; + if (reqsize <= MI_MEDIUM_OBJ_WSIZE_MAX) { + reqsize = mi_good_size(reqsize * sizeof(PyObject *))/sizeof(PyObject*); + } + else { + // ensure geometric spacing for large arrays + size_t shift = mi_bsr(reqsize) - 2; + reqsize = ((reqsize >> shift) + 1) << shift; + } + } + return reqsize - 1; +} + +static PyObject** +list_allocate_items(size_t capacity) +{ + if (capacity > PY_SSIZE_T_MAX / sizeof(PyObject *) - 1) { + return NULL; + } + PyObject **items = PyMem_Malloc(capacity * sizeof(PyObject *)); + return items; +} +#endif + +static PyListObject * +list_new(Py_ssize_t size) +{ + PyListObject *op; + assert(size >= 0); +#ifdef WITH_FREELISTS + struct _Py_list_freelist *list_freelist = get_list_freelist(); + if (PyList_MAXFREELIST && list_freelist->numfree > 0) { + list_freelist->numfree--; + op = list_freelist->items[list_freelist->numfree]; + OBJECT_STAT_INC(from_freelist); + _Py_NewReference((PyObject *)op); + } + else +#endif + { + op = PyObject_GC_New(PyListObject, &PyList_Type); + if (op == NULL) { + return NULL; + } + } + if (size <= 0) { + op->ob_item = NULL; + op->allocated = 0; + } + else { +#ifdef Py_GIL_DISABLED + size_t capacity = list_good_size(size); + PyObject **items = list_allocate_items(capacity); +#else + size_t capacity = size; + PyObject **items = (PyObject **) PyMem_Calloc(size, sizeof(PyObject *)); +#endif + if (items == NULL) { + op->ob_item = NULL; + Py_DECREF(op); + PyErr_NoMemory(); + return NULL; + } + op->ob_item = items; + op->allocated = capacity; + } + Py_SET_SIZE(op, size); + _PyObject_GC_TRACK(op); + return op; +} + /* Ensure ob_item has room for at least newsize elements, and set * ob_size to newsize. If newsize > ob_size on entry, the content * of the new slots at exit is undefined heap trash; it's the caller's @@ -151,61 +236,18 @@ _PyList_DebugMallocStats(FILE *out) PyObject * PyList_New(Py_ssize_t size) { - PyListObject *op; - if (size < 0) { PyErr_BadInternalCall(); return NULL; } - -#ifdef WITH_FREELISTS - struct _Py_list_freelist *list_freelist = get_list_freelist(); - if (PyList_MAXFREELIST && list_freelist->numfree > 0) { - list_freelist->numfree--; - op = list_freelist->items[list_freelist->numfree]; - OBJECT_STAT_INC(from_freelist); - _Py_NewReference((PyObject *)op); - } - else -#endif - { - op = PyObject_GC_New(PyListObject, &PyList_Type); - if (op == NULL) { - return NULL; - } - } - if (size <= 0) { - op->ob_item = NULL; - } - else { - op->ob_item = (PyObject **) PyMem_Calloc(size, sizeof(PyObject *)); - if (op->ob_item == NULL) { - Py_DECREF(op); - return PyErr_NoMemory(); + PyListObject *op = list_new(size); + if (op && op->ob_item) { + PyObject **items = op->ob_item; + for (Py_ssize_t i = 0, n = op->allocated; i < n; i++) { + FT_ATOMIC_STORE_PTR_RELEASE(items[i], NULL); } } - Py_SET_SIZE(op, size); - op->allocated = size; - _PyObject_GC_TRACK(op); - return (PyObject *) op; -} - -static PyObject * -list_new_prealloc(Py_ssize_t size) -{ - assert(size > 0); - PyListObject *op = (PyListObject *) PyList_New(0); - if (op == NULL) { - return NULL; - } - assert(op->ob_item == NULL); - op->ob_item = PyMem_New(PyObject *, size); - if (op->ob_item == NULL) { - Py_DECREF(op); - return PyErr_NoMemory(); - } - op->allocated = size; - return (PyObject *) op; + return (PyObject *)op; } Py_ssize_t @@ -515,7 +557,7 @@ list_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) if (len <= 0) { return PyList_New(0); } - np = (PyListObject *) list_new_prealloc(len); + np = (PyListObject *) list_new(len); if (np == NULL) return NULL; @@ -567,7 +609,7 @@ list_concat_lock_held(PyListObject *a, PyListObject *b) if (size == 0) { return PyList_New(0); } - np = (PyListObject *) list_new_prealloc(size); + np = (PyListObject *) list_new(size); if (np == NULL) { return NULL; } @@ -617,7 +659,7 @@ list_repeat_lock_held(PyListObject *a, Py_ssize_t n) return PyErr_NoMemory(); Py_ssize_t output_size = input_size * n; - PyListObject *np = (PyListObject *) list_new_prealloc(output_size); + PyListObject *np = (PyListObject *) list_new(output_size); if (np == NULL) return NULL; @@ -658,7 +700,7 @@ list_repeat(PyObject *aa, Py_ssize_t n) } static void -list_clear(PyListObject *a) +list_clear_impl(PyListObject *a, bool is_resize) { PyObject **items = a->ob_item; if (items == NULL) { @@ -675,16 +717,31 @@ list_clear(PyListObject *a) Py_XDECREF(items[i]); } // TODO: Use QSBR technique, if the list is shared between threads, - PyMem_Free(items); - +#ifdef Py_GIL_DISABLED + bool use_qsbr = is_resize && _PyObject_GC_IS_SHARED(a); +#else + bool use_qsbr = false; +#endif + if (use_qsbr) { + _PyMem_FreeDelayed(items); + } + else { + PyMem_Free(items); + } // Note that there is no guarantee that the list is actually empty // at this point, because XDECREF may have populated it indirectly again! } +static void +list_clear(PyListObject *a) +{ + list_clear_impl(a, true); +} + static int list_clear_slot(PyObject *self) { - list_clear((PyListObject *)self); + list_clear_impl((PyListObject *)self, false); return 0; } @@ -3095,7 +3152,7 @@ list_subscript(PyObject* _self, PyObject* item) return list_slice(self, start, stop); } else { - result = list_new_prealloc(slicelength); + result = (PyObject *)list_new(slicelength); if (!result) return NULL; src = self->ob_item; From 67a7f32f0d2606dac17d6da52bffa85e63eea2e2 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 24 Feb 2024 16:45:44 +0900 Subject: [PATCH 2/9] Replace list_preallocate_exact to list_ensure_capacity --- Lib/test/test_list.py | 1 - Objects/listobject.c | 65 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index 22b09b36d78eee..74a370e7ee3155 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -230,7 +230,6 @@ def __eq__(self, other): self.assertFalse(list3 == list4) @cpython_only - @unittest.skipIf(Py_GIL_DISABLED, 'Only for the default build') def test_preallocation(self): iterable = [0] * 10 iter_size = sys.getsizeof(iterable) diff --git a/Objects/listobject.c b/Objects/listobject.c index 169003833b2f61..7c3cbaf04879f5 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -67,6 +67,56 @@ list_allocate_items(size_t capacity) PyObject **items = PyMem_Malloc(capacity * sizeof(PyObject *)); return items; } + +/* Ensure ob_item has room for at least newsize elements, and set + * ob_size to newsize. If newsize > ob_size on entry, the content + * of the new slots at exit is undefined heap trash; it's the caller's + * responsibility to overwrite them with sane values. + * The number of allocated elements may grow, shrink, or stay the same. + * Note that self->ob_item may change, and even if newsize is less + * than ob_size on entry. + */ +static int +list_ensure_capacity_slow(PyListObject *self, Py_ssize_t base, Py_ssize_t extra) +{ + if (base > PY_SSIZE_T_MAX/(Py_ssize_t)sizeof(PyObject*) - extra) { + PyErr_NoMemory(); + return -1; + } + + Py_ssize_t reqsize = base + extra; + Py_ssize_t allocated = self->allocated; + if (allocated >= reqsize) { + assert(self->ob_item != NULL || reqsize == 0); + return 0; + } + + if (!_Py_IsOwnedByCurrentThread((PyObject *)self)) { + _PyObject_GC_SET_SHARED(self); + } + + size_t capacity = list_good_size(reqsize); + PyObject **items = list_allocate_items(capacity); + if (items == NULL) { + PyErr_NoMemory(); + return -1; + } + PyObject **old = self->ob_item; + if (self->ob_item) { + memcpy(items, self->ob_item, allocated * sizeof(PyObject*)); + } + _Py_atomic_store_ptr_release(&self->ob_item, items); + self->allocated = capacity; + if (old) { + if (_PyObject_GC_IS_SHARED(self)) { + _PyMem_FreeDelayed(old); + } + else { + PyMem_Free(old); + } + } + return 0; +} #endif static PyListObject * @@ -184,8 +234,14 @@ list_resize(PyListObject *self, Py_ssize_t newsize) } static int -list_preallocate_exact(PyListObject *self, Py_ssize_t size) +list_ensure_capacity(PyListObject *self, Py_ssize_t base, Py_ssize_t extra) { +#ifdef Py_GIL_DISABLED + if (base > self->allocated - extra) { + return list_ensure_capacity_slow(self, base, extra); + } +#else + Py_ssize_t size = extra; assert(self->ob_item == NULL); assert(size > 0); @@ -202,6 +258,7 @@ list_preallocate_exact(PyListObject *self, Py_ssize_t size) } self->ob_item = items; self->allocated = size; +#endif return 0; } @@ -389,7 +446,7 @@ PyList_Insert(PyObject *op, Py_ssize_t where, PyObject *newitem) int _PyList_AppendTakeRefListResize(PyListObject *self, PyObject *newitem) { - Py_ssize_t len = Py_SIZE(self); + Py_ssize_t len = PyList_GET_SIZE(self); assert(self->allocated == -1 || self->allocated == len); if (list_resize(self, len + 1) < 0) { Py_DECREF(newitem); @@ -1027,7 +1084,7 @@ list_extend_fast(PyListObject *self, PyObject *iterable) // an overflow on any relevant platform. assert(m < PY_SSIZE_T_MAX - n); if (self->ob_item == NULL) { - if (list_preallocate_exact(self, n) < 0) { + if (list_ensure_capacity(self, m, n) < 0) { return -1; } Py_SET_SIZE(self, n); @@ -1075,7 +1132,7 @@ list_extend_iter(PyListObject *self, PyObject *iterable) */ } else if (self->ob_item == NULL) { - if (n && list_preallocate_exact(self, n) < 0) + if (n && list_ensure_capacity(self, m, n) < 0) goto error; } else { From 6ad0ab4ef78b163c9994c09dfaa5b20c526f65ef Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 24 Feb 2024 16:50:55 +0900 Subject: [PATCH 3/9] fix --- Lib/test/test_list.py | 2 +- Lib/test/test_sys.py | 11 +++++++---- Objects/listobject.c | 1 - 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index 74a370e7ee3155..2969c6e2f98a23 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -1,6 +1,6 @@ import sys from test import list_tests -from test.support import cpython_only, Py_GIL_DISABLED +from test.support import cpython_only import pickle import unittest diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 71671a5a984256..04a7fd2cded9b6 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -12,7 +12,7 @@ import sysconfig import test.support from test import support -from test.support import os_helper +from test.support import os_helper, Py_GIL_DISABLED from test.support.script_helper import assert_python_ok, assert_python_failure from test.support import threading_helper from test.support import import_helper @@ -1575,9 +1575,12 @@ def get_gen(): yield 1 check(re.finditer('',''), size('2P')) # list check(list([]), vsize('Pn')) - check(list([1]), vsize('Pn') + 2*self.P) - check(list([1, 2]), vsize('Pn') + 2*self.P) - check(list([1, 2, 3]), vsize('Pn') + 4*self.P) + if Py_GIL_DISABLED: + pass + else: + check(list([1]), vsize('Pn') + 2*self.P) + check(list([1, 2]), vsize('Pn') + 2*self.P) + check(list([1, 2, 3]), vsize('Pn') + 4*self.P) # sortwrapper (list) # XXX # cmpwrapper (list) diff --git a/Objects/listobject.c b/Objects/listobject.c index 7c3cbaf04879f5..0a97d4c117a177 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -773,7 +773,6 @@ list_clear_impl(PyListObject *a, bool is_resize) while (--i >= 0) { Py_XDECREF(items[i]); } - // TODO: Use QSBR technique, if the list is shared between threads, #ifdef Py_GIL_DISABLED bool use_qsbr = is_resize && _PyObject_GC_IS_SHARED(a); #else From 626409423b82e7f0bbfd967d46edae974b8010b3 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 24 Feb 2024 16:58:17 +0900 Subject: [PATCH 4/9] Update test --- Lib/test/test_sys.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 04a7fd2cded9b6..c76abe501ab3c2 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1224,7 +1224,7 @@ def test_pystats(self): @test.support.cpython_only @unittest.skipUnless(hasattr(sys, 'abiflags'), 'need sys.abiflags') def test_disable_gil_abi(self): - self.assertEqual('t' in sys.abiflags, support.Py_GIL_DISABLED) + self.assertEqual('t' in sys.abiflags, Py_GIL_DISABLED) @test.support.cpython_only @@ -1576,11 +1576,21 @@ def get_gen(): yield 1 # list check(list([]), vsize('Pn')) if Py_GIL_DISABLED: - pass + check(list([1]), vsize('Pn') + 3*self.P) + check(list([1, 2]), vsize('Pn') + 3*self.P) + check(list([1, 2, 3]), vsize('Pn') + 3*self.P) + check(list([1, 2, 3, 4]), vsize('Pn') + 7*self.P) + check(list([1, 2, 3, 4, 5]), vsize('Pn') + 7*self.P) + check(list([1, 2, 3, 4, 5, 6]), vsize('Pn') + 7*self.P) + check(list([1, 2, 3, 4, 5, 6, 7]), vsize('Pn') + 7*self.P) else: check(list([1]), vsize('Pn') + 2*self.P) check(list([1, 2]), vsize('Pn') + 2*self.P) check(list([1, 2, 3]), vsize('Pn') + 4*self.P) + check(list([1, 2, 3, 4]), vsize('Pn') + 4*self.P) + check(list([1, 2, 3, 4, 5]), vsize('Pn') + 6*self.P) + check(list([1, 2, 3, 4, 5, 6]), vsize('Pn') + 6*self.P) + check(list([1, 2, 3, 4, 5, 6, 7]), vsize('Pn') + 8*self.P) # sortwrapper (list) # XXX # cmpwrapper (list) From e514c653ba9ee1b7901ada48f94ddf7455264e19 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 24 Feb 2024 17:58:59 +0900 Subject: [PATCH 5/9] Update list_item --- Objects/listobject.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 0a97d4c117a177..08e8ab32dad8e2 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -597,11 +597,21 @@ static PyObject * list_item(PyObject *aa, Py_ssize_t i) { PyListObject *a = (PyListObject *)aa; + PyObject *item = NULL; + Py_BEGIN_CRITICAL_SECTION(a); +#ifdef Py_GIL_DISABLED + if (!_PyObject_GC_IS_SHARED(a)) { + _PyObject_GC_SET_SHARED(a); + } +#endif if (!valid_index(i, PyList_GET_SIZE(a))) { PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); - return NULL; + goto exit; } - return Py_NewRef(a->ob_item[i]); + item = Py_NewRef(a->ob_item[i]); +exit: + Py_END_CRITICAL_SECTION(); + return item; } static PyObject * From df90d37cd25ae21cd414a5bc35249e7793fe5dc9 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Thu, 29 Feb 2024 07:03:04 +0900 Subject: [PATCH 6/9] Revert memory optimization changes --- Lib/test/test_sys.py | 23 ++------- Objects/listobject.c | 120 ++++--------------------------------------- 2 files changed, 15 insertions(+), 128 deletions(-) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index c76abe501ab3c2..71671a5a984256 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -12,7 +12,7 @@ import sysconfig import test.support from test import support -from test.support import os_helper, Py_GIL_DISABLED +from test.support import os_helper from test.support.script_helper import assert_python_ok, assert_python_failure from test.support import threading_helper from test.support import import_helper @@ -1224,7 +1224,7 @@ def test_pystats(self): @test.support.cpython_only @unittest.skipUnless(hasattr(sys, 'abiflags'), 'need sys.abiflags') def test_disable_gil_abi(self): - self.assertEqual('t' in sys.abiflags, Py_GIL_DISABLED) + self.assertEqual('t' in sys.abiflags, support.Py_GIL_DISABLED) @test.support.cpython_only @@ -1575,22 +1575,9 @@ def get_gen(): yield 1 check(re.finditer('',''), size('2P')) # list check(list([]), vsize('Pn')) - if Py_GIL_DISABLED: - check(list([1]), vsize('Pn') + 3*self.P) - check(list([1, 2]), vsize('Pn') + 3*self.P) - check(list([1, 2, 3]), vsize('Pn') + 3*self.P) - check(list([1, 2, 3, 4]), vsize('Pn') + 7*self.P) - check(list([1, 2, 3, 4, 5]), vsize('Pn') + 7*self.P) - check(list([1, 2, 3, 4, 5, 6]), vsize('Pn') + 7*self.P) - check(list([1, 2, 3, 4, 5, 6, 7]), vsize('Pn') + 7*self.P) - else: - check(list([1]), vsize('Pn') + 2*self.P) - check(list([1, 2]), vsize('Pn') + 2*self.P) - check(list([1, 2, 3]), vsize('Pn') + 4*self.P) - check(list([1, 2, 3, 4]), vsize('Pn') + 4*self.P) - check(list([1, 2, 3, 4, 5]), vsize('Pn') + 6*self.P) - check(list([1, 2, 3, 4, 5, 6]), vsize('Pn') + 6*self.P) - check(list([1, 2, 3, 4, 5, 6, 7]), vsize('Pn') + 8*self.P) + check(list([1]), vsize('Pn') + 2*self.P) + check(list([1, 2]), vsize('Pn') + 2*self.P) + check(list([1, 2, 3]), vsize('Pn') + 4*self.P) # sortwrapper (list) # XXX # cmpwrapper (list) diff --git a/Objects/listobject.c b/Objects/listobject.c index 08e8ab32dad8e2..25a0f3070db5bc 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -31,96 +31,8 @@ get_list_freelist(void) } #endif -#ifdef Py_GIL_DISABLED -static size_t -list_good_size(Py_ssize_t size) -{ - // 4, 8, 16, 24, 32, 40, 48, 64, 80, ... - // NOTE: we add one here so that the rounding accounts for the "allocated" - size_t reqsize = (size_t)size + 1; - if (reqsize <= 4) { - reqsize = 4; - } - else if (reqsize <= 48) { - reqsize = (reqsize + 7) & ~7; - } - else { - reqsize = (reqsize + 15) & ~15; - if (reqsize <= MI_MEDIUM_OBJ_WSIZE_MAX) { - reqsize = mi_good_size(reqsize * sizeof(PyObject *))/sizeof(PyObject*); - } - else { - // ensure geometric spacing for large arrays - size_t shift = mi_bsr(reqsize) - 2; - reqsize = ((reqsize >> shift) + 1) << shift; - } - } - return reqsize - 1; -} - -static PyObject** -list_allocate_items(size_t capacity) -{ - if (capacity > PY_SSIZE_T_MAX / sizeof(PyObject *) - 1) { - return NULL; - } - PyObject **items = PyMem_Malloc(capacity * sizeof(PyObject *)); - return items; -} - -/* Ensure ob_item has room for at least newsize elements, and set - * ob_size to newsize. If newsize > ob_size on entry, the content - * of the new slots at exit is undefined heap trash; it's the caller's - * responsibility to overwrite them with sane values. - * The number of allocated elements may grow, shrink, or stay the same. - * Note that self->ob_item may change, and even if newsize is less - * than ob_size on entry. - */ -static int -list_ensure_capacity_slow(PyListObject *self, Py_ssize_t base, Py_ssize_t extra) -{ - if (base > PY_SSIZE_T_MAX/(Py_ssize_t)sizeof(PyObject*) - extra) { - PyErr_NoMemory(); - return -1; - } - - Py_ssize_t reqsize = base + extra; - Py_ssize_t allocated = self->allocated; - if (allocated >= reqsize) { - assert(self->ob_item != NULL || reqsize == 0); - return 0; - } - - if (!_Py_IsOwnedByCurrentThread((PyObject *)self)) { - _PyObject_GC_SET_SHARED(self); - } - - size_t capacity = list_good_size(reqsize); - PyObject **items = list_allocate_items(capacity); - if (items == NULL) { - PyErr_NoMemory(); - return -1; - } - PyObject **old = self->ob_item; - if (self->ob_item) { - memcpy(items, self->ob_item, allocated * sizeof(PyObject*)); - } - _Py_atomic_store_ptr_release(&self->ob_item, items); - self->allocated = capacity; - if (old) { - if (_PyObject_GC_IS_SHARED(self)) { - _PyMem_FreeDelayed(old); - } - else { - PyMem_Free(old); - } - } - return 0; -} -#endif - static PyListObject * -list_new(Py_ssize_t size) +list_new_prealloc(Py_ssize_t size) { PyListObject *op; assert(size >= 0); @@ -145,13 +57,8 @@ list_new(Py_ssize_t size) op->allocated = 0; } else { -#ifdef Py_GIL_DISABLED - size_t capacity = list_good_size(size); - PyObject **items = list_allocate_items(capacity); -#else size_t capacity = size; PyObject **items = (PyObject **) PyMem_Calloc(size, sizeof(PyObject *)); -#endif if (items == NULL) { op->ob_item = NULL; Py_DECREF(op); @@ -234,14 +141,8 @@ list_resize(PyListObject *self, Py_ssize_t newsize) } static int -list_ensure_capacity(PyListObject *self, Py_ssize_t base, Py_ssize_t extra) +list_preallocate_exact(PyListObject *self, Py_ssize_t size) { -#ifdef Py_GIL_DISABLED - if (base > self->allocated - extra) { - return list_ensure_capacity_slow(self, base, extra); - } -#else - Py_ssize_t size = extra; assert(self->ob_item == NULL); assert(size > 0); @@ -258,7 +159,6 @@ list_ensure_capacity(PyListObject *self, Py_ssize_t base, Py_ssize_t extra) } self->ob_item = items; self->allocated = size; -#endif return 0; } @@ -297,7 +197,7 @@ PyList_New(Py_ssize_t size) PyErr_BadInternalCall(); return NULL; } - PyListObject *op = list_new(size); + PyListObject *op = list_new_prealloc(size); if (op && op->ob_item) { PyObject **items = op->ob_item; for (Py_ssize_t i = 0, n = op->allocated; i < n; i++) { @@ -446,7 +346,7 @@ PyList_Insert(PyObject *op, Py_ssize_t where, PyObject *newitem) int _PyList_AppendTakeRefListResize(PyListObject *self, PyObject *newitem) { - Py_ssize_t len = PyList_GET_SIZE(self); + Py_ssize_t len = Py_SIZE(self); assert(self->allocated == -1 || self->allocated == len); if (list_resize(self, len + 1) < 0) { Py_DECREF(newitem); @@ -624,7 +524,7 @@ list_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) if (len <= 0) { return PyList_New(0); } - np = (PyListObject *) list_new(len); + np = (PyListObject *) list_new_prealloc(len); if (np == NULL) return NULL; @@ -676,7 +576,7 @@ list_concat_lock_held(PyListObject *a, PyListObject *b) if (size == 0) { return PyList_New(0); } - np = (PyListObject *) list_new(size); + np = (PyListObject *) list_new_prealloc(size); if (np == NULL) { return NULL; } @@ -726,7 +626,7 @@ list_repeat_lock_held(PyListObject *a, Py_ssize_t n) return PyErr_NoMemory(); Py_ssize_t output_size = input_size * n; - PyListObject *np = (PyListObject *) list_new(output_size); + PyListObject *np = (PyListObject *) list_new_prealloc(output_size); if (np == NULL) return NULL; @@ -1093,7 +993,7 @@ list_extend_fast(PyListObject *self, PyObject *iterable) // an overflow on any relevant platform. assert(m < PY_SSIZE_T_MAX - n); if (self->ob_item == NULL) { - if (list_ensure_capacity(self, m, n) < 0) { + if (list_preallocate_exact(self, n) < 0) { return -1; } Py_SET_SIZE(self, n); @@ -1141,7 +1041,7 @@ list_extend_iter(PyListObject *self, PyObject *iterable) */ } else if (self->ob_item == NULL) { - if (n && list_ensure_capacity(self, m, n) < 0) + if (n && list_preallocate_exact(self, n) < 0) goto error; } else { @@ -3218,7 +3118,7 @@ list_subscript(PyObject* _self, PyObject* item) return list_slice(self, start, stop); } else { - result = (PyObject *)list_new(slicelength); + result = (PyObject *)list_new_prealloc(slicelength); if (!result) return NULL; src = self->ob_item; From e7f36834739163092fc46ca79fc2bc963b27d93e Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Thu, 29 Feb 2024 07:21:09 +0900 Subject: [PATCH 7/9] Address code review --- Objects/listobject.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 25a0f3070db5bc..1e339c238282d0 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -497,19 +497,18 @@ static PyObject * list_item(PyObject *aa, Py_ssize_t i) { PyListObject *a = (PyListObject *)aa; - PyObject *item = NULL; + if (!valid_index(i, PyList_GET_SIZE(a))) { + PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); + return NULL; + } + PyObject *item; Py_BEGIN_CRITICAL_SECTION(a); #ifdef Py_GIL_DISABLED - if (!_PyObject_GC_IS_SHARED(a)) { + if (!_Py_IsOwnedByCurrentThread((PyObject *)a) && !_PyObject_GC_IS_SHARED(a)) { _PyObject_GC_SET_SHARED(a); } #endif - if (!valid_index(i, PyList_GET_SIZE(a))) { - PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); - goto exit; - } item = Py_NewRef(a->ob_item[i]); -exit: Py_END_CRITICAL_SECTION(); return item; } From 41204cf8803a5712c2e512347b9b1f7b892672d7 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 1 Mar 2024 10:22:21 +0900 Subject: [PATCH 8/9] Revert list_prealloc --- Objects/listobject.c | 97 ++++++++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 48 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 1e339c238282d0..f9856d27b4f259 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -31,48 +31,6 @@ get_list_freelist(void) } #endif -static PyListObject * -list_new_prealloc(Py_ssize_t size) -{ - PyListObject *op; - assert(size >= 0); -#ifdef WITH_FREELISTS - struct _Py_list_freelist *list_freelist = get_list_freelist(); - if (PyList_MAXFREELIST && list_freelist->numfree > 0) { - list_freelist->numfree--; - op = list_freelist->items[list_freelist->numfree]; - OBJECT_STAT_INC(from_freelist); - _Py_NewReference((PyObject *)op); - } - else -#endif - { - op = PyObject_GC_New(PyListObject, &PyList_Type); - if (op == NULL) { - return NULL; - } - } - if (size <= 0) { - op->ob_item = NULL; - op->allocated = 0; - } - else { - size_t capacity = size; - PyObject **items = (PyObject **) PyMem_Calloc(size, sizeof(PyObject *)); - if (items == NULL) { - op->ob_item = NULL; - Py_DECREF(op); - PyErr_NoMemory(); - return NULL; - } - op->ob_item = items; - op->allocated = capacity; - } - Py_SET_SIZE(op, size); - _PyObject_GC_TRACK(op); - return op; -} - /* Ensure ob_item has room for at least newsize elements, and set * ob_size to newsize. If newsize > ob_size on entry, the content * of the new slots at exit is undefined heap trash; it's the caller's @@ -193,18 +151,61 @@ _PyList_DebugMallocStats(FILE *out) PyObject * PyList_New(Py_ssize_t size) { + PyListObject *op; + if (size < 0) { PyErr_BadInternalCall(); return NULL; } - PyListObject *op = list_new_prealloc(size); - if (op && op->ob_item) { - PyObject **items = op->ob_item; - for (Py_ssize_t i = 0, n = op->allocated; i < n; i++) { - FT_ATOMIC_STORE_PTR_RELEASE(items[i], NULL); + +#ifdef WITH_FREELISTS + struct _Py_list_freelist *list_freelist = get_list_freelist(); + if (PyList_MAXFREELIST && list_freelist->numfree > 0) { + list_freelist->numfree--; + op = list_freelist->items[list_freelist->numfree]; + OBJECT_STAT_INC(from_freelist); + _Py_NewReference((PyObject *)op); + } + else +#endif + { + op = PyObject_GC_New(PyListObject, &PyList_Type); + if (op == NULL) { + return NULL; } } - return (PyObject *)op; + if (size <= 0) { + op->ob_item = NULL; + } + else { + op->ob_item = (PyObject **) PyMem_Calloc(size, sizeof(PyObject *)); + if (op->ob_item == NULL) { + Py_DECREF(op); + return PyErr_NoMemory(); + } + } + Py_SET_SIZE(op, size); + op->allocated = size; + _PyObject_GC_TRACK(op); + return (PyObject *) op; +} + +static PyObject * +list_new_prealloc(Py_ssize_t size) +{ + assert(size > 0); + PyListObject *op = (PyListObject *) PyList_New(0); + if (op == NULL) { + return NULL; + } + assert(op->ob_item == NULL); + op->ob_item = PyMem_New(PyObject *, size); + if (op->ob_item == NULL) { + Py_DECREF(op); + return PyErr_NoMemory(); + } + op->allocated = size; + return (PyObject *) op; } Py_ssize_t From 9f5d04885b56f958d9973379e1ffd24f6d68267a Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 1 Mar 2024 10:24:47 +0900 Subject: [PATCH 9/9] nit --- Objects/listobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index f9856d27b4f259..87effb1b3a65fa 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3118,7 +3118,7 @@ list_subscript(PyObject* _self, PyObject* item) return list_slice(self, start, stop); } else { - result = (PyObject *)list_new_prealloc(slicelength); + result = list_new_prealloc(slicelength); if (!result) return NULL; src = self->ob_item;