| msg158638 - (view) |
Author: Eric Snow (eric.snow) *  |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2012-04-19 01:33 |
| for modules_reloading, |
|
|