From c908d1f87d287a4b3ec58c85b692a7eb617fa6ea Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 23 Jul 2024 12:24:24 -0400 Subject: [PATCH] gh-120974: Use common freelist code in asyncio (#122132) This refactors asyncio to use the common freelist helper functions and macros. As a side effect, the freelist for _asyncio.Future is now re-enabled in the free-threaded build. --- Include/internal/pycore_freelist_state.h | 2 + Modules/_asynciomodule.c | 73 ++---------------------- Objects/object.c | 5 +- 3 files changed, 10 insertions(+), 70 deletions(-) diff --git a/Include/internal/pycore_freelist_state.h b/Include/internal/pycore_freelist_state.h index d4c3d600b61..edf79dd7521 100644 --- a/Include/internal/pycore_freelist_state.h +++ b/Include/internal/pycore_freelist_state.h @@ -20,6 +20,7 @@ extern "C" { # define Py_contexts_MAXFREELIST 255 # define Py_async_gens_MAXFREELIST 80 # define Py_async_gen_asends_MAXFREELIST 80 +# define Py_futureiters_MAXFREELIST 255 # define Py_object_stack_chunks_MAXFREELIST 4 #else # define PyTuple_MAXSAVESIZE 0 @@ -47,6 +48,7 @@ struct _Py_freelists { struct _Py_freelist contexts; struct _Py_freelist async_gens; struct _Py_freelist async_gen_asends; + struct _Py_freelist futureiters; struct _Py_freelist object_stack_chunks; #else char _unused; // Empty structs are not allowed. diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index dd64b233e90..c44e89d9825 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -4,6 +4,7 @@ #include "Python.h" #include "pycore_dict.h" // _PyDict_GetItem_KnownHash() +#include "pycore_freelist.h" // _Py_FREELIST_POP() #include "pycore_modsupport.h" // _PyArg_CheckPositional() #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_object.h" // _Py_SetImmortalUntracked @@ -75,8 +76,6 @@ typedef struct { #define Future_Check(state, obj) PyObject_TypeCheck(obj, state->FutureType) #define Task_Check(state, obj) PyObject_TypeCheck(obj, state->TaskType) -#define FI_FREELIST_MAXLEN 255 - #ifdef Py_GIL_DISABLED # define ASYNCIO_STATE_LOCK(state) PyMutex_Lock(&state->mutex) # define ASYNCIO_STATE_UNLOCK(state) PyMutex_Unlock(&state->mutex) @@ -138,11 +137,6 @@ typedef struct { /* Counter for autogenerated Task names */ uint64_t task_name_counter; -#ifndef Py_GIL_DISABLED - futureiterobject *fi_freelist; - Py_ssize_t fi_freelist_len; -#endif - /* Linked-list of all tasks which are instances of asyncio.Task or subclasses of it. Third party tasks implementations which don't inherit from asyncio.Task are tracked separately using the 'non_asyncio_tasks' WeakSet. @@ -1584,24 +1578,7 @@ FutureIter_dealloc(futureiterobject *it) PyObject_GC_UnTrack(it); tp->tp_clear((PyObject *)it); -#ifndef Py_GIL_DISABLED - // GH-115874: We can't use PyType_GetModuleByDef here as the type might have - // already been cleared, which is also why we must check if ht_module != NULL. - PyObject *module = ((PyHeapTypeObject*)tp)->ht_module; - asyncio_state *state = NULL; - if (module && _PyModule_GetDef(module) == &_asynciomodule) { - state = get_asyncio_state(module); - } - - // TODO GH-121621: This should be moved to thread state as well. - if (state && state->fi_freelist_len < FI_FREELIST_MAXLEN) { - state->fi_freelist_len++; - it->future = (FutureObj*) state->fi_freelist; - state->fi_freelist = it; - } - else -#endif - { + if (!_Py_FREELIST_PUSH(futureiters, it, Py_futureiters_MAXFREELIST)) { PyObject_GC_Del(it); Py_DECREF(tp); } @@ -1805,17 +1782,8 @@ future_new_iter(PyObject *fut) asyncio_state *state = get_asyncio_state_by_def((PyObject *)fut); ENSURE_FUTURE_ALIVE(state, fut) -#ifndef Py_GIL_DISABLED - if (state->fi_freelist_len) { - state->fi_freelist_len--; - it = state->fi_freelist; - state->fi_freelist = (futureiterobject*) it->future; - it->future = NULL; - _Py_NewReference((PyObject*) it); - } - else -#endif - { + it = _Py_FREELIST_POP(futureiterobject, futureiters); + if (it == NULL) { it = PyObject_GC_New(futureiterobject, state->FutureIterType); if (it == NULL) { return NULL; @@ -3678,27 +3646,6 @@ _asyncio_all_tasks_impl(PyObject *module, PyObject *loop) return tasks; } -static void -module_free_freelists(asyncio_state *state) -{ -#ifndef Py_GIL_DISABLED - PyObject *next; - PyObject *current; - - next = (PyObject*) state->fi_freelist; - while (next != NULL) { - assert(state->fi_freelist_len > 0); - state->fi_freelist_len--; - - current = next; - next = (PyObject*) ((futureiterobject*) current)->future; - PyObject_GC_Del(current); - } - assert(state->fi_freelist_len == 0); - state->fi_freelist = NULL; -#endif -} - static int module_traverse(PyObject *mod, visitproc visit, void *arg) { @@ -3727,16 +3674,6 @@ module_traverse(PyObject *mod, visitproc visit, void *arg) Py_VISIT(state->context_kwname); -#ifndef Py_GIL_DISABLED - // Visit freelist. - PyObject *next = (PyObject*) state->fi_freelist; - while (next != NULL) { - PyObject *current = next; - Py_VISIT(current); - next = (PyObject*) ((futureiterobject*) current)->future; - } -#endif - return 0; } @@ -3768,8 +3705,6 @@ module_clear(PyObject *mod) Py_CLEAR(state->context_kwname); - module_free_freelists(state); - return 0; } diff --git a/Objects/object.c b/Objects/object.c index 6d6bb87433c..8a648a46487 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -829,7 +829,9 @@ static void free_object(void *obj) { PyObject *op = (PyObject *)obj; - Py_TYPE(op)->tp_free(op); + PyTypeObject *tp = Py_TYPE(op); + tp->tp_free(op); + Py_DECREF(tp); } #endif @@ -851,6 +853,7 @@ _PyObject_ClearFreeLists(struct _Py_freelists *freelists, int is_finalization) clear_freelist(&freelists->contexts, is_finalization, free_object); clear_freelist(&freelists->async_gens, is_finalization, free_object); clear_freelist(&freelists->async_gen_asends, is_finalization, free_object); + clear_freelist(&freelists->futureiters, is_finalization, free_object); if (is_finalization) { // Only clear object stack chunks during finalization. We use object // stacks during GC, so emptying the free-list is counterproductive.