gh-76785: Improved Subinterpreters Compatibility with 3.12 #115424
Conversation
ericsnowcurrently
force-pushed
the
compat-3.12-fixes
branch
from
912e5d3 to
c906ee3
Compare
ericsnowcurrently
force-pushed
the
compat-3.12-fixes
branch
from
c906ee3 to
455460c
Compare
…hongh-115424) For the most part, these changes make is substantially easier to backport subinterpreter-related code to 3.12, especially the related modules (e.g. _xxsubinterpreters). The main motivation is to support releasing a PyPI package with the 3.13 capabilities compiled for 3.12. A lot of the changes here involve either hiding details behind macros/functions or splitting up some files.
| // This helps with backporting certain changes to 3.12. | ||
| #define _PyCode_HAS_EXECUTORS(CODE) \ | ||
| (CODE->co_executors != NULL) | ||
| #define _PyCode_HAS_INSTRUMENTATION(CODE) \ |
There was a problem hiding this comment.
Apologies for not noticing this at the time.
This is incorrect. The _co_instrumentation_version being greater than zero does not indicate whether a code object is instrumented.
It may suggest that it was instrumented at some point, although that may not remain true.
The version number has no meaning at all, expect to the instrumentation system, and shouldn't be used to infer properties of the code object.
There was a problem hiding this comment.
FWIW, this change only moved an existing hard-coded expression to a macro. I have no problems with any of it getting refactored to be more correct or better named. My motivation here is strictly to support building the 3.12 backport of my PyPI package. Replacing a macro is a lot easier than directly replacing a line of code in a file.
There was a problem hiding this comment.
Maybe so, but it is still incorrect.
It was added originally here: 92ca90b#diff-f29800af0b7052514f5cc3d1a5858d704a8f0dee4c88788b741c00a0ff39f8d0R402
What are you guarding against, and why?
There was a problem hiding this comment.
In that spot we are rejecting (for now) any code object that isn't completely basic. That includes ones that might have instrumentation.
There was a problem hiding this comment.
Instrumentation is not a feature of the code object any more than specialization. It is how the interpreter does monitoring. If you marshal or copy a code object, the instrumentation is stripped.
In other words, checking for instrumentation is unnecessary here (or anyway else, in theory).
Even if it were necessary, _co_instrumentation_version does not indicate whether a code object is instrumented or not.
We would need a proper API to detect instrumentation (which would probably need to scan the code)
_PyCode_HAS_INSTRUMENTATION is misnamed and an attractive nuisance. Can we please remove it?
There was a problem hiding this comment.
The same reasoning applies to executors, and _co_monitoring.
We can ignore _co_extras as well, as _PyInterpreterState_SetEvalFrameFunc is per-interpreter now.
For the most part, these changes make is substantially easier to backport subinterpreter-related code to 3.12, especially the related modules (e.g. _xxsubinterpreters). The main motivation is to support releasing a PyPI package with the 3.13 capabilities compiled for 3.12.
A lot of the changes here involve either hiding details behind macros/functions or splitting up some files.