This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: pull some import state out of the interpreter state
Type: Stage:
Components: Interpreter Core Versions: Python 3.3
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: brett.cannon, eric.snow, loewis
Priority: normal Keywords:

Created on 2012-04-18 16:32 by eric.snow, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Messages (11)
msg158638 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-04-18 16:32
Once the dust clears from the issue 2377 and issue 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.
msg158640 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-18 16:48
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).
msg158643 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-04-18 17:54
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;
}
msg158647 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-18 18:43
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?
msg158652 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-04-18 19:32
> 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.
msg158659 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-04-18 20:42
Sorry, Martin, for not looking at PEP 3121 before.  I was thinking modules_by_index was a lot older.
msg158666 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-04-18 21:36
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 issue 12633), but it's easy to deal with.
msg158671 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-04-18 22:07
> 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.
msg158690 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-04-19 01:06
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.
msg158692 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-04-19 01:33
Thanks for your feedback, Martin.  I'll go ahead and make a new issue for modules_reloading (and one for interp->modules when appropriate).
msg158693 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-04-19 01:33
for modules_reloading, issue14618
History
Date User Action Args
2022-04-11 14:57:29adminsetgithub: 58820
2012-04-19 01:33:29eric.snowsetmessages: + msg158693
2012-04-19 01:33:05eric.snowsetstatus: open -> closed
resolution: rejected
messages: + msg158692
2012-04-19 01:06:51brett.cannonsetmessages: + msg158690
title: pull some import state out of the interpreter state -> pull some import state out of the interpreter state
2012-04-18 22:07:08loewissetmessages: + msg158671
2012-04-18 21:36:21eric.snowsetmessages: + msg158666
2012-04-18 20:42:28eric.snowsetmessages: + msg158659
2012-04-18 19:32:28loewissetmessages: + msg158652
title: pull some import state out of the interpreter state -> pull some import state out of the interpreter state
2012-04-18 18:55:09pitrousetnosy: + loewis
2012-04-18 18:43:06brett.cannonsetmessages: + msg158647
2012-04-18 17:54:17eric.snowsetmessages: + msg158643
2012-04-18 16:48:00brett.cannonsetmessages: + msg158640
2012-04-18 16:32:41eric.snowcreate