-
-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
[C API] Support the limited C API in debug mode (Py_INCREF and Py_DECREF) #87854
Comments
Currently, setup.py doesn't build xxlimited and xxlimited_35 extension modules with the limited C API if Python is built in debug mode. I only found two functions affected by Py_DEBUG macro in the limited C API: Py_INCREF() and Py_DECREF(). Example: #if defined(Py_DEBUG) && !defined(Py_REF_DEBUG)
#define Py_REF_DEBUG
#endif
static inline void _Py_INCREF(PyObject *op)
{
#ifdef Py_REF_DEBUG
_Py_RefTotal++;
#endif
op->ob_refcnt++;
}
#define Py_INCREF(op) _Py_INCREF(_PyObject_CAST(op)) If Py_DEBUG is defined (Python built in debug mode), Py_INCREF() increments the private _Py_RefTotal variable. The limited C API is supposed to provide a stable ABI, but Py_INCREF() leaks _Py_RefTotal implementation if Python is built in debug mode. I propose to modify Py_INCREF() and Py_DECREF() of the limited C API to always declare them as opaque function calls. The regular (non limited) C API will continue to use the same static inline functions. See also #25115 and bpo-41111 "[C API] Convert a few stdlib extensions to the limited C API (PEP-384)". Note: Py_XINCREF() and Py_XDECREF() should be fine. |
One nice advantage of this change is that it becomes possible to check for reference leaks in C extensions written with the limited C API! $ ./python -m test test_xxlimited -R 3:3
0:00:00 load avg: 2.99 Run tests sequentially
0:00:00 load avg: 2.99 [1/1] test_xxlimited
beginning 6 repetitions
123456
...... == Tests result: SUCCESS == 1 test OK. Total duration: 367 ms |
If you do this, please check the performance impact. Py_INCREF/Py_DECREF are very common. |
With this change, "Tests / Check if generated files are up to date" job started fails (on PR 25135): "Some symbols from the limited API are missing: PyType_HasFeature". I fixed this issue in bpo-43690 which removes PyType_HasFeature from Doc/data/stable_abi.dat. |
I wrote a microbenchmark on Py_INCREF()+Py_DECREF(): $ python3 -m pyperf compare_to ref.json limited.json
Mean +- std dev: [ref] 3.45 ns +- 0.17 ns -> [limited] 6.03 ns +- 0.21 ns: 1.75x slower If a function is only made of Py_INCREF() and Py_DECREF(), it can be up to 1.8x slower in the worst case. But in practice, I don't think that functions are only made of Py_INCREF() and Py_DECREF(). They do a few other things. I'm not sure how to run a "macro benchmark" on my PR 25131, since I don't know any C extension doing anything useful. There is xxlimited, but it does almost nothing. What would be a fair benchmark for this change? -- To run my microbenchmark: git apply bench.patch |
I modified my PR 25131 to only implement Py_INCREF/Py_DECREF as opaque function calls under two conditions:
So this PR 25131 now only has an impact on performance if Python is built in debug mode. Performance on Python built in release mode is not affected. |
In release mode, I get the libraries:
In debug mode, I get the libraries:
It's still a different ABI: "cpython-310" vs "cpython-310d" ("D" for debug). So there is risk to be confused between the stable ABI in release mode and the stable ABI in debug mode :-) I don't how how to get the ".abi3.so" suffix. |
Thanks for the help Petr. I close the issue, it's now fixed! |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: