Adopt multi-phase init (PEP 489)#2577
Conversation
|
There's errors like: |
|
@giampaolo sorry! copy and paste error. I've pushed a commit to fix, please could you approve CI? A |
Since this is all repeated code, I wonder if it makes sense to somehow define it in |
psutil/_psutil_posix.c
Outdated
| #ifdef Py_mod_multiple_interpreters // Python 3.12+ | ||
| {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED}, |
There was a problem hiding this comment.
This pattern doesn't work well with stable ABI extensions. For example, if you compile psutil for cp36+ abi3, it won't have this slot set even when loaded into Python 3.12+.
https://pypi.org/project/psutil/#files
If you want it to work with the stable ABI, I think you need to hard code the constant and dynamically set the slot at runtime (i.e., in PyInit__psutil_posix).
The same would be true for Py_mod_gil, but the free threaded build doesn't currently work with the stable ABI, so it's not currently relevant.
There was a problem hiding this comment.
@colesbury Indeed, I've thinking about this but haven't come up with any good solutions. The 'simplest' is for psutil to additionally distribute a wheel built with the limited API set to 3.12+, which would double the number of wheels in the medium term, but still be O(1).
One potential solution would be to define two slots arrays, slots_py36 and slots_py312. We could then choose between them based on the runtime version (though Py_Version was only added in 3.11, so this might have to go via sys.hexversion). This would still need to hard-code the symbols that don't exist in Python 3.6, but avoids dynamic manipulation of the arrays.
cc @encukou for any advice you may have here!
A
There was a problem hiding this comment.
I suggest not adding new functionality in this PR. Keep the PyUnstable_Module_SetGIL call, and remove the new slots for now.
Allowing per-interpreter GIL should be its own PR and discussion. (And preparing a good proposal for psutil devs might not be trivial.)
Alas, limited API does limit you to a minimum Python version. It is not designed to dynamically allow newer features.
We can change that, but this is not the place to do it.
There was a problem hiding this comment.
Thanks to Petr for the advice, I've removed the Py_mod_gil and Py_mod_multiple_interpreters slots from this PR and added back the PyUnstable_Module_SetGIL() call. @giampaolo please could you run CI?
A
There was a problem hiding this comment.
@giampaolo please could you run CI?
It looks like there are failures on Windows.
There was a problem hiding this comment.
@AA-Turner, the suggestions should fix those.
| }; | ||
| static int module_loaded = 0; | ||
|
|
||
| static int module_loaded = 0; |
There was a problem hiding this comment.
| static int module_loaded = 0; |
| .m_base = PyModuleDef_HEAD_INIT, | ||
| .m_name = "_psutil_windows", | ||
| .m_size = sizeof(struct module_state), | ||
| .m_methods = mod_methods, |
There was a problem hiding this comment.
| .m_methods = mod_methods, | |
| .m_methods = PsutilMethods, |
|
For the record, all the .c files recently changed due to the adoption of clang-format, so a rebase is required. |
cc @giampaolo
xref #2576
The mechanical changes for each module are:
PyMODINIT_FUNC PyInit_{name}(void)tostatic int {name}_exec(PyObject *mod)return NULLto-1andreturn modto0PyModule_CreateandPyUnstable_Module_SetGIL.m_slotsmemberPyMODINIT_FUNC PyInit_{name}(void)function, now just with thePyModuleDef_Init()callA