Hi Victor,
I say no. Why the sudden urgency?
I think that you are making too many assumptions about how inter-interpreter communication is going to work, and about sharing of objects.
On 06/05/2020 6:49 pm, Victor Stinner wrote:
Hi,
tl; dr I'm asking for your permission to merge the following PR :-)
https://github.com/python/cpython/pull/19958
In bpo-40514, I added a new --with-experimental-isolated-subinterpreters configuration option. I chose to use a very long option name and to not document it on purpose: prevent users to use it "by mistake", without understanding its purpose. The option is related to "per-interpreter GIL":
https://bugs.python.org/issue40512
It's a practical solution to be able to experiment quickly per-interpreter GIL without having to fix all issues at once. For example, it disables Unicode interned strings which is unsafe with multiple interpreters running in parallel.
If the interning is done on a per-interpreter basis, then it is safe.
I added this #ifdef to encourage other core developers work on this project, and let early adopters test this experimental feature to give us their feedback.
You say this is to let other core developers work on this, but has anyone actually asked for these changes?
I hope that soon we will discover all places which need to be fixed, so it will help to better estimate how much work is needed to finish the implementation of per-interpreter GIL.
Currently, the special build changes:
- Per-interpreter GIL
- Store the current Python thread state in a TLS
- Disable dict, frame, tuple and list free list
- Disable type method cache
- Disable pymalloc: force usage of libc malloc
- Disable the GC in subinterpreters
- _xxsubinterpreters.run_string() releases the GIL
These changes are going to have such a large impact on robustness and performance as to make any comparisons meaningless.
I consider that's a reasonable small number of changes to get a cool feature (per-interpreter GIL), compared to the same of the CPython code base (637K lines).
I'm not concerned about the size of the change. What concerns me is that it is spread all over the code and introduces assumptions that are not made explicit.
--
All these "#ifdef EXPERIMENTAL_ISOLATED_SUBINTERPRETERS" are temporary workarounds until a proper fix is designed. For example, some caches should be made "per-interpreter".
EXPERIMENTAL_ISOLATED_SUBINTERPRETERS seems to have already appeared in many places. AFIACT, the changes adding it represent a change of over 250 lines without any review.
Most changes are easy to write, but some other changes are non trivial. For example, I modified _PyThreadState_GET() and _PyThreadState_Swap() to use a Thread Local Storage (TLS) to get and set the current Python thread state.
Does that mean you are going to remove all the PyThreadState *tstate
parameters that have been added lately?
Currently, "#ifdef EXPERIMENTAL_ISOLATED_SUBINTERPRETERS" are not "visible" to users: it is only used in .c files and a few internal header files. But the following PR modify the public Include/object.h header file to make PyObject.ob_refcnt atomic:
https://github.com/python/cpython/pull/19958
I would like to ensure that you are ok to put a few more temporary "#ifdef EXPERIMENTAL_ISOLATED_SUBINTERPRETERS" in CPython to speedup the development of subinterpreters running in parallel (per-interpreter GIL), or if you consider that it has gone too far and a Git fork would be better.
A fork would be much better.
Rather than sprinking #ifdefs everywhere, could you continue consolidating all "global" objects into a single data structure? Once that has been done, experimentation becomes much easier and the changes more localized.
Cheers, Mark.
Victor