Skip to content

[subinterpreters] Move _PyRuntimeState.ceval to PyInterpreterState #84693

Closed
Closed
[subinterpreters] Move _PyRuntimeState.ceval to PyInterpreterState#84693
Labels
3.9 (EOL)end of lifetopic-subinterpreters

Description

@vstinner
vstinner
opened on May 5, 2020
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']

    Activity

    1. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

      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.

    2. vstinner
      added
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      3.9 (EOL)end of life
      on May 5, 2020

    3. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

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

    4. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

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

    5. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

      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.

    6. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

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

    7. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

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

    8. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

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

    9. ericsnowcurrently commented on May 5, 2020

      @ericsnowcurrently
      ericsnowcurrently
      on May 5, 2020
      Member

      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.

    10. ericsnowcurrently commented on May 5, 2020

      @ericsnowcurrently
      ericsnowcurrently
      on May 5, 2020
      Member

      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.

    11. ericsnowcurrently commented on May 5, 2020

      @ericsnowcurrently
      ericsnowcurrently
      on May 5, 2020
      Member

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

    12. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

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

    13. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

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

      Oh right. I forgot about the details. I reverted my change, but this time I added a comment to prevent me to write the same change in 6 months :-)

    14. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

      From a user perspective, does it make sense to have a different recursion_limit per interpreter?

      Yes, I think so. Someone might use a lower limit to run a template rendering engine or "unsafe" code. Well, it's not hard to find random usage for interpreters with a configuration different than the main interpreter.

    15. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

      New changeset 0dd5e7a by Victor Stinner in branch 'master':
      bpo-40513: new_interpreter() init GIL earlier (GH-19942)
      0dd5e7a

    16. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

      New changeset 7be4e35 by Victor Stinner in branch 'master':
      bpo-40513: Per-interpreter GIL (GH-19943)
      7be4e35

    17. vstinner commented on May 5, 2020

      @vstinner
      vstinner
      on May 5, 2020
      MemberAuthor

      New changeset fb2c7c4 by Victor Stinner in branch 'master':
      bpo-40513: _xxsubinterpreters.run_string() releases the GIL (GH-19944)
      fb2c7c4

    18. vstinner
      added
      topic-subinterpreters
      and removed
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      on May 15, 2020
    19. vstinner
      changed the title [-]Move _PyRuntimeState.ceval to PyInterpreterState[/-] [+][subinterpreters] Move _PyRuntimeState.ceval to PyInterpreterState[/+] on May 15, 2020
    20. vstinner
      added
      topic-subinterpreters
      and removed
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      on May 15, 2020
    21. vstinner
      changed the title [-]Move _PyRuntimeState.ceval to PyInterpreterState[/-] [+][subinterpreters] Move _PyRuntimeState.ceval to PyInterpreterState[/+] on May 15, 2020

    22. vstinner commented on May 15, 2020

      @vstinner
      vstinner
      on May 15, 2020
      MemberAuthor

      Well, the remaining field which should be moved is the GIL lock itself. It will likely be the last thing to do in bpo-40512. I consider that the work is done in this issue and so I close it.

    23. vstinner
      closed this as completedon May 15, 2020
    24. vstinner
      closed this as completedon May 15, 2020
    25. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.9 (EOL)end of lifetopic-subinterpreters

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions