GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Maybe Drop "channels" from _xxsubinterpreters · Issue #101524 · python/cpython

Maybe Drop "channels" from _xxsubinterpreters #101524

New issue
Copy link
New issue
Copy link
Closed
Closed
Maybe Drop "channels" from _xxsubinterpreters#101524
Copy link
Labels
3.12only security fixesonly security fixesextension-modulesC modules in the Modules dirC modules in the Modules dirtopic-subinterpreterstype-featureA feature request or enhancementA feature request or enhancement

Description

@ericsnowcurrently
ericsnowcurrently
opened on Feb 2, 2023
Issue body actions

The _xxsubinterpreters module is essentially the low-level implementation of PEP 554. However, we added it a while back for testing purposes, especially to further exercise the runtime relative to subinterpreters. Since then, I've removed "channels" from PEP 554. So it may make sense to drop that part of the implementation. That part of Modules/_xxsubinterpretersmodule.c is much more code and certainly much more complex than the basic functionality the PEP now describes.

Linked PRs

  • gh-101524: Split Up the _xxsubinterpreters Module #101526
  • gh-101524: Fix the ChannelID tp_name #102655
  • gh-101524: Only Use Public C-API in the _xxsubinterpreters Module #105258
  • [3.12] gh-101524: Only Use Public C-API in the _xxsubinterpreters Module (gh-105258) #107303
  • gh-101524: Only Use Public C-API in the _xxsubinterpreters Module #107359
Reacted by Erlend E. Aasland, Donghee Na and Michael Droettboom

Activity

  1. ericsnowcurrently
    added
    type-featureA feature request or enhancementA feature request or enhancement
    extension-modulesC modules in the Modules dirC modules in the Modules dir
    topic-subinterpreters
    3.12only security fixesonly security fixes
    on Feb 2, 2023
  2. ezio-melotti
    added this to Subinterpreterson Feb 2, 2023
  3. ericsnowcurrently
    added a commit that references this issue on Feb 4, 2023

    gh-101524: Split Up the _xxsubinterpreters Module (gh-101526)

    c67b005
  4. rhettinger commented on Feb 5, 2023

    @rhettinger
    rhettinger
    on Feb 5, 2023
    Contributor
    More actions

    This is a bummer. It was one of the few demonstrable benefits of subinterpreters. It seems that we're left with only the most ancient and awkward forms of communcation, shared file descriptors and pipes. Almost nothing in CSP can be readily expressed with those low level tools.

    Side note: The PEP seems significantly understate the impact of isolating modules, "This situation is limited to modules that use C globals (or use libraries that use C globals) to store internal state.". As Erland Aasland sweeps through standard library making edits, the cost is becoming clear. Even simple stateless modules like bisect get significant churn. More interesting but stateless modules like itertools have about half the lines in the module changed (see gh-101277 for just one-fourth of the edits). The impact is massive. Along the way, the edits negatively impact performance which is annoying because almost the entire point of writing C accelerator code is to fix performance issues on critical code paths. We're losing freelists, ability of a type to reference its own methods, the _Py_IDENTIFIER() optimization, singletons, etc.

    So far, this massive effort seems like all cost and no benefit. Gaining Go-like or CSP-like channels would have been the one win. Otherwise, all we left with is the questionable benefit of "looking like one process to the O/S". It isn't clear at all that that is something we want to pay for.

    The cost/benefit proposition of PEP 554 has changed considerably since the beginning. The code churn is huge and is taking many man-months of time. Meanwhile the proposed benefits are down to "looks like a single process" and "ability to experiment with new concurrency models".

  5. encukou commented on Feb 6, 2023

    @encukou
    encukou
    on Feb 6, 2023
    Member
    More actions

    The cost/benefit proposition of PEP 554 has changed considerably since the beginning. The code churn is huge and is taking many man-months of time.

    Note that the changes you mention are needed to properly support Py_NewInterpreter, which isn't a newly proposed change -- it predates PEPs. The known issues are similarly old, see e.g. this note in Python 2.0 docs:

    (XXX This is a hard-to-fix bug that will be addressed in a future release.)

    PEP 554 exposes the ancient functionality to Python code. Withoun channels its benefit decreases (or rather, is moved to a future PEP as this one is too long), but the cost is unrelated to PEP 554.

    So where to express concerns with “isolation” changes? Please read https://docs.python.org/3/howto/isolating-extensions.html, and open a Discourse discussion if you disagree with anything there or if the changes do more than necessary. (FWIW, I do think we should be a little more conservative/careful and invest more in groundwork so the individual changes aren't as invasive -- but on the other hand I don't want to stop people from improving things. The groundwork for fixing static types, for example, would probably be multi-year effort with uncertain results.)

    Reacted by Erlend E. Aasland
  6. ericsnowcurrently commented on Feb 6, 2023

    @ericsnowcurrently
    ericsnowcurrently
    on Feb 6, 2023
    MemberAuthor
    More actions

    This is a bummer. It was one of the few demonstrable benefits of subinterpreters. It seems that we're left with only the most ancient and awkward forms of communcation, shared file descriptors and pipes. Almost nothing in CSP can be readily expressed with those low level tools.

    Thanks for speaking up about this, Raymond.

    I removed channels from PEP 554 because readers kept getting lost in that part of the proposed API and my descriptions and examples. Ultimately, I'm still not sure the API is quite right. Furthermore, it isn't a good sign that the implementation is so complex.

    That said, I still think channels (or whatever we call them) are the best primitive for the concurrency model and anticipate they will be part of the stdlib sooner rather than later. I just don't want PEP 554 to be held up by that. In the meantime, we can give the channels design some time to bake on PyPI.

    FWIW, I'm open to more discussion on this if you think channels are important enough to keep in PEP 554. Just keep in mind that I'm hoping to have PEP 554 accepted in time for 3.12.

    Side note: The PEP seems significantly understate the impact of isolating modules

    Petr has covered everything I would have said.

    Reacted by Erlend E. Aasland and Raymond Hettinger
  7. erlend-aasland commented on Feb 7, 2023

    @erlend-aasland
    erlend-aasland
    on Feb 7, 2023
    Contributor
    More actions

    I removed channels from PEP 554 because readers kept getting lost in that part of the proposed API and my descriptions and examples. Ultimately, I'm still not sure the API is quite right. Furthermore, it isn't a good sign that the implementation is so complex.

    I think it is wise to leave channels out of PEP-554, leaving the resulting PEP more focused, especially since it is targeting 3.12.

    That said, I still think channels (or whatever we call them) are the best primitive for the concurrency model and anticipate they will be part of the stdlib sooner rather than later. I just don't want PEP 554 to be held up by that. In the meantime, we can give the channels design some time to bake on PyPI.

    This sounds like a very good path forward.

    Side note: The PEP seems significantly understate the impact of isolating modules

    Petr has covered everything I would have said.

    +1

  8. erlend-aasland commented on Feb 8, 2023

    @erlend-aasland
    erlend-aasland
    on Feb 8, 2023
    Contributor
    More actions

    AFAICS, we can close this.

  9. ericsnowcurrently commented on Feb 8, 2023

    @ericsnowcurrently
    ericsnowcurrently
    on Feb 8, 2023
    MemberAuthor
    More actions

    Thus far I've only split out an _xxinterpchannels module. I haven't removed it. It might still be worth keeping for testing purposes (it has uncovered various bugs), regardless of PEP 554.

    Reacted by Erlend E. Aasland
  10. erlend-aasland commented on Feb 8, 2023

    @erlend-aasland
    erlend-aasland
    on Feb 8, 2023
    Contributor
    More actions

    FWIW, I'm fine with keeping it; having a broad test suite is never a bad idea.

    Reacted by Eric Snow
  11. ericsnowcurrently
    added a commit that references this issue on Mar 13, 2023

    gh-101524: Fix the ChannelID tp_name (gh-102655)

    74885a0
  12. carljm
    added a commit that references this issue on Mar 14, 2023

    Merge branch 'main' into inlinecomp2

    6c5f269
  13. Fidget-Spinner
    added a commit that references this issue on Mar 27, 2023

    pythongh-101524: Fix the ChannelID tp_name (pythongh-102655)

    762e024
  14. warsaw
    added a commit that references this issue on Apr 11, 2023

    pythongh-101524: Fix the ChannelID tp_name (pythongh-102655)

    e77e377
  15. ericsnowcurrently
    added a commit that references this issue on Jun 2, 2023

    gh-101524: Only Use Public C-API in the _xxsubinterpreters Module (gh…

    e6373c0
  16. miss-islington
    added a commit that references this issue on Jul 26, 2023

    pythongh-101524: Only Use Public C-API in the _xxsubinterpreters Modu…

    f1e88f8
  17. ericsnowcurrently
    added 2 commits that reference this issue on Jul 27, 2023

    [3.12] gh-101524: Only Use Public C-API in the _xxsubinterpreters Mod…

    57ef065

    gh-101524: Only Use Public C-API in the _xxsubinterpreters Module (gh…

    8bdae14
  18. encukou commented on Aug 2, 2023

    @encukou
    encukou
    on Aug 2, 2023
    Member
    More actions

    From #107359:

    The _xxsubinterpreters module should not rely on internal API. Some of the functions it uses were recently moved there however. Here we move them back (and expose them properly).

    To expose unstable API properly, it needs documentation and tests. Are you planning to add those?

  19. ericsnowcurrently commented on Aug 2, 2023

    @ericsnowcurrently
    ericsnowcurrently
    on Aug 2, 2023
    MemberAuthor
    More actions

    I will.

    Reacted by Petr Viktorin and Erlend E. Aasland
  20. ericsnowcurrently commented on Aug 3, 2023

    @ericsnowcurrently
    ericsnowcurrently
    on Aug 3, 2023
    MemberAuthor
    More actions

    See gh-107784.

  21. erlend-aasland commented on Jan 12, 2024

    @erlend-aasland
    erlend-aasland
    on Jan 12, 2024
    Contributor
    More actions

    To expose unstable API properly, it needs documentation and tests. Are you planning to add those?

    Documentation has been split out to a separate issue. IIRC, there is already an issue regarding test coverage for subinterpreters. Given this, I don't see the need for holding this issue open anymore; I recommend closing this issue.

  22. erlend-aasland
    added
    pendingThe issue will be closed if no feedback is providedThe issue will be closed if no feedback is provided
    on Jan 12, 2024
  23. encukou
    closed this as completedon Jan 25, 2024
  24. github-project-automation
    moved this to Done in Subinterpreterson Jan 25, 2024
  25. erlend-aasland
    removed
    pendingThe issue will be closed if no feedback is providedThe issue will be closed if no feedback is provided
    on Jan 25, 2024
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.12only security fixesonly security fixesextension-modulesC modules in the Modules dirC modules in the Modules dirtopic-subinterpreterstype-featureA feature request or enhancementA feature request or enhancement

    Projects

    • Subinterpreters
      Status
      Done
      Show more project fields

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

    You are visitor 01402503 — GeezerVison does not store the pages you look up.
    Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.