Skip to content

pull some import state out of the interpreter state #58820

Closed
Closed
pull some import state out of the interpreter state#58820
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)

Description

@ericsnowcurrently
ericsnowcurrently
opened on Apr 18, 2012
BPO 14615
Nosy @loewis, @brettcannon, @ericsnowcurrently

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 2012-04-19.01:33:05.070>
created_at = <Date 2012-04-18.16:32:41.758>
labels = ['interpreter-core']
title = 'pull some import state out of the interpreter state'
updated_at = <Date 2012-04-19.01:33:29.108>
user = 'https://github.com/ericsnowcurrently'

bugs.python.org fields:

activity = <Date 2012-04-19.01:33:29.108>
actor = 'eric.snow'
assignee = 'none'
closed = True
closed_date = <Date 2012-04-19.01:33:05.070>
closer = 'eric.snow'
components = ['Interpreter Core']
creation = <Date 2012-04-18.16:32:41.758>
creator = 'eric.snow'
dependencies = []
files = []
hgrepos = []
issue_num = 14615
keywords = []
message_count = 11.0
messages = ['158638', '158640', '158643', '158647', '158652', '158659', '158666', '158671', '158690', '158692', '158693']
nosy_count = 3.0
nosy_names = ['loewis', 'brett.cannon', 'eric.snow']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue14615'
versions = ['Python 3.3']

Activity

  1. ericsnowcurrently commented on Apr 18, 2012

    @ericsnowcurrently
    ericsnowcurrently
    on Apr 18, 2012
    MemberAuthor

    Once the dust clears from the bpo-2377 and bpo-13959, we should consider what import-state-related members of PyInterpreterState (Include/pystate.h) can be removed. This is in the interest of simplifying the interpreter state.

    The most straightforward candidate is 'modules_reloading' (since reload() will likely become pure python), but we'll have to make sure we do not introduce any race conditions.

    Another candidate that could probably go away, regardless of the import work, is 'modules_by_index'. As far as I can see, there is only one use of interp->modules_by_index in the cpython code-base: PyState_FindModule() in Python/pystate.c. Likewise there is only one use of PyState_FindModule(): atexit_callfuncs() in Modules/atexitmodule.c.

    Ultimately I'd love it if modules were also removed from the interpreter state, but doing that is not so cut-and-dry.

  2. ericsnowcurrently
    added
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    on Apr 18, 2012

  3. brettcannon commented on Apr 18, 2012

    @brettcannon
    brettcannon
    on Apr 18, 2012
    Member

    So PyState_FindModule() is an (undocumented) part of the public API, which means that it has to somehow be supported to keep ABI compatibility (unless I am reading PEP-384 incorrectly).

  4. ericsnowcurrently commented on Apr 18, 2012

    @ericsnowcurrently
    ericsnowcurrently
    on Apr 18, 2012
    MemberAuthor

    Curse you, undocumented API that we have to support! It could still wrap a simple get() on sys.modules, roughly like this:

    PyObject*
PyState_FindModule(struct PyModuleDef* m)
{
    PyObject* modules, module;
    modules = PyImport_GetModuleDict();
    if (modules == NULL)
        return NULL;
    module = PyDict_GetItemString(sys_modules, m->m_name);
    return module==Py_None ? NULL : module;
}

  5. brettcannon commented on Apr 18, 2012

    @brettcannon
    brettcannon
    on Apr 18, 2012
    Member

    Right, so the question is why wasn't that done in the first place? Who decided this modules_by_index concept was even worth it?

  6. loewis commented on Apr 18, 2012

    @loewis
    loewismannequin
    on Apr 18, 2012
    Mannequin

    Another candidate that could probably go away, regardless of the
    import work, is 'modules_by_index'. As far as I can see, there is
    only one use of interp->modules_by_index in the cpython code-base:
    PyState_FindModule() in Python/pystate.c. Likewise there is only one
    use of PyState_FindModule(): atexit_callfuncs() in
    Modules/atexitmodule.c.

    There will be certainly many more uses of PySteate_FindModule in the
    future. I fail to see what is gained by this kind of change, and am
    firmly -1 on removing variables arbitrarily.

  7. loewis
    changed the title [-]pull some import state out of the interpreter state[/-] [+]pull some import state out of the interpreter state[/+] on Apr 18, 2012

  8. ericsnowcurrently commented on Apr 18, 2012

    @ericsnowcurrently
    ericsnowcurrently
    on Apr 18, 2012
    MemberAuthor

    Sorry, Martin, for not looking at PEP-3121 before. I was thinking modules_by_index was a lot older.

  9. ericsnowcurrently commented on Apr 18, 2012

    @ericsnowcurrently
    ericsnowcurrently
    on Apr 18, 2012
    MemberAuthor

    Rather than being arbitrary, the motivation here is to limit amount of the import state that is specific to CPython. I apologize that that wasn't clear. The three import-related members of PyInterpreterState (modules, modules_reloading, and modules_by_index), are implementation-specific. Regarding each of them:

    modules_by_index was explicitly designed for C extension modules, which are (mostly) CPython-specific. It's essentially a behind-the-scenes optimization. Martin's right that we should leave it alone.

    modules_reloading is an artifact of the C import implementation, and (like modules_by_index) doesn't have extra bearing on the import state. However, if the importlib bootstrap renders it superfluous, why not yank it?

    Finally, modules (the biggie) is accessible as sys.modules. importlib uses sys.modules. The C import implementation used interp->modules directly. [1] Most (all) of that usage is going away. Again, _if_ that is the case, why keep it around?

    Granted, the C implementation takes advantage of interp->modules as an optimized path for getting a module (vs. the extra step of grabbing sys.modules directly), so perhaps the intention is to keep that fast path for C extension modules, etc. However, I'm naively skeptical of how much performance that buys you when all is said and done.

    Just to be clear, I do _not_ want to make changes willy-nilly. (I've even grown more conservative in what discussion topics I bring up.) This issue has no urgency attached to it, in my mind. It is the result of an actionable conversation that I didn't want to lose track of. If anything here is doable for 3.3, great! If not, that's fine too. If people think it's a bad idea, I'll accept that gladly, but I think consideration of the idea is justifiable. I'm glad there are plenty of sensible minds around to keep Python on a good track. :)

    [1] This causes a problem in a corner case (see bpo-12633), but it's easy to deal with.

  10. loewis commented on Apr 18, 2012

    @loewis
    loewismannequin
    on Apr 18, 2012
    Mannequin

    Rather than being arbitrary, the motivation here is to limit amount
    of the import state that is specific to CPython.

    What is gained by doing that?

    Finally, modules (the biggie) is accessible as sys.modules.
    importlib uses sys.modules. The C import implementation used
    interp->modules directly. [1] Most (all) of that usage is going
    away. Again, _if_ that is the case, why keep it around?

    I think each of them should be considered on a case-by-case basis.
    Feel free to submit a patch that eliminates ->modules. People may
    find reasons not to do so when they see an actual patch.

    I suggest to close this issue, and encourage people who have the
    desire to eliminate certain state as individual patches.

    Just to be clear, I do _not_ want to make changes willy-nilly. (I've
    even grown more conservative in what discussion topics I bring up.)
    This issue has no urgency attached to it, in my mind. It is the
    result of an actionable conversation that I didn't want to lose track
    of.

    Then I think it doesn't belong in this bug tracker. I have five or
    ten "grand plans" of things that should change in Python at some point;
    putting them into a bug tracker is only confusing people, though, since
    no implementation might be coming forward (for some of the things, I
    have been pondering for the last eight years). For many of the things,
    I ended up writing PEPs since they were significant changes.

    So if this is one of your grand plans, feel free to mention it on
    python-dev. Putting it on the bug tracker asks for specific action.
    If I had to act on this issue, I'd outright reject it.

  11. brettcannon commented on Apr 19, 2012

    @brettcannon
    brettcannon
    on Apr 19, 2012
    Member

    Eric put this in the bug tracker because I asked him to; I'm getting more emails about stuff about importlib that it's a little hard to keep track of everything.

    But originally this was about exposing what is left of the C-level APIs so it no longer was hidden. I'm fine with rescoping this bug to only looking at modules_reloading.

  12. brettcannon
    changed the title [-]pull some import state out of the interpreter state[/-] [+]pull some import state out of the interpreter state[/+] on Apr 19, 2012

  13. ericsnowcurrently commented on Apr 19, 2012

    @ericsnowcurrently
    ericsnowcurrently
    on Apr 19, 2012
    MemberAuthor

    Thanks for your feedback, Martin. I'll go ahead and make a new issue for modules_reloading (and one for interp->modules when appropriate).

  14. ericsnowcurrently
    closed this as completedon Apr 19, 2012

  15. ericsnowcurrently commented on Apr 19, 2012

    @ericsnowcurrently
    ericsnowcurrently
    on Apr 19, 2012
    MemberAuthor

    for modules_reloading, bpo-14618

  16. 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

    interpreter-core(Objects, Python, Grammar, and Parser dirs)

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions