classification
Title: [C API] Support the limited C API in debug mode (Py_INCREF and Py_DECREF)
Type: Stage: resolved
Components: C API Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: petr.viktorin, vstinner
Priority: normal Keywords: patch

Created on 2021-04-01 06:47 by vstinner, last changed 2021-05-26 22:20 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
bench_limited.py vstinner, 2021-04-01 17:04
bench_testcapi.py vstinner, 2021-04-01 17:04
bench.patch vstinner, 2021-04-01 17:04
Pull Requests
URL Status Linked Edit
PR 25131 merged vstinner, 2021-04-01 07:42
PR 25133 merged vstinner, 2021-04-01 08:10
PR 25134 merged vstinner, 2021-04-01 08:26
PR 25135 merged vstinner, 2021-04-01 09:42
Messages (12)
msg389956 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-01 06:47
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 https://github.com/python/cpython/pull/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.
msg389960 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-01 08:05
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
Tests result: SUCCESS
msg389962 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-01 09:30
New changeset baf10da75072d1f8ec714d3c2c8550d34db343a9 by Victor Stinner in branch 'master':
bpo-43688: Run make regen-limited-abi (GH-25134)
https://github.com/python/cpython/commit/baf10da75072d1f8ec714d3c2c8550d34db343a9
msg389971 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-04-01 11:17
If you do this, please check the performance impact. Py_INCREF/Py_DECREF are very common.
msg389982 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-01 13:09
New changeset 2ac0515027699b5694d9a6ff40f1ddaba82c74c2 by Victor Stinner in branch 'master':
bpo-43688: Fix Py_LIMITED_API version of xxlimited (GH-25135)
https://github.com/python/cpython/commit/2ac0515027699b5694d9a6ff40f1ddaba82c74c2
msg389983 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-01 13:47
> New changeset baf10da75072d1f8ec714d3c2c8550d34db343a9 by Victor Stinner in branch 'master':
> bpo-43688: Run make regen-limited-abi (GH-25134)

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.
msg390002 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-01 17:04
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
./configure --with-lto --enable-optimizations
make
./python -m venv env
./env/bin/python -m pip install pyperf
./env/bin/python ../bench_limited.py -o ../limited.json -v
./env/bin/python ../bench_testcapi.py -o ../ref.json -v
msg390057 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-02 12:46
I modified my PR 25131 to only implement Py_INCREF/Py_DECREF as opaque function calls under two conditions:

* Python is built in debug mode
* Py_LIMITED_API macro targets Python 3.10 or newer

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.
msg390062 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-02 13:42
In release mode, I get the libraries:

* xxlimited.cpython-310-x86_64-linux-gnu.so
* xxlimited_35.cpython-310-x86_64-linux-gnu.so

In debug mode, I get the libraries:

* xxlimited.cpython-310d-x86_64-linux-gnu.so
* xxlimited_35.cpython-310d-x86_64-linux-gnu.so

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.
msg390063 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-02 13:45
New changeset 3359cab038968935b40344fad7c30d211f9692e4 by Victor Stinner in branch 'master':
bpo-43688: Support the limited C API in debug mode (GH-25131)
https://github.com/python/cpython/commit/3359cab038968935b40344fad7c30d211f9692e4
msg390067 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-02 14:46
New changeset 9bb5658bd122d40fff9f34a912be3297b303d18b by Victor Stinner in branch 'master':
bpo-43688: Support "make regen-limited-abi" in debug mode (GH-25133)
https://github.com/python/cpython/commit/9bb5658bd122d40fff9f34a912be3297b303d18b
msg394478 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-05-26 22:20
Thanks for the help Petr. I close the issue, it's now fixed!
History
Date User Action Args
2021-05-26 22:20:07vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg394478

stage: patch review -> resolved
2021-04-02 14:46:16vstinnersetmessages: + msg390067
2021-04-02 13:45:45vstinnersetmessages: + msg390063
2021-04-02 13:42:09vstinnersetmessages: + msg390062
2021-04-02 12:46:22vstinnersetmessages: + msg390057
2021-04-01 17:04:32vstinnersetfiles: + bench.patch
2021-04-01 17:04:27vstinnersetfiles: + bench_testcapi.py
2021-04-01 17:04:22vstinnersetfiles: + bench_limited.py
2021-04-01 17:04:01vstinnersetmessages: + msg390002
2021-04-01 13:47:06vstinnersetmessages: + msg389983
2021-04-01 13:09:49vstinnersetmessages: + msg389982
2021-04-01 11:17:05petr.viktorinsetmessages: + msg389971
2021-04-01 09:42:59vstinnersetpull_requests: + pull_request23882
2021-04-01 09:30:06vstinnersetmessages: + msg389962
2021-04-01 08:26:29vstinnersetpull_requests: + pull_request23881
2021-04-01 08:10:28vstinnersetpull_requests: + pull_request23880
2021-04-01 08:05:16vstinnersetmessages: + msg389960
2021-04-01 07:56:42vstinnersetnosy: + petr.viktorin
2021-04-01 07:42:07vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request23877
2021-04-01 07:41:40vstinnersettitle: [C API] Support the limited C API in debug build (Py_INCREF and Py_DECREF) -> [C API] Support the limited C API in debug mode (Py_INCREF and Py_DECREF)
2021-04-01 07:25:47vstinnersettitle: [C API] Fix Py_INCREF and Py_DECREF in the limited C API for Python built in debug mode -> [C API] Support the limited C API in debug build (Py_INCREF and Py_DECREF)
2021-04-01 06:47:31vstinnercreate