Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PEP 683 (Immortal Objects): Implement Py_INCREF() as function call in limited C API 3.12 #105387

Closed
vstinner opened this issue Jun 6, 2023 · 8 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@vstinner
Copy link
Member

vstinner commented Jun 6, 2023

The implementation of Py_INCREF() and Py_DECREF() functions changed in Python 3.12 to implement PEP 683 (Immortal Objects). Stable ABI sections of PEP 683 says:

The implementation approach described in this PEP is compatible with extensions compiled to the stable ABI (with the exception of Accidental Immortality and Accidental De-Immortalizing). Due to the nature of the stable ABI, unfortunately, such extensions use versions of Py_INCREF(), etc. that directly modify the object’s ob_refcnt field. This will invalidate all the performance benefits of immortal objects.

I propose to change the limited C API implementation to use an opaque function call in the limited C API 3.12 and newer. It hides tricky implementation details: see issue #105059 and PR #105275 for a concrete example. It will also ease the hypothetical implementation of PEP 703 "NO GIL".

Converting Py_INCREF() and Py_DECREF() static inline functions to opaque function calls has an impact on performance. IMO the change is worth it. I didn't measure the impact on performance.

In Python 3.10 and 3.11, I modified the Python C API to abstract access to PyObject and PyVarObject with function calls, rather than accessing directly structure members: see issue #83754. Currently, it's not enforced, it's still possible to access directly structure members. This step is another step towards fully opaque PyObject and PyVarObject structure (remove their members from the public API).

Linked PRs

@vstinner vstinner added the type-bug An unexpected behavior, bug, or error label Jun 6, 2023
@vstinner vstinner changed the title PEP 683 (Immortal Objects): Implement Py_INCREF() and Py_DECREF() on limited C API 3.12 and newer PEP 683 (Immortal Objects): Implement Py_INCREF() as function call in limited C API 3.12 Jun 6, 2023
vstinner added a commit to vstinner/cpython that referenced this issue Jun 6, 2023
In the limited C API version 3.12 and newer, Py_INCREF() and
Py_DECREF() functions are now implemented as opaque function calls in
the stable ABI to hide implementation details.
@vstinner
Copy link
Member Author

vstinner commented Jun 7, 2023

See also capi-workgroup/problems#45

@gvanrossum
Copy link
Member

But isn't it too late to do this in 3.12? We've already released two betas. I think this would require a release manager call.

vstinner added a commit to vstinner/cpython that referenced this issue Jun 9, 2023
In the limited C API version 3.12, Py_INCREF() and Py_DECREF()
functions are now implemented as opaque function calls to hide
implementation details.
@vstinner
Copy link
Member Author

vstinner commented Jun 9, 2023

Code built with beta1 and beta2 for the stable ABI access directly PyObject.ob_refnct, but Py_INCREF/DECREF implementation may change before Python 3.12 final: see PR #105059 discussion.

But isn't it too late to do this in 3.12? We've already released two betas. I think this would require a release manager call.

@Yhg1s: What's your call on this change (if it's accepted)? Are you ok to change the limited C API version 3.12?

@vstinner
Copy link
Member Author

vstinner commented Jun 9, 2023

@eduardo-elizondo
Copy link
Contributor

I wrote this in the GH-105059 thread - but should we treat this issue independently of PEP683? As is, the PEP683 change should be backwards compatible. To be clear, yes, we are indeed changing the semantics and yes, extensions have the ability to invalidate the immortal state but it doesn't mean that it will cause correctness issues. I personally don't see this as a motivator to completely make these functions opaque (but curious to hear other people's thoughts).

On the other hand though, I do see a lot of value of making incref and decref opaque, but I do think that this has to be evaluated on its own to properly weigh the pros and cons. This implies is that it doesn't necessarily need to be part of 3.12 and this can be pushed to 3.13.

@vstinner
Copy link
Member Author

PEP 683 made Py_INCREF() and Py_DECREF() more complicated, changed PyObject, and there are currently C compiler warnings/errors related to these changes. For the limited C API, in general, it's bad to leak so many implementation details. It will bite us soon or later. The less implementation details are exposed, the easier it is for us to change the implementation without having to worry about breaking some random C extensions for various reasons.

It's not about correctness, it's more about avoiding to rely on some C semantics of the exact Py_INCREF/Py_DECREF implementation: we better control how CPython is built (C compiler, version, flags) than how C extensions are built. Some "C" extensions are built in C++, in Rust, or whatever else. (Well, Rust is unlikely to reuse our inlined versions, but rewrite their own code in Rust.)

PEP 683 is not the only motivation here, but it's a good motivation to switch to opaque function calls.

@vstinner
Copy link
Member Author

I created a discussion: https://discuss.python.org/t/limited-c-api-implement-py-incref-and-py-decref-as-function-calls/27592

vstinner added a commit that referenced this issue Jun 14, 2023
In the limited C API version 3.12, Py_INCREF() and Py_DECREF()
functions are now implemented as opaque function calls to hide
implementation details.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 14, 2023
…GH-105388)

In the limited C API version 3.12, Py_INCREF() and Py_DECREF()
functions are now implemented as opaque function calls to hide
implementation details.
(cherry picked from commit b542972)

Co-authored-by: Victor Stinner <[email protected]>
vstinner added a commit that referenced this issue Jun 14, 2023
…5388) (#105763)

gh-105387: Limited C API implements Py_INCREF() as func (GH-105388)

In the limited C API version 3.12, Py_INCREF() and Py_DECREF()
functions are now implemented as opaque function calls to hide
implementation details.
(cherry picked from commit b542972)

Co-authored-by: Victor Stinner <[email protected]>
@vstinner
Copy link
Member Author

I merged my change into main and 3.12. It got approved by @gpshead.

I wanted to merge it soon to make the limited C API even better in Python 3.12 for PEP 683. It may help fixing the tricky issue #105059 which is more complicated and severe (impact non-limited C API users).

If this change is causing too much troubles, we can revisit its design or even revert it before Python 3.12 final.

But again, it only impacts users of the new limited C API version 3.12 and newer. The limited C API version 3.11 and older is left unchanged by my change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants