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 670: Convert static inline functions to functions #97016

Closed
vstinner opened this issue Sep 22, 2022 · 3 comments
Closed

PEP 670: Convert static inline functions to functions #97016

vstinner opened this issue Sep 22, 2022 · 3 comments
Labels
type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

PEP 670 converted macros to static inline functions in Python 3.11. I propose to implement the second phase of PEP 670: convert static inline functions to regular functions in Python 3.12.

Static inline functions are causing C++ warnings. I had to add _Py_CAST() and _Py_NULL macros to Python 3.11 to limit the number of C++ warnings. The _Py_CAST() macro has a complicated history. @serge-sans-paille helped me to write a correct C++ implementation of _Py_CAST(), but it had to be reverted to fix the issue #94731. See:

As explained in PEP 670, static inline functions cannot be used in programming languages other than C, and these functions cannot be loaded by loading the symbol from the Python library (libpython). For example, the vim text editor loads uses the Python C API by loading symbols.

Converting static inline functions also hides implementation details. When using regular function calls, at the ABI level, it's just a regular function call, the machine code doesn't access directly structure members. So we have more freedom to changes Python internals (like the structure).

This issue is not about changing Python internals. It's just about convert static inline functions to regular functions.

@vstinner
Copy link
Member Author

Converting PyBytes_AS_STRING() and PyBytes_GET_SIZE() static inline functions to regular functions is a prerequirement to hide PyBytesObject members from the public C API.

Exposing members of this structure causes two issues:

Using regular function calls rather than accessing directly structure member avoids these issues.

For the flexible array, we should replace the last member with char ob_sval[0]; (C99) or char ob_sval[]; (GNU extension?). It can cause issues on projects using sizeof(PyBytesObject) and making assumption on the memory layout (that the structure always contains 1 item as part of the structure size).

@vstinner
Copy link
Member Author

Converting PyBytes_AS_STRING() and PyBytes_GET_SIZE() static inline functions to regular functions makes them in average 1.25x slower: +2.7 ns per function call, in the worst case, according to a micro-benchmark: #97017 (comment)

In Python core, PyBytes_AS_STRING() and PyBytes_GET_SIZE() function calls should still be inlined thanks to LTO. But in C extensions built as shared libraries (ex: array, _asyncio, _csv, _json, _struct, _datetime, _codecs_jp, _socket, etc.), they become (slower) regular function calls.

If performance is an issue in some shared libraries, we can provide static inline functions in the internal C API and use explicitly this faster flavor.

@vstinner
Copy link
Member Author

vstinner commented Nov 3, 2022

It seems like this change is controversial, so I close the issue. See: #97017 (comment)

@vstinner vstinner closed this as completed Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

1 participant