gh-99113: A Per-Interpreter GIL! #99114
Conversation
ericsnowcurrently
requested review from
1st1,
asvetlov,
kumaraditya303 and
willingc
as code owners
|
318 commits, 114 files, 23 reviewers… How are we going to do this? |
We can create a separate PR that just defines a no-op slot Thanks to it, this PR will become a much smaller follow-up. |
AlexWaygood
removed request for
a team,
corona10,
pablogsal,
erlend-aasland,
Fidget-Spinner,
isidentical,
kumaraditya303 and
AlexWaygood
Not sure how that happened — I only meant to remove my own request for review... |
Actually, most of the changed files are repetitive, trivial updates of various extension module's PyModuleDef structs. |
Their repetitiveness takes attention from other parts of the diff and requires to constantly scroll through all of them for each inter-file comparison in other parts of the PR. It makes the analysis harder. That's why I proposed to move them out into a separate PR, merge it and update this PR. |
|
I quickly went through and marked all those files as "Viewed", then used the File Filter drop down to hide viewed files. So I've only got 7 files left with non-trivial changes, one of which is the NEWS entry.
Looks like the "viewed" state is remembered, but the filter needs to be set each time I go back in. |
| const PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT; | ||
| PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT; | ||
| // The main interpreter always has its own GIL. | ||
| config.own_gil = 1; |
There was a problem hiding this comment.
Presumably we're in a lot of trouble if the main interpreter exits first...
Do we have mechanisms to ensure it outlives any subinterpreters that share its GIL? Or perhaps we should reference count the GIL instead of marking ownership?
There was a problem hiding this comment.
Yeah, the main interpreter must be the last one. There are many things that break if that isn't the case.
| struct _gil_runtime_state *gil = &_PyRuntime.ceval.gil; | ||
| PyInterpreterState *interp = _PyInterpreterState_Get(); | ||
| struct _gil_runtime_state *gil = interp->ceval.gil; | ||
| assert(gil != NULL); | ||
| gil->interval = microseconds; |
There was a problem hiding this comment.
Do we need some way to propagate this to new subinterpreters? I don't see it if it's here.
Not sure whether this would catch people out or not. Presumably increasing the switch count is less important with proper parallelism, and it's documented as setting "the interpreter's switch interval" so we're clear on that front. But it may be surprising.
Probably only worth adding a note to the sys docs to clarify that it doesn't propagate to new subinterpreter GILs.
There was a problem hiding this comment.
Yeah, we'll make sure the docs are clear.
There was a problem hiding this comment.
I've left a TODO note on the issue: #99113 (comment).
|
I had seriously considered splitting this PR up yesterday. I should have listened to myself. 🙂 I'm going to split it up. |
|
I've noted the new PRs above in the PR summary. |
…pythongh-104148) I'll be adding a value to indicate support for per-interpreter GIL in pythongh-99114.
I've split up this PR:
That last one is effectively the superseder of this one.
This is the culmination of PEP 684 (and of my 8-year long multi-core Python project)!
Each subinterpreter may now be created with its own GIL (via
Py_NewInterpreterFromConfig()). If not so configured then the interpreter will share with the main interpreter--the status quo since the subinterpreters were added decades ago. The main interpreter always has its own GIL and subinterpreters fromPy_NewInterpreter()will always share with the main interpreter.This is essentially the correct implementation but it may change here and there before we've reached the end.
We won't merge this until:
I'm merging in other branches that this one relies on, but those will wash out as the other PRs get merged. In the meantime, you can see the actual changes here: https://github.com/python/cpython/compare/main...ericsnowcurrently:per-interpreter-gil-new-bare?expand=1.