[subinterpreters] Move _PyRuntimeState.ceval to PyInterpreterState

[vstinner]

BPO	40513
Nosy	@vstinner, @ericsnowcurrently, @shihai1991
PRs	bpo-40513: Per-interpreter signals pending #19924 bpo-40513: Per-interpreter gil_drop_request #19927 bpo-40513: Per-interpreter recursion_limit #19929 Revert "bpo-40513: Per-interpreter signals pending (GH-19924)" #19932 bpo-40513: new_interpreter() init GIL earlier #19942 bpo-40513: Per-interpreter GIL #19943 bpo-40513: _xxsubinterpreters.run_string() releases the GIL #19944

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-05-15.01:34:28.794>
created_at = <Date 2020-05-05.12:56:32.971>
labels = ['expert-subinterpreters', '3.9']
title = '[subinterpreters] Move _PyRuntimeState.ceval to PyInterpreterState'
updated_at = <Date 2020-05-15.01:34:28.793>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2020-05-15.01:34:28.793>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2020-05-15.01:34:28.794>
closer = 'vstinner'
components = ['Subinterpreters']
creation = <Date 2020-05-05.12:56:32.971>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 40513
keywords = ['patch']
message_count = 17.0
messages = ['368137', '368139', '368147', '368150', '368151', '368153', '368162', '368165', '368168', '368169', '368174', '368178', '368179', '368190', '368194', '368196', '368909']
nosy_count = 3.0
nosy_names = ['vstinner', 'eric.snow', 'shihai1991']
pr_nums = ['19924', '19927', '19929', '19932', '19942', '19943', '19944']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue40513'
versions = ['Python 3.9']

[vstinner]

To get one GIL per interpreter (bpo-40512), remaining _PyRuntimeState.ceval members should be moved to PyInterpreterState.ceval. The work started in bpo-39984 with ceval "pending calls" and ceval "eval breaker".

There are 4 remaining fields:

struct _ceval_runtime_state {
    int recursion_limit;
    /* Request for dropping the GIL */
    _Py_atomic_int gil_drop_request;
    /* Request for checking signals. */
    _Py_atomic_int signals_pending;
    struct _gil_runtime_state gil;
};

The most complex part will be to move the "gil" member: bpo-40512 lists all sub-tasks which must be completed before being able to have one GIL per interpreter without crashing.

[vstinner]

See also: ericsnowcurrently/multi-core-python#34

[vstinner]

New changeset 4e01946 by Victor Stinner in branch 'master':
bpo-40513: Per-interpreter signals pending (GH-19924)
4e01946

[vstinner]

Python/ceval.c also has "int _Py_CheckRecursionLimit = Py_DEFAULT_RECURSION_LIMIT;". _Py_CheckRecursiveCall() has this comment:

/* Needed for ABI backwards-compatibility (see bpo-31857) */
_Py_CheckRecursionLimit = recursion_limit;

I converted Py_EnterRecursiveCall() and Py_LeaveRecursiveCall() macros into opaque functions in bpo-38644: commit f4b1e3d.

I don't think that the "ABI backwards-compatibility" rationale is correct. These macros never worked in the limited C API: they accessed PyThreadState.recursion_depth and PyThreadState.overflowed members, whereas the PyThreadState structure is opaque in the limited C API (on purpose, see also bpo-39947).

I think that it is now safe to simply remove _Py_CheckRecursionLimit.

[vstinner]

New changeset 0b1e330 by Victor Stinner in branch 'master':
bpo-40513: Per-interpreter gil_drop_request (GH-19927)
0b1e330

[vstinner]

For signals_pending, see also: bpo-40010 "Inefficient signal handling in multithreaded applications" which added _Py_ThreadCanHandleSignals() and _Py_ThreadCanHandlePendingCalls() functions.

[vstinner]

New changeset 4e30ed3 by Victor Stinner in branch 'master':
bpo-40513: Per-interpreter recursion_limit (GH-19929)
4e30ed3

[ericsnowcurrently]

FWIW, I think it would make sense to keep "signals_pending" under _PyRuntimeState rather than moving it to PyInterpreterState.

Signals are only handled by the main interpreter (in its main thread). Even though "signals_pending" is useful to only one interpreter in the runtime, making it per-interpreter sends the wrong message that it is significant at that level. This may be confusing to readers of the code.

At the very least there should be a clear comment with the field in Include/internal/pycore_interp.h explaining how it is only used by the main interpreter and why we made it per-interpreter anyway.

[ericsnowcurrently]

From a user perspective, does it make sense to have a different recursion_limit per interpreter? I don't see a problem with it. However, does it make sense to also keep a global value that we default to when a per-interpreter value is not set? I think it might.

I suppose a bigger question is what users will expect the recursion limit (AKA "sys.getrecursionlimit()") to be for a newly created subinterpreter. Will it be some global default? Will it be the value from the parent interpreter? I'd go with a global default, which would imply that the default value should be stored under _PyRuntimeState like we had it (but still keep the actual per-interpreter field for the actual value).

FWIW, the existing docs don't really block either approach.

[ericsnowcurrently]

If this issue covers the GIL (which it seems to) then I'd expect _PyRuntimeState.gilstate to be handled here too.

[vstinner]

New changeset 299b8c6 by Victor Stinner in branch 'master':
Revert "bpo-40513: Per-interpreter signals pending (GH-19924)" (GH-19932)
299b8c6