From f895ca9b3a485ba9820105517a3c5db56e356243 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 17 May 2024 01:43:34 -0400 Subject: [PATCH 1/7] gh-119053: Implement the fast path for list.__getitem__ --- Objects/listobject.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Objects/listobject.c b/Objects/listobject.c index 7070165014f137..53517bfa7d10b4 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -656,6 +656,16 @@ list_item(PyObject *aa, Py_ssize_t i) return NULL; } PyObject *item; +#ifdef Py_GIL_DISABLED + PyObject **ob_item = _Py_atomic_load_ptr(&a->ob_item); + item = _Py_TryXGetRef(&ob_item[i]); + if (item) { + if (_Py_atomic_load_ptr(&ob_item[i]) == item) { + return item; + } + Py_DECREF(item); + } +#endif Py_BEGIN_CRITICAL_SECTION(a); #ifdef Py_GIL_DISABLED if (!_Py_IsOwnedByCurrentThread((PyObject *)a) && !_PyObject_GC_IS_SHARED(a)) { From 8b96ae23fe796a6c26cf25ad630c227352f2de11 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 17 May 2024 02:27:32 -0400 Subject: [PATCH 2/7] fix --- Objects/listobject.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 53517bfa7d10b4..9d7b8a2e83d8a3 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -658,12 +658,9 @@ list_item(PyObject *aa, Py_ssize_t i) PyObject *item; #ifdef Py_GIL_DISABLED PyObject **ob_item = _Py_atomic_load_ptr(&a->ob_item); - item = _Py_TryXGetRef(&ob_item[i]); - if (item) { - if (_Py_atomic_load_ptr(&ob_item[i]) == item) { - return item; - } - Py_DECREF(item); + item = _Py_atomic_load_ptr(&ob_item[i]); + if (item && _Py_TryIncrefCompare(&ob_item[i],item)) { + return item; } #endif Py_BEGIN_CRITICAL_SECTION(a); From 8e832e05885fa42ce7540a2a2c16b9170c2723d0 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 17 May 2024 02:28:04 -0400 Subject: [PATCH 3/7] nit --- Objects/listobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 9d7b8a2e83d8a3..3131c0dc914429 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -659,7 +659,7 @@ list_item(PyObject *aa, Py_ssize_t i) #ifdef Py_GIL_DISABLED PyObject **ob_item = _Py_atomic_load_ptr(&a->ob_item); item = _Py_atomic_load_ptr(&ob_item[i]); - if (item && _Py_TryIncrefCompare(&ob_item[i],item)) { + if (item && _Py_TryIncrefCompare(&ob_item[i], item)) { return item; } #endif From b6ac85fe35cd0be8baf066da4eb145d0998053ec Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 17 May 2024 02:28:30 -0400 Subject: [PATCH 4/7] nit --- Objects/listobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 3131c0dc914429..98089596efdc4b 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -658,7 +658,7 @@ list_item(PyObject *aa, Py_ssize_t i) PyObject *item; #ifdef Py_GIL_DISABLED PyObject **ob_item = _Py_atomic_load_ptr(&a->ob_item); - item = _Py_atomic_load_ptr(&ob_item[i]); + item = _Py_atomic_load_ptr(&ob_item[i]); if (item && _Py_TryIncrefCompare(&ob_item[i], item)) { return item; } From c451a5d4464c5aa83ad42b3b2628f5cb6a50ad6c Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 17 May 2024 02:34:11 -0400 Subject: [PATCH 5/7] fix --- Objects/listobject.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Objects/listobject.c b/Objects/listobject.c index 98089596efdc4b..ce77355ff5f2a6 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -660,8 +660,13 @@ list_item(PyObject *aa, Py_ssize_t i) PyObject **ob_item = _Py_atomic_load_ptr(&a->ob_item); item = _Py_atomic_load_ptr(&ob_item[i]); if (item && _Py_TryIncrefCompare(&ob_item[i], item)) { + if (ob_item != _Py_atomic_load_ptr(&a->ob_item)) { + Py_DECREF(item); + goto retry; + } return item; } +retry: #endif Py_BEGIN_CRITICAL_SECTION(a); #ifdef Py_GIL_DISABLED From 5ae49e3d75ed30218f4b7d034c3ac518f29fd84b Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 18 May 2024 06:21:32 -0400 Subject: [PATCH 6/7] Address code review --- Objects/listobject.c | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index ce77355ff5f2a6..be6a112bf5578a 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -351,7 +351,11 @@ list_item_impl(PyListObject *self, Py_ssize_t idx) if (!valid_index(idx, size)) { goto exit; } +#ifdef Py_GIL_DISABLED + item = _Py_NewRefWithLock(self->ob_item[idx]); +#else item = Py_NewRef(self->ob_item[idx]); +#endif exit: Py_END_CRITICAL_SECTION(); return item; @@ -655,28 +659,11 @@ list_item(PyObject *aa, Py_ssize_t i) PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); return NULL; } - PyObject *item; #ifdef Py_GIL_DISABLED - PyObject **ob_item = _Py_atomic_load_ptr(&a->ob_item); - item = _Py_atomic_load_ptr(&ob_item[i]); - if (item && _Py_TryIncrefCompare(&ob_item[i], item)) { - if (ob_item != _Py_atomic_load_ptr(&a->ob_item)) { - Py_DECREF(item); - goto retry; - } - return item; - } -retry: -#endif - Py_BEGIN_CRITICAL_SECTION(a); -#ifdef Py_GIL_DISABLED - if (!_Py_IsOwnedByCurrentThread((PyObject *)a) && !_PyObject_GC_IS_SHARED(a)) { - _PyObject_GC_SET_SHARED(a); - } + return list_get_item_ref(a, i); +#else + return Py_NewRef(a->ob_item[i]); #endif - item = Py_NewRef(a->ob_item[i]); - Py_END_CRITICAL_SECTION(); - return item; } static PyObject * From 7fd2126ea011799b64a481b039ca61c42a7b4a52 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 18 May 2024 18:10:10 -0400 Subject: [PATCH 7/7] Raise index error if the item is NULL; --- Objects/listobject.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index be6a112bf5578a..d09bb6391034d1 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -659,11 +659,17 @@ list_item(PyObject *aa, Py_ssize_t i) PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); return NULL; } + PyObject *item; #ifdef Py_GIL_DISABLED - return list_get_item_ref(a, i); + item = list_get_item_ref(a, i); + if (item == NULL) { + PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); + return NULL; + } #else - return Py_NewRef(a->ob_item[i]); + item = Py_NewRef(a->ob_item[i]); #endif + return item; } static PyObject *