Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-112087: Store memory allocation information into _PyListArray#116529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
9 commits Select commit Hold shift + click to select a range
b98f853 gh-112087: Store memory allocation information into _PyListArray
corona10 d1ac2d4 nit
corona10 3b7d24e Fix casting
corona10 460dbc4 nit
corona10 d429298 nit
corona10 eb277c4 fix
corona10 2252858 Address code review
corona10 2493543 Address code review
corona10 a60d9cb Address code review
corona10 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
1 change: 1 addition & 0 deletions 1 Misc/NEWS.d/next/Core and Builtins/2024-03-09-11-10-53.gh-issue-112087.nbI0Pw.rst
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| :class:`list` is now compatible with the implementation of :pep:`703`. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -31,6 +31,50 @@ get_list_freelist(void) | ||
| } | ||
| #endif | ||
| #ifdef Py_GIL_DISABLED | ||
| typedef struct{ | ||
| Py_ssize_t allocated; | ||
| PyObject *ob_item[]; | ||
| } _PyListArray; | ||
| static _PyListArray * | ||
| list_allocate_array(size_t capacity) | ||
| { | ||
| if (capacity > PY_SSIZE_T_MAX/sizeof(PyObject*) - 1){ | ||
| return NULL; | ||
| } | ||
| _PyListArray *array = PyMem_Malloc(sizeof(_PyListArray) + capacity * sizeof(PyObject *)); | ||
| if (array == NULL){ | ||
| return NULL; | ||
| } | ||
| array->allocated = capacity; | ||
| return array; | ||
| } | ||
| static Py_ssize_t | ||
| list_capacity(PyObject **items) | ||
| { | ||
| _PyListArray *array = _Py_CONTAINER_OF(items, _PyListArray, ob_item); | ||
| return array->allocated; | ||
| } | ||
| #endif | ||
| static void | ||
| free_list_items(PyObject** items, bool use_qsbr) | ||
| { | ||
| #ifdef Py_GIL_DISABLED | ||
| _PyListArray *array = _Py_CONTAINER_OF(items, _PyListArray, ob_item); | ||
| if (use_qsbr){ | ||
| _PyMem_FreeDelayed(array); | ||
| } | ||
| else{ | ||
| PyMem_Free(array); | ||
| } | ||
| #else | ||
| PyMem_Free(items); | ||
| #endif | ||
| } | ||
| /* 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 | ||
| @@ -47,8 +91,7 @@ get_list_freelist(void) | ||
| static int | ||
| list_resize(PyListObject *self, Py_ssize_t newsize) | ||
| { | ||
| PyObject **items; | ||
| size_t new_allocated, num_allocated_bytes; | ||
| size_t new_allocated, target_bytes; | ||
| Py_ssize_t allocated = self->allocated; | ||
| /* Bypass realloc() when a previous overallocation is large enough | ||
| @@ -80,9 +123,34 @@ list_resize(PyListObject *self, Py_ssize_t newsize) | ||
| if (newsize == 0) | ||
| new_allocated = 0; | ||
| #ifdef Py_GIL_DISABLED | ||
| _PyListArray *array = list_allocate_array(new_allocated); | ||
| if (array == NULL){ | ||
| PyErr_NoMemory(); | ||
| return -1; | ||
| } | ||
| PyObject **old_items = self->ob_item; | ||
| if (self->ob_item){ | ||
| if (new_allocated < (size_t)allocated){ | ||
| target_bytes = new_allocated * sizeof(PyObject*); | ||
| } | ||
| else{ | ||
| target_bytes = allocated * sizeof(PyObject*); | ||
| } | ||
| memcpy(array->ob_item, self->ob_item, target_bytes); | ||
| } | ||
| _Py_atomic_store_ptr_release(&self->ob_item, &array->ob_item); | ||
corona10 marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| self->allocated = new_allocated; | ||
| Py_SET_SIZE(self, newsize); | ||
| if (old_items != NULL){ | ||
| free_list_items(old_items, _PyObject_GC_IS_SHARED(self)); | ||
| } | ||
| #else | ||
| PyObject **items; | ||
| if (new_allocated <= (size_t)PY_SSIZE_T_MAX / sizeof(PyObject *)){ | ||
| num_allocated_bytes = new_allocated * sizeof(PyObject *); | ||
| items = (PyObject **)PyMem_Realloc(self->ob_item, num_allocated_bytes); | ||
| target_bytes = new_allocated * sizeof(PyObject *); | ||
| items = (PyObject **)PyMem_Realloc(self->ob_item, target_bytes); | ||
| } | ||
| else{ | ||
| // integer overflow | ||
| @@ -95,12 +163,14 @@ list_resize(PyListObject *self, Py_ssize_t newsize) | ||
| self->ob_item = items; | ||
| Py_SET_SIZE(self, newsize); | ||
| self->allocated = new_allocated; | ||
| #endif | ||
| return 0; | ||
| } | ||
| static int | ||
| list_preallocate_exact(PyListObject *self, Py_ssize_t size) | ||
| { | ||
| PyObject **items; | ||
| assert(self->ob_item == NULL); | ||
| assert(size > 0); | ||
| @@ -110,11 +180,20 @@ list_preallocate_exact(PyListObject *self, Py_ssize_t size) | ||
| * allocated size up to the nearest even number. | ||
| */ | ||
| size = (size + 1) & ~(size_t)1; | ||
| PyObject **items = PyMem_New(PyObject*, size); | ||
| #ifdef Py_GIL_DISABLED | ||
| _PyListArray *array = list_allocate_array(size); | ||
| if (array == NULL){ | ||
| PyErr_NoMemory(); | ||
| return -1; | ||
| } | ||
| items = array->ob_item; | ||
| #else | ||
| items = PyMem_New(PyObject*, size); | ||
| if (items == NULL){ | ||
| PyErr_NoMemory(); | ||
| return -1; | ||
| } | ||
| #endif | ||
| self->ob_item = items; | ||
| self->allocated = size; | ||
| return 0; | ||
| @@ -178,7 +257,17 @@ PyList_New(Py_ssize_t size) | ||
| op->ob_item = NULL; | ||
| } | ||
| else{ | ||
| #ifdef Py_GIL_DISABLED | ||
| _PyListArray *array = list_allocate_array(size); | ||
| if (array == NULL){ | ||
| Py_DECREF(op); | ||
| return PyErr_NoMemory(); | ||
| } | ||
| memset(&array->ob_item, 0, size * sizeof(PyObject *)); | ||
| op->ob_item = array->ob_item; | ||
| #else | ||
| op->ob_item = (PyObject **) PyMem_Calloc(size, sizeof(PyObject *)); | ||
| #endif | ||
| if (op->ob_item == NULL){ | ||
| Py_DECREF(op); | ||
| return PyErr_NoMemory(); | ||
| @@ -199,11 +288,20 @@ list_new_prealloc(Py_ssize_t size) | ||
| return NULL; | ||
| } | ||
| assert(op->ob_item == NULL); | ||
| #ifdef Py_GIL_DISABLED | ||
| _PyListArray *array = list_allocate_array(size); | ||
| if (array == NULL){ | ||
| Py_DECREF(op); | ||
| return PyErr_NoMemory(); | ||
| } | ||
| op->ob_item = array->ob_item; | ||
| #else | ||
| op->ob_item = PyMem_New(PyObject *, size); | ||
| if (op->ob_item == NULL){ | ||
| Py_DECREF(op); | ||
| return PyErr_NoMemory(); | ||
| } | ||
| #endif | ||
| op->allocated = size; | ||
| return (PyObject *) op; | ||
| } | ||
| @@ -268,7 +366,7 @@ list_get_item_ref(PyListObject *op, Py_ssize_t i) | ||
| if (ob_item == NULL){ | ||
| return NULL; | ||
| } | ||
| Py_ssize_t cap = _Py_atomic_load_ssize_relaxed(&op->allocated); | ||
| Py_ssize_t cap = list_capacity(ob_item); | ||
| assert(cap != -1 && cap >= size); | ||
| if (!valid_index(i, cap)){ | ||
| return NULL; | ||
| @@ -438,7 +536,7 @@ list_dealloc(PyObject *self) | ||
| while (--i >= 0){ | ||
| Py_XDECREF(op->ob_item[i]); | ||
| } | ||
| PyMem_Free(op->ob_item); | ||
| free_list_items(op->ob_item, false); | ||
| } | ||
| #ifdef WITH_FREELISTS | ||
| struct _Py_list_freelist *list_freelist = get_list_freelist(); | ||
| @@ -737,12 +835,7 @@ list_clear_impl(PyListObject *a, bool is_resize) | ||
| #else | ||
| bool use_qsbr = false; | ||
| #endif | ||
| if (use_qsbr){ | ||
| _PyMem_FreeDelayed(items); | ||
| } | ||
| else{ | ||
| PyMem_Free(items); | ||
| } | ||
| free_list_items(items, use_qsbr); | ||
| // Note that there is no guarantee that the list is actually empty | ||
| // at this point, because XDECREF may have populated it indirectly again! | ||
| } | ||
| @@ -2758,7 +2851,12 @@ list_sort_impl(PyListObject *self, PyObject *keyfunc, int reverse) | ||
| while (--i >= 0){ | ||
| Py_XDECREF(final_ob_item[i]); | ||
| } | ||
| PyMem_Free(final_ob_item); | ||
| #ifdef Py_GIL_DISABLED | ||
| bool use_qsbr = _PyObject_GC_IS_SHARED(self); | ||
| #else | ||
| bool use_qsbr = false; | ||
| #endif | ||
| free_list_items(final_ob_item, use_qsbr); | ||
| } | ||
| return Py_XNewRef(result); | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR should be the final PR for #112087.
I think that we can close the issue, and after GIL is disabled, we can track relevant issues from a separate issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing I noticed here is that
list.sortshould have a critical section