PEP 803: Position abi3t as a new variant; use Py_TARGET_ABI3T to select it#4747
PEP 803: Position abi3t as a new variant; use Py_TARGET_ABI3T to select it#4747encukou wants to merge 5 commits intopython:mainfrom
Conversation
Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
|
Thank you! |
|
From more discussion, a new knob called |
| compiled extension to support both at once, and thus be compatible with | ||
| CPython 3.15+ (both free-threaded and GIL-eanbled builds). | ||
| Initially, any extension compiled for ``abi3t`` will be compatible with | ||
| ``abi3`` as well. |
There was a problem hiding this comment.
I'm a little concerned that abi3t as you're proposing it doesn't include PyCriticalSection and maybe PyMutex.
See e.g. my comment here - in practice the restriction that abi3t must be a subset of abi3 means that getting extensions that already support the free-threaded build working on abi3t will be very challenging. CFFI uses PyMutex and critical sections. NumPy uses critical sections and PyMutex in its implementation and many libraries that depend on NumPy use critical sections via Cython. PyO3 exposes wrappers for PyMutex but uses native locks for the most part in its implementation. It does use critical sections in several places in its implementation.
All that to say - I really think an ABI that targets the free-threaded build needs at least PyCriticalSection. I think PyMutex can be replaced with native locks for the most part, but having PyMutex is often convenient.
What I think might make sense is to propose two new ABIs. What you're currently proposing could be called abi3o where o is for opaque. You can build abi3o extensions to support both the free-threaded build and the GIL-enabled build on 3.15 and newer, but you can't rely on the GIL or critical sections for thread safety. There are extensions like this, and if there was a target for that maybe people could update bindings generators to target it by adopting different locking strategies.
But, to get to a place where e.g. cryptography can ship O(1) wheels for Python 3.17, I think we do need an abi3t that looks more like the current free-threaded ABI. This could include an opauque PyObject too.
Another way we can fix this is to add PyMutex, PyCriticalSection, and PyCriticalSection2 to to the stable ABI. I guess you'd have to make operations that use PyCriticalSection no-ops on the GIL-enabled build since there's no per-object locks? But then it becomes much more straightforward to write extensions targeting both builds.
There was a problem hiding this comment.
I just double-checked and at least for CFFI and NumPy it’s not a big deal for project that depend on them. The critical sections are all in internals.
That said, PyO3 doesn’t have as clear a separation between internals and generated extensions, so it is a problem there for extensions built with PyO3 on the free-threaded build.
There was a problem hiding this comment.
Yeah. This PEP is at a lower level currently.
I fear that adding the PyCriticalSection discussions to this would send it spinning for another month or two. I'd rather keep them out of scope here.
Another way we can fix this is to add
PyMutex,PyCriticalSection, andPyCriticalSection2to to the stable ABI. I guess you'd have to make operations that usePyCriticalSectionno-ops on the GIL-enabled build since there's no per-object locks? But then it becomes much more straightforward to write extensions targeting both builds.
That's the plan. No need to complicate things for build tools and such.
There was a problem hiding this comment.
OK, fair enough. I hope the case for PyO3 will help convincing everyone it's worth doing this.
I can probably find more examples of load-bearing critical sections if it would help that discussion.
ngoldbaum
left a comment
There was a problem hiding this comment.
I left a few more comments after my main one.
For what it's worth, I have a branch of PyO3 that compiles with _Py_OPAQUE_PYOBJECT set on the GIL-enabled build. If I also define Py_GIL_DISABLED, then I hit issues in PyO3 internals because PyCriticalSection isn't exposed. I could stub out the critical section wrappers like they're currently stubbed on the GIL-enabled build, but then the resulting extensions won't be safe on the free-threaded build.
The tests run, but I get a segfault early on that I'm still debugging - it may be a CPython bug but it may also be a bug in how I set up _Py_OPAQUE_PYOBJECT support in PyO3. I'm meeting with David Hewitt tomorrow morning to figure that out.
|
|
||
| * ``Py_LIMITED_API=<version>`` (existing): | ||
| Compile for ``abi3`` of the given version. | ||
| * ``Py_TARGET_ABI3T=<version>`` (proposed here): |
| These two macros are functionally very similar. | ||
| In hindsight, ``Py_TARGET_ABI3`` (without the ``T``) would be a more fitting | ||
| name for :c:macro:`Py_LIMITED_API`. | ||
| We keep the existing name for backwards compatibility. |
There was a problem hiding this comment.
You could also introduce Py_TARGET_ABI3 as an alias and make the headers error out if you define both. IMO that would be nice for the symmetry. But I guess "there should preferably only be one way to do it"...
This isn't a big deal though and this spelling is fine.
There was a problem hiding this comment.
We could error out if Py_TARGET_ABI3 is defined, and suggest the proper spelling.
I don't think the PEP needs to specify that though.
| - :c:func:`Py_SIZE` | ||
| - :c:func:`Py_SET_SIZE` | ||
| - :c:func:`Py_SIZE` | ||
| - :c:func:`Py_SET_SIZE` |
There was a problem hiding this comment.
Yeah, for completeness -- although the static inline definition Py_TYPE(o) == t is fine, since Py_TYPE is a function.
| - :c:func:`Py_SET_SIZE` | |
| - :c:func:`Py_SET_SIZE` | |
| - :c:func:`Py_IS_TYPE` |
| similar -- will be added via the C API working group, or in a follow-up PEP. | ||
| Limited API to allow thread-safety without a GIL -- presumably ``PyMutex``, | ||
| ``PyCriticalSection``, and similar -- will be added via the C API working group, | ||
| or in a follow-up PEP. |
There was a problem hiding this comment.
See my comment above about how I think this really needs to be resolved now, otherwise I think it will be difficult to get extensions working in-practice.
There was a problem hiding this comment.
Now, yes. But I'd much prefer a separate discussion.
|
I plan to merge this early next week and update the discussion. |
PEP 123: Summary of changes)📚 Documentation preview 📚: https://pep-previews--4747.org.readthedocs.build/