Skip to content
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

Add internal-only _PyThreadStateImpl "wrapper" for PyThreadState #112538

Closed
Tracked by #108219
colesbury opened this issue Nov 29, 2023 · 0 comments
Closed
Tracked by #108219

Add internal-only _PyThreadStateImpl "wrapper" for PyThreadState #112538

colesbury opened this issue Nov 29, 2023 · 0 comments
Labels
3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-free-threading type-feature A feature request or enhancement

Comments

@colesbury
Copy link
Contributor

colesbury commented Nov 29, 2023

Feature or enhancement

The PyThreadState struct definitions (i.e., struct _ts) is visible in Python's public C API. Our documentation lists only the "interp" field as public. In practice, all the fields are visible, and some extensions (like Cython) make use of some of those fields.

We will want to add some private fields on a per-PyThreadState basis for the --disable-gil builds, but we also do not want to expose them publicly.

The "solution" used in nogil-3.12 was to add a new struct _PyThreadStateImpl that contains the PyThreadState as well as the private fields. Every PyThreadState is actually a _PyThreadStateImpl -- you can cast pointers from one to the other safely. Only the code that allocates & frees PyThreadStates (in pystate.c) and code that accesses those private fields needs to know about _PyThreadStateImpl. Everything else can keep using PyThreadState* pointers.

Here is an example definition:

typedef struct {
    PyThreadState base;
    // private fields go here
#ifdef Py_GIL_DISABLED
    struct _Py_float_state float_state; // e.g., float free-list
#endif
    ...
} _PyThreadStateImpl;

Some of the private fields include free lists, state for biased reference counting, and mimalloc heaps. Free lists are currently stored per-interpreter (in PyInterpreterState), but we will want them per-PyThreadState in --disable-gil builds.

Alternative

We could instead add an opaque pointer from PyThreadState to private data. For example, something like:

typedef struct _PyThreadStatePrivate _PyThreadStatePrivate;  // opaque in the public API

struct _ts {
    ...
    _PyThreadStatePrivate *private;
};

This requires an extra pointer-dereference to access private data, and many of the use cases for these private fields are performance sensitive (e.g., free lists, memory allocation).

cc @ericsnowcurrently, @vstinner

Linked PRs

@colesbury colesbury added type-feature A feature request or enhancement interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.13 bugs and security fixes topic-free-threading labels Nov 29, 2023
colesbury added a commit to colesbury/cpython that referenced this issue Nov 30, 2023
…yThreadState

Every PyThreadState instance is now actually a _PyThreadStateImpl. It is
safe to cast from `PyThreadState*` to `_PyThreadStateImpl*` and back. The
_PyThreadStateImpl will contain fields that we do not want to expose in
the public C API.
ericsnowcurrently pushed a commit that referenced this issue Dec 7, 2023
…dState (gh-112560)

Every PyThreadState instance is now actually a _PyThreadStateImpl.
It is safe to cast from `PyThreadState*` to `_PyThreadStateImpl*` and back.
The _PyThreadStateImpl will contain fields that we do not want to expose
in the public C API.
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
…yThreadState (pythongh-112560)

Every PyThreadState instance is now actually a _PyThreadStateImpl.
It is safe to cast from `PyThreadState*` to `_PyThreadStateImpl*` and back.
The _PyThreadStateImpl will contain fields that we do not want to expose
in the public C API.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…yThreadState (pythongh-112560)

Every PyThreadState instance is now actually a _PyThreadStateImpl.
It is safe to cast from `PyThreadState*` to `_PyThreadStateImpl*` and back.
The _PyThreadStateImpl will contain fields that we do not want to expose
in the public C API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-free-threading type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants