-
-
Notifications
You must be signed in to change notification settings - Fork 33.3k
gh-140544: store pointer to interpreter state as a thread local for fast access #140573
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
Conversation
428d3e0 to
7115c75
Compare
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.
The core change looks good. Mostly nitpicks from me.
Misc/NEWS.d/next/Core_and_Builtins/2025-10-25-07-25-52.gh-issue-140544.lwjtQe.rst
Show resolved
Hide resolved
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.
LGTM
| _Py_EnsureTstateNotNULL(tstate); | ||
| #endif | ||
| return tstate->interp; | ||
| #if !defined(Py_BUILD_CORE_MODULE) |
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.
Should this be
#if defined(HAVE_THREAD_LOCAL) && !defined(Py_BUILD_CORE_MODULE)
instead?
When testing the current main against mypyc, I'm seeing a new error message
In file included from /.../mypy/mypyc/lib-rt/misc_ops.c:1137:
In file included from /.../cpython/Include/internal/pycore_object.h:12:
In file included from /.../cpython/Include/internal/pycore_gc.h:12:
/.../cpython/Include/internal/pycore_pystate.h:213:12: error: use of undeclared identifier '_Py_tss_interp'
return _Py_tss_interp;
^
1 error generated.
Mypyc does use some of the internal headers. In particular
#define Py_BUILD_CORE
...
#include "internal/pycore_object.h"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 guess it is because mypyc doesn't define HAVE_THREAD_LOCAL which is defined when python is built, does #140623 fixes it for you?
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 should just remove the check for HAVE_THREAD_LOCAL on line 92.
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 guess it is because mypyc doesn't define HAVE_THREAD_LOCAL which is defined when python is built, does #140623 fixes it for you?
Yes, that works.
I think we should just remove the check for
HAVE_THREAD_LOCALon line 92.
Guess, this would work too.
Uh oh!
There was an error while loading. Please reload this page.