-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-121621: Move asyncio running loop to thread state #121695
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
Fidget-Spinner
merged 21 commits into
python:main
from
Fidget-Spinner:asyncio_rest_threadsafe
Jul 16, 2024
Merged
Changes from 8 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
4822190
Use PyMutex for writes to asyncio state
Fidget-Spinner 143747b
Address review
Fidget-Spinner 580304c
fix default build
Fidget-Spinner 1faed12
Fix typo
Fidget-Spinner ba3d771
Address review
Fidget-Spinner ec2c175
fixup whitespace
Fidget-Spinner e1b730a
minor nit
Fidget-Spinner f9fb6e1
Move asyncio freelist to per-thread
Fidget-Spinner c9ac6bb
Move running loop to thread state
Fidget-Spinner b450b96
Merge remote-tracking branch 'upstream/main' into asyncio_rest_thread…
Fidget-Spinner 3d78e8a
remove freelist changes
Fidget-Spinner 4f03f71
Use just a single threadstate field
Fidget-Spinner a1ce760
fix whitespace
Fidget-Spinner f3f1c57
fix stray thing
Fidget-Spinner 4e3c64a
Update comment
Fidget-Spinner 19953a8
Clean up code even more
Fidget-Spinner ba039ee
make regen-all
Fidget-Spinner 3429231
Partially address review
Fidget-Spinner c6df89f
Address review
Fidget-Spinner 91438a5
Change Py_None to NULL
Fidget-Spinner 46432e8
add back missing comment
Fidget-Spinner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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 |
---|---|---|
|
@@ -77,10 +77,21 @@ typedef struct { | |
|
||
#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) | ||
#else | ||
# define ASYNCIO_STATE_LOCK(state) ((void)state) | ||
# define ASYNCIO_STATE_UNLOCK(state) ((void)state) | ||
#endif | ||
|
||
typedef struct futureiterobject futureiterobject; | ||
|
||
/* State of the _asyncio module */ | ||
typedef struct { | ||
#ifdef Py_GIL_DISABLED | ||
PyMutex mutex; | ||
#endif | ||
PyTypeObject *FutureIterType; | ||
PyTypeObject *TaskStepMethWrapper_Type; | ||
PyTypeObject *FutureType; | ||
|
@@ -130,9 +141,6 @@ typedef struct { | |
/* Counter for autogenerated Task names */ | ||
uint64_t task_name_counter; | ||
|
||
futureiterobject *fi_freelist; | ||
Py_ssize_t fi_freelist_len; | ||
|
||
/* 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. | ||
|
@@ -209,6 +217,16 @@ get_asyncio_state_by_def(PyObject *self) | |
#include "clinic/_asynciomodule.c.h" | ||
|
||
|
||
#ifdef WITH_FREELISTS | ||
static struct _Py_asyncmodule_futureiter_freelist * | ||
get_futureiter_freelist(void) | ||
{ | ||
struct _Py_object_freelists *freelists = _Py_object_freelists_GET(); | ||
assert(freelists != NULL); | ||
return &freelists->futureiters; | ||
} | ||
#endif | ||
|
||
/*[clinic input] | ||
class _asyncio.Future "FutureObj *" "&Future_Type" | ||
[clinic start generated code]*/ | ||
|
@@ -341,6 +359,8 @@ get_running_loop(asyncio_state *state, PyObject **loop) | |
} | ||
} | ||
|
||
// TODO GH-121621: The should be moved to PyThreadState | ||
// for easier and quicker access. | ||
state->cached_running_loop = rl; | ||
state->cached_running_loop_tsid = ts_id; | ||
} | ||
|
@@ -384,6 +404,9 @@ set_running_loop(asyncio_state *state, PyObject *loop) | |
return -1; | ||
} | ||
|
||
|
||
// TODO GH-121621: The should be moved to PyThreadState | ||
// for easier and quicker access. | ||
state->cached_running_loop = loop; // borrowed, kept alive by ts_dict | ||
state->cached_running_loop_tsid = PyThreadState_GetID(tstate); | ||
|
||
|
@@ -1653,26 +1676,19 @@ FutureIter_dealloc(futureiterobject *it) | |
// FutureIter is a heap type so any subclass must also be a heap type. | ||
assert(_PyType_HasFeature(tp, Py_TPFLAGS_HEAPTYPE)); | ||
|
||
PyObject *module = ((PyHeapTypeObject*)tp)->ht_module; | ||
asyncio_state *state = NULL; | ||
|
||
PyObject_GC_UnTrack(it); | ||
tp->tp_clear((PyObject *)it); | ||
|
||
// 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. | ||
// Due to this restriction, subclasses that belong to a different module | ||
// will not be able to use the free list. | ||
if (module && _PyModule_GetDef(module) == &_asynciomodule) { | ||
state = get_asyncio_state(module); | ||
} | ||
|
||
if (state && state->fi_freelist_len < FI_FREELIST_MAXLEN) { | ||
state->fi_freelist_len++; | ||
it->future = (FutureObj*) state->fi_freelist; | ||
state->fi_freelist = it; | ||
#ifdef WITH_FREELISTS | ||
struct _Py_asyncmodule_futureiter_freelist* freelist = get_futureiter_freelist(); | ||
if (freelist->fi_freelist_len < FI_FREELIST_MAXLEN) { | ||
freelist->fi_freelist_len++; | ||
it->future = (FutureObj*) freelist->fi_freelist; | ||
freelist->fi_freelist = it; | ||
} | ||
else { | ||
else | ||
#endif | ||
{ | ||
PyObject_GC_Del(it); | ||
Py_DECREF(tp); | ||
} | ||
|
@@ -1876,14 +1892,18 @@ future_new_iter(PyObject *fut) | |
asyncio_state *state = get_asyncio_state_by_def((PyObject *)fut); | ||
ENSURE_FUTURE_ALIVE(state, fut) | ||
|
||
if (state->fi_freelist_len) { | ||
state->fi_freelist_len--; | ||
it = state->fi_freelist; | ||
state->fi_freelist = (futureiterobject*) it->future; | ||
#ifdef WITH_FREELISTS | ||
struct _Py_asyncmodule_futureiter_freelist* freelist = get_futureiter_freelist(); | ||
if (freelist->fi_freelist_len) { | ||
freelist->fi_freelist_len--; | ||
it = freelist->fi_freelist; | ||
freelist->fi_freelist = (futureiterobject*) it->future; | ||
it->future = NULL; | ||
_Py_NewReference((PyObject*) it); | ||
} | ||
else { | ||
else | ||
#endif | ||
{ | ||
it = PyObject_GC_New(futureiterobject, state->FutureIterType); | ||
if (it == NULL) { | ||
return NULL; | ||
|
@@ -2018,10 +2038,12 @@ static PyMethodDef TaskWakeupDef = { | |
static void | ||
register_task(asyncio_state *state, TaskObj *task) | ||
{ | ||
ASYNCIO_STATE_LOCK(state); | ||
assert(Task_Check(state, task)); | ||
assert(task != &state->asyncio_tasks.tail); | ||
if (task->next != NULL) { | ||
// already registered | ||
ASYNCIO_STATE_UNLOCK(state); | ||
return; | ||
} | ||
assert(task->prev == NULL); | ||
|
@@ -2030,6 +2052,7 @@ register_task(asyncio_state *state, TaskObj *task) | |
task->next = state->asyncio_tasks.head; | ||
state->asyncio_tasks.head->prev = task; | ||
state->asyncio_tasks.head = task; | ||
ASYNCIO_STATE_UNLOCK(state); | ||
} | ||
|
||
static int | ||
|
@@ -2039,7 +2062,7 @@ register_eager_task(asyncio_state *state, PyObject *task) | |
} | ||
|
||
static void | ||
unregister_task(asyncio_state *state, TaskObj *task) | ||
unregister_task_lock_held(asyncio_state *state, TaskObj *task) | ||
{ | ||
assert(Task_Check(state, task)); | ||
assert(task != &state->asyncio_tasks.tail); | ||
|
@@ -2061,6 +2084,14 @@ unregister_task(asyncio_state *state, TaskObj *task) | |
assert(state->asyncio_tasks.head != task); | ||
} | ||
|
||
static void | ||
unregister_task(asyncio_state *state, TaskObj *task) | ||
{ | ||
ASYNCIO_STATE_LOCK(state); | ||
unregister_task_lock_held(state, task); | ||
ASYNCIO_STATE_UNLOCK(state); | ||
} | ||
|
||
static int | ||
unregister_eager_task(asyncio_state *state, PyObject *task) | ||
{ | ||
|
@@ -2213,7 +2244,12 @@ _asyncio_Task___init___impl(TaskObj *self, PyObject *coro, PyObject *loop, | |
// optimization: defer task name formatting | ||
// store the task counter as PyLong in the name | ||
// for deferred formatting in get_name | ||
name = PyLong_FromUnsignedLongLong(++state->task_name_counter); | ||
#ifdef Py_GIL_DISABLED | ||
unsigned long long counter = _Py_atomic_add_uint64(&state->task_name_counter, 1) + 1; | ||
#else | ||
unsigned long long counter = ++state->task_name_counter; | ||
#endif | ||
name = PyLong_FromUnsignedLongLong(counter); | ||
} else if (!PyUnicode_CheckExact(name)) { | ||
name = PyObject_Str(name); | ||
} else { | ||
|
@@ -3328,6 +3364,24 @@ task_wakeup(TaskObj *task, PyObject *o) | |
/*********************** Functions **************************/ | ||
|
||
|
||
/*[clinic input] | ||
_asyncio._clear_freelist | ||
|
||
Clears the asyncio freelist. | ||
|
||
Internal CPython implementation detail. Do not depend on this or use it! | ||
This function is thread-specific. | ||
|
||
[clinic start generated code]*/ | ||
|
||
static PyObject * | ||
_asyncio__clear_freelist_impl(PyObject *module) | ||
/*[clinic end generated code: output=8d0e295bbbe2f8b6 input=f3ef7630d66cf63a]*/ | ||
{ | ||
_PyAsyncModule_ClearFreeLists(_Py_object_freelists_GET(), 0); | ||
Py_RETURN_NONE; | ||
} | ||
|
||
/*[clinic input] | ||
_asyncio._get_running_loop | ||
|
||
|
@@ -3743,24 +3797,6 @@ _asyncio_all_tasks_impl(PyObject *module, PyObject *loop) | |
return tasks; | ||
} | ||
|
||
static void | ||
module_free_freelists(asyncio_state *state) | ||
{ | ||
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; | ||
} | ||
|
||
static int | ||
module_traverse(PyObject *mod, visitproc visit, void *arg) | ||
|
@@ -3791,12 +3827,15 @@ module_traverse(PyObject *mod, visitproc visit, void *arg) | |
Py_VISIT(state->context_kwname); | ||
|
||
// Visit freelist. | ||
PyObject *next = (PyObject*) state->fi_freelist; | ||
#ifdef WITH_FREELISTS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't seem right. We don't visit freelists anywhere else. The objects in the freelist should never be tracked by the GC. |
||
struct _Py_asyncmodule_futureiter_freelist* freelist = get_futureiter_freelist(); | ||
PyObject *next = (PyObject*) freelist->fi_freelist; | ||
while (next != NULL) { | ||
PyObject *current = next; | ||
Py_VISIT(current); | ||
next = (PyObject*) ((futureiterobject*) current)->future; | ||
} | ||
#endif | ||
return 0; | ||
} | ||
|
||
|
@@ -3828,7 +3867,8 @@ module_clear(PyObject *mod) | |
|
||
Py_CLEAR(state->context_kwname); | ||
|
||
module_free_freelists(state); | ||
_PyAsyncModule_ClearFreeLists(_Py_object_freelists_GET(), 0); | ||
|
||
|
||
return 0; | ||
} | ||
|
@@ -3940,6 +3980,7 @@ static PyMethodDef asyncio_methods[] = { | |
_ASYNCIO__LEAVE_TASK_METHODDEF | ||
_ASYNCIO__SWAP_CURRENT_TASK_METHODDEF | ||
_ASYNCIO_ALL_TASKS_METHODDEF | ||
_ASYNCIO__CLEAR_FREELIST_METHODDEF | ||
{NULL, NULL} | ||
}; | ||
|
||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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.
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.
I think we want to avoid this sort of duplication if at all possible.