-
-
Notifications
You must be signed in to change notification settings - Fork 31.3k
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
bpo-40512: Store pointer to interpreter state in a thread local variable #29228
Changes from all commits
7c7e2b2
73c410b
f22036b
f33f4a4
55cc2e4
4b83356
8502145
28bc042
193b646
a30b7e6
3982884
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
|
||
#ifndef Py_CORE_TLS_H | ||
#define Py_CORE_TLS_H | ||
|
||
#ifndef Py_BUILD_CORE | ||
# error "this header requires Py_BUILD_CORE to be defined" | ||
#endif | ||
|
||
#ifndef _Py_THREAD_LOCAL | ||
#if defined(__GNUC__) | ||
#define _Py_THREAD_LOCAL __thread | ||
#endif | ||
#if defined(_MSC_VER) | ||
#define _Py_THREAD_LOCAL __declspec(thread) | ||
#endif | ||
#endif | ||
|
||
#endif /* Py_CORE_TLS_H */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
A pointer to the interpreter state is stored in a thread local variable | ||
on platforms that support it. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ | |
#define PY_SSIZE_T_CLEAN | ||
#include "Python.h" | ||
#include "_iomodule.h" | ||
#include "pycore_pystate.h" // _PyInterpreterState_GET() | ||
#include "pycore_pystate.h" // PyInterpreterState_Get() | ||
|
||
#ifdef HAVE_SYS_TYPES_H | ||
#include <sys/types.h> | ||
|
@@ -91,7 +91,7 @@ PyDoc_STRVAR(module_doc, | |
" I/O classes. open() uses the file's blksize (as obtained by os.stat) if\n" | ||
" possible.\n" | ||
); | ||
|
||
|
||
/* | ||
* The main open() function | ||
|
@@ -509,7 +509,7 @@ _io_text_encoding_impl(PyObject *module, PyObject *encoding, int stacklevel) | |
/*[clinic end generated code: output=91b2cfea6934cc0c input=bf70231213e2a7b4]*/ | ||
{ | ||
if (encoding == NULL || encoding == Py_None) { | ||
PyInterpreterState *interp = _PyInterpreterState_GET(); | ||
PyInterpreterState *interp = PyInterpreterState_Get(); | ||
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. Why the switch to 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. Modules cannot access a thread-local directly because they are dynamically loaded. It is a loader, rather a language, limitation, I think. 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. That's good to know. Thanks for explaining. This might not be obvious to maintainers. Would you mind adding a short comment next to the declaration of Also, this doesn't affect builtin modules, right? Aren't some of the modules updated here builtin modules? |
||
if (_PyInterpreterState_GetConfig(interp)->warn_default_encoding) { | ||
if (PyErr_WarnEx(PyExc_EncodingWarning, | ||
"'encoding' argument not specified", stacklevel)) { | ||
|
@@ -542,7 +542,7 @@ _io_open_code_impl(PyObject *module, PyObject *path) | |
{ | ||
return PyFile_OpenCodeObject(path); | ||
} | ||
|
||
/* | ||
* Private helpers for the io module. | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -27,7 +27,7 @@ | |||||||
#include "pycore_ceval.h" // _PyEval_ReInitThreads() | ||||||||
#include "pycore_import.h" // _PyImport_ReInitLock() | ||||||||
#include "pycore_initconfig.h" // _PyStatus_EXCEPTION() | ||||||||
#include "pycore_pystate.h" // _PyInterpreterState_GET() | ||||||||
#include "pycore_pystate.h" // PyInterpreterState_Get() | ||||||||
|
||||||||
#include "structmember.h" // PyMemberDef | ||||||||
#ifndef MS_WINDOWS | ||||||||
|
@@ -555,7 +555,7 @@ run_at_forkers(PyObject *lst, int reverse) | |||||||
void | ||||||||
PyOS_BeforeFork(void) | ||||||||
{ | ||||||||
run_at_forkers(_PyInterpreterState_GET()->before_forkers, 1); | ||||||||
run_at_forkers(PyInterpreterState_Get()->before_forkers, 1); | ||||||||
|
||||||||
_PyImport_AcquireLock(); | ||||||||
} | ||||||||
|
@@ -566,7 +566,7 @@ PyOS_AfterFork_Parent(void) | |||||||
if (_PyImport_ReleaseLock() <= 0) | ||||||||
Py_FatalError("failed releasing import lock after fork"); | ||||||||
|
||||||||
run_at_forkers(_PyInterpreterState_GET()->after_forkers_parent, 0); | ||||||||
run_at_forkers(PyInterpreterState_Get()->after_forkers_parent, 0); | ||||||||
} | ||||||||
|
||||||||
void | ||||||||
|
@@ -6665,7 +6665,7 @@ os_register_at_fork_impl(PyObject *module, PyObject *before, | |||||||
check_null_or_callable(after_in_parent, "after_in_parent")) { | ||||||||
return NULL; | ||||||||
} | ||||||||
interp = _PyInterpreterState_GET(); | ||||||||
interp = PyInterpreterState_Get(); | ||||||||
|
||||||||
if (register_at_forker(&interp->before_forkers, before)) { | ||||||||
return NULL; | ||||||||
|
@@ -6695,8 +6695,7 @@ os_fork1_impl(PyObject *module) | |||||||
/*[clinic end generated code: output=0de8e67ce2a310bc input=12db02167893926e]*/ | ||||||||
{ | ||||||||
pid_t pid; | ||||||||
|
||||||||
if (_PyInterpreterState_GET() != PyInterpreterState_Main()) { | ||||||||
if (PyInterpreterState_Get() != PyInterpreterState_Main()) { | ||||||||
PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters"); | ||||||||
return NULL; | ||||||||
} | ||||||||
|
@@ -6730,7 +6729,7 @@ os_fork_impl(PyObject *module) | |||||||
/*[clinic end generated code: output=3626c81f98985d49 input=13c956413110eeaa]*/ | ||||||||
{ | ||||||||
pid_t pid; | ||||||||
PyInterpreterState *interp = _PyInterpreterState_GET(); | ||||||||
PyInterpreterState *interp = PyInterpreterState_Get(); | ||||||||
if (interp->config._isolated_interpreter) { | ||||||||
PyErr_SetString(PyExc_RuntimeError, | ||||||||
"fork not supported for isolated subinterpreters"); | ||||||||
|
@@ -7348,7 +7347,7 @@ os_forkpty_impl(PyObject *module) | |||||||
int master_fd = -1; | ||||||||
pid_t pid; | ||||||||
|
||||||||
if (_PyInterpreterState_GET() != PyInterpreterState_Main()) { | ||||||||
if (PyInterpreterState_Get() != PyInterpreterState_Main()) { | ||||||||
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.
Suggested change
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. It is too easy to overlook the |
||||||||
PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters"); | ||||||||
return NULL; | ||||||||
} | ||||||||
|
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.
Why the current interpreter and not the current
PyThreadState
?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.
Two reasons.
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.
Would it make sense to do both? (They are both hot in different situations.) What is the cost to the number of thread-local variables?