General concerns about C API changes
Overall, I support the efforts to improve the C API, but over the last few weeks have become worried. I don't want to hold up progress with fear, uncertainty, and doubt. Yet, I would like to be more comfortable that we're all aware of what is occurring and what are the potential benefits and risks. * Inline functions are great. They provide true local variables, better separation of concerns, are far less kludgy than text based macro substitution, and will typically generate the same code as the equivalent macro. This is good tech when used with in a single source file where it has predictable results. However, I'm not at all confident about moving these into header files which are included in multiple target .c files which need be compiled into separate .o files and linked to other existing libraries. With a macro, I know for sure that the substitution is taking place. This happens at all levels of optimization and in a debug mode. The effects are 100% predictable and have a well-established track record in our mature battle-tested code base. With cross module function calls, I'm less confident about what is happening, partly because compilers are free to ignore inline directives and partly because the semantics of inlining are less clear when the crossing module boundaries. * Other categories of changes that we make tend to have only a shallow reach. However, these C API changes will likely touch every C extension that has ever been written, some of which is highly tuned but not actively re-examined. If any mistakes are make, they will likely be pervasive. Accordingly, caution is warranted. My expectation was that the changes would be conducted in experimental branches. But extensive changes are already being made (or about to be made) on the 3.8 master. If a year from now, we decide that the changes were destabilizing or that the promised benefits didn't materialize, they will be difficult to undo because there are so many of them and because they will be interleaved with other changes. The original motivation was to achieve a 2x speedup in return for significantly churning the C API. However, the current rearranging of the include files and macro-to-inline-function changes only give us churn. At the very best, they will be performance neutral. At worst, formerly cheap macro calls will become expensive in places that we haven't thought to run timings on. Given that compilers don't have to honor an inline directive, we can't really know for sure -- perhaps today it works out fine, and perhaps tomorrow the compilers opt for a different behavior. Maybe everything that is going on is fine. Maybe it's not. I am not expert enough to know for sure, but we should be careful before green-lighting such an extensive series of changes directly to master. Reasonable questions to ask are: 1) What are the risks to third party modules, 2) Do we really know that the macro-to-inline-function transformations are semantically neutral. 3) If there is no performance benefit (none has been seen so far, nor is any promised in the pending PRs), is it worth it? We do know that PyPy folks have had their share of issues with the C API, but I'm not sure that we can make any of this go away without changing the foundations of the whole ecosystem. It is inconvenient for a full GC environment to interact with the API for a reference counted environment -- I don't think we can make this challenge go away without giving up reference counting. It is inconvenient for a system that manifests objects on demand to interact with an API that assumes that objects have identity and never more once they are created -- I don't think we can make this go away either. It is inconvenient to a system that uses unboxed data to interact with our API where everything is an object that includes a type pointer and reference count -- We have provided an API for boxing and boxing, but the trip back-and-forth is inconveniently expensive -- I don't think we can make that go away either because too much of the ecosystem depends on that API. There are some things that can be mitigated such as challenges with borrowed references but that doesn't seem to have been the focus on any of the PRs. In short, I'm somewhat concerned about the extensive changes that are occurring. I do know they will touch substantially every C module in the entire ecosystem. I don't know whether they are safe or whether they will give any real benefit. FWIW, none of this is a criticism of the work being done. Someone needs to think deeply about the C API or else progress will never be made. That said, it is a high risk project with many PRs going directly into master, so it does warrant having buy in that the churn isn't destabilizing and will actually produce a benefit that is worth it. Raymond
To me, the "new C API" discussion and the "converting macros into inline functions" discussions are very different, almost unrelated. There are always lots of small C API changes happening, and AFAIK the macros->inline changes fall into that category. It sounds like you want to discuss whether inline functions are a good idea? Or are there other changes happening that you're worried about? Or is there some connection between inline functions and API breakage that I'm not aware of? Your email touches on a lot of different topics and I'm having trouble understanding how they fit together. (And I guess like most people here, I'm not watching every commit to the master branch so I may not even know what changes you're referring to.) On Tue, Nov 13, 2018 at 7:06 PM, Raymond Hettinger <raymond.hettinger@gmail.com> wrote:
Overall, I support the efforts to improve the C API, but over the last few weeks have become worried. I don't want to hold up progress with fear, uncertainty, and doubt. Yet, I would like to be more comfortable that we're all aware of what is occurring and what are the potential benefits and risks.
* Inline functions are great. They provide true local variables, better separation of concerns, are far less kludgy than text based macro substitution, and will typically generate the same code as the equivalent macro. This is good tech when used with in a single source file where it has predictable results.
However, I'm not at all confident about moving these into header files which are included in multiple target .c files which need be compiled into separate .o files and linked to other existing libraries.
With a macro, I know for sure that the substitution is taking place. This happens at all levels of optimization and in a debug mode. The effects are 100% predictable and have a well-established track record in our mature battle-tested code base. With cross module function calls, I'm less confident about what is happening, partly because compilers are free to ignore inline directives and partly because the semantics of inlining are less clear when the crossing module boundaries.
* Other categories of changes that we make tend to have only a shallow reach. However, these C API changes will likely touch every C extension that has ever been written, some of which is highly tuned but not actively re-examined. If any mistakes are make, they will likely be pervasive. Accordingly, caution is warranted.
My expectation was that the changes would be conducted in experimental branches. But extensive changes are already being made (or about to be made) on the 3.8 master. If a year from now, we decide that the changes were destabilizing or that the promised benefits didn't materialize, they will be difficult to undo because there are so many of them and because they will be interleaved with other changes.
The original motivation was to achieve a 2x speedup in return for significantly churning the C API. However, the current rearranging of the include files and macro-to-inline-function changes only give us churn. At the very best, they will be performance neutral. At worst, formerly cheap macro calls will become expensive in places that we haven't thought to run timings on. Given that compilers don't have to honor an inline directive, we can't really know for sure -- perhaps today it works out fine, and perhaps tomorrow the compilers opt for a different behavior.
Maybe everything that is going on is fine. Maybe it's not. I am not expert enough to know for sure, but we should be careful before green-lighting such an extensive series of changes directly to master. Reasonable questions to ask are: 1) What are the risks to third party modules, 2) Do we really know that the macro-to-inline-function transformations are semantically neutral. 3) If there is no performance benefit (none has been seen so far, nor is any promised in the pending PRs), is it worth it?
We do know that PyPy folks have had their share of issues with the C API, but I'm not sure that we can make any of this go away without changing the foundations of the whole ecosystem. It is inconvenient for a full GC environment to interact with the API for a reference counted environment -- I don't think we can make this challenge go away without giving up reference counting. It is inconvenient for a system that manifests objects on demand to interact with an API that assumes that objects have identity and never more once they are created -- I don't think we can make this go away either. It is inconvenient to a system that uses unboxed data to interact with our API where everything is an object that includes a type pointer and reference count -- We have provided an API for boxing and boxing, but the trip back-and-forth is inconveniently expensive -- I don't think we can make that go away either because too much of the ecosystem depends on that API. There are some things that ca n be mitigated such as challenges with borrowed references but that doesn't seem to have been the focus on any of the PRs.
In short, I'm somewhat concerned about the extensive changes that are occurring. I do know they will touch substantially every C module in the entire ecosystem. I don't know whether they are safe or whether they will give any real benefit.
FWIW, none of this is a criticism of the work being done. Someone needs to think deeply about the C API or else progress will never be made. That said, it is a high risk project with many PRs going directly into master, so it does warrant having buy in that the churn isn't destabilizing and will actually produce a benefit that is worth it.
Raymond
_______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/njs%40pobox.com
-- Nathaniel J. Smith -- https://vorpus.org
Hi, I made many different kinds of changes to the C API last weeks. None of them should impact the backward compatibility. If it's the case, the changes causing the backward compatibility should be reverted. The most controversial changes are the conversion of macros to static inline functions, but these changes have been discussed in length and approved by multiple core developers. (*) Move private internal API outside Include/: continue the work started in Python 3.7 to move it to Include/internal/ This work is almost complete. The remaining issue is the _PyObject_GC_TRACK() function. The main change is that an explicit #include "pycore_<header>.h" is now required in C code. https://bugs.python.org/issue35081 https://mail.python.org/pipermail/python-dev/2018-October/155587.html https://mail.python.org/pipermail/python-dev/2018-November/155688.html (*) Move "unstable" API outside Include/: move "#ifndef Py_LIMITED_API" code to a new subdirectory This work didn't start. It's still being discussed to see how we should do it. I chose to try to finish Include/internal/ first. https://bugs.python.org/issue35134 (*) Add a *new* C API which doesn't leak implementation details This work didn't start. It's still under discussion, I'm not sure that it's going to happen in the master branch in the short term. https://pythoncapi.readthedocs.io/ https://bugs.python.org/issue35206 https://mail.python.org/pipermail/python-dev/2018-November/155702.html (*) Convert macros to static inline functions I guess that Raymond is worried by the impact on performance of these changes. So far, the following macros have been converted to static inline functions: * PyObject_INIT(), PyObject_INIT_VAR() * _Py_NewReference(), _Py_ForgetReference() * Py_INCREF(), Py_DECREF() * Py_XINCREF(), Py_XDECREF() * _Py_Dealloc() First, I attempted for "force inlining" (ex: __attribute__((always_inline)) for GCC/Clang), but it has been decided to not do that. Please read the discussion on the issue for the rationale. https://bugs.python.org/issue35059 I modified the Visual Studio project to ask the compiler to respect "inline" *hint* when CPython is compiled in debug mode: "Set InlineFunctionExpansion to OnlyExplicitInline ("/Ob1" option) on all projects (in pyproject.props) in Debug mode on Win32 and x64 platforms to expand functions marked as inline." This change spotted a bug in _decimal ("Add missing EXTINLINE in mpdecimal.h"). Macros have many pitfalls, and the intent here is to prevent these pitfalls.: https://gcc.gnu.org/onlinedocs/cpp/Macro-Pitfalls.html _Py_Dealloc() example: #ifdef COUNT_ALLOCS #define _Py_INC_TPFREES(OP) dec_count(Py_TYPE(OP)) #define _Py_COUNT_ALLOCS_COMMA , #else #define _Py_INC_TPFREES(OP) #define _Py_COUNT_ALLOCS_COMMA #endif /* COUNT_ALLOCS */ #define _Py_Dealloc(op) ( \ _Py_INC_TPFREES(op) _Py_COUNT_ALLOCS_COMMA \ (*Py_TYPE(op)->tp_dealloc)((PyObject *)(op))) _Py_Dealloc() produced something like "dealloc()" or "dec_count(), dealloc()" depending on compiler options on Python 3.7. On Python 3.8, it's now a well defined function call which returns "void" (no result): "[static inline] void _Py_Dealloc(PyObject *);" Another example: #define Py_DECREF(op) \ do { \ PyObject *_py_decref_tmp = (PyObject *)(op); \ if (_Py_DEC_REFTOTAL _Py_REF_DEBUG_COMMA \ --(_py_decref_tmp)->ob_refcnt != 0) \ _Py_CHECK_REFCNT(_py_decref_tmp) \ else \ _Py_Dealloc(_py_decref_tmp); \ } while (0) This macro required a temporary "_py_decref_tmp" variable in Python 3.7, to cast the argument to PyObject*. It's gone in Python 3.8: static inline void _Py_DECREF(const char *filename, int lineno, PyObject *op) { _Py_DEC_REFTOTAL; if (--op->ob_refcnt != 0) { #ifdef Py_REF_DEBUG if (op->ob_refcnt < 0) { _Py_NegativeRefcount(filename, lineno, op); } #endif } else { _Py_Dealloc(op); } } #define Py_DECREF(op) _Py_DECREF(__FILE__, __LINE__, (PyObject *)(op)) The cast is now done in Py_DECREF() macro. The Py_DECREF() contract is that it accepts basically any pointer. I chose to not change that (always require the exact PyObject* type and nothing else), since it would create a compiler warning on basically every usage of Py_DECREF() for no real benefit. Victor
First, I attempted for "force inlining" (ex: __attribute__((always_inline)) for GCC/Clang), but it has been decided to not do that. Please read the discussion on the issue for the rationale.
It has been decided to use "static inline" syntax since it's the one required by the PEP 7, but I'm open to switch to "inline" (without static) or something else. Honestly, I don't understand exactly all consequences of the exact syntax, so I relied on other core devs who understand C99 that better than me :-) Join the discussion on the issue ;-) Victor
On Tue, Nov 13, 2018 at 7:06 PM Raymond Hettinger < raymond.hettinger@gmail.com> wrote:
Overall, I support the efforts to improve the C API, but over the last few weeks have become worried. I don't want to hold up progress with fear, uncertainty, and doubt. Yet, I would like to be more comfortable that we're all aware of what is occurring and what are the potential benefits and risks.
* Inline functions are great. They provide true local variables, better separation of concerns, are far less kludgy than text based macro substitution, and will typically generate the same code as the equivalent macro. This is good tech when used with in a single source file where it has predictable results.
However, I'm not at all confident about moving these into header files which are included in multiple target .c files which need be compiled into separate .o files and linked to other existing libraries.
With a macro, I know for sure that the substitution is taking place. This happens at all levels of optimization and in a debug mode. The effects are 100% predictable and have a well-established track record in our mature battle-tested code base. With cross module function calls, I'm less confident about what is happening, partly because compilers are free to ignore inline directives and partly because the semantics of inlining are less clear when the crossing module boundaries.
* Other categories of changes that we make tend to have only a shallow reach. However, these C API changes will likely touch every C extension that has ever been written, some of which is highly tuned but not actively re-examined. If any mistakes are make, they will likely be pervasive. Accordingly, caution is warranted.
My expectation was that the changes would be conducted in experimental branches. But extensive changes are already being made (or about to be made) on the 3.8 master. If a year from now, we decide that the changes were destabilizing or that the promised benefits didn't materialize, they will be difficult to undo because there are so many of them and because they will be interleaved with other changes.
The original motivation was to achieve a 2x speedup in return for significantly churning the C API. However, the current rearranging of the include files and macro-to-inline-function changes only give us churn. At the very best, they will be performance neutral. At worst, formerly cheap macro calls will become expensive in places that we haven't thought to run timings on. Given that compilers don't have to honor an inline directive, we can't really know for sure -- perhaps today it works out fine, and perhaps tomorrow the compilers opt for a different behavior.
Maybe everything that is going on is fine. Maybe it's not. I am not expert enough to know for sure, but we should be careful before green-lighting such an extensive series of changes directly to master. Reasonable questions to ask are: 1) What are the risks to third party modules, 2) Do we really know that the macro-to-inline-function transformations are semantically neutral. 3) If there is no performance benefit (none has been seen so far, nor is any promised in the pending PRs), is it worth it?
We do know that PyPy folks have had their share of issues with the C API, but I'm not sure that we can make any of this go away without changing the foundations of the whole ecosystem. It is inconvenient for a full GC environment to interact with the API for a reference counted environment -- I don't think we can make this challenge go away without giving up reference counting. It is inconvenient for a system that manifests objects on demand to interact with an API that assumes that objects have identity and never more once they are created -- I don't think we can make this go away either. It is inconvenient to a system that uses unboxed data to interact with our API where everything is an object that includes a type pointer and reference count -- We have provided an API for boxing and boxing, but the trip back-and-forth is inconveniently expensive -- I don't think we can make that go away either because too much of the ecosystem depends on that API. There are some things that ca n be mitigated such as challenges with borrowed references but that doesn't seem to have been the focus on any of the PRs.
In short, I'm somewhat concerned about the extensive changes that are occurring. I do know they will touch substantially every C module in the entire ecosystem. I don't know whether they are safe or whether they will give any real benefit.
FWIW, none of this is a criticism of the work being done. Someone needs to think deeply about the C API or else progress will never be made. That said, it is a high risk project with many PRs going directly into master, so it does warrant having buy in that the churn isn't destabilizing and will actually produce a benefit that is worth it.
Raymond
From my point of view: A static inline function is a much nicer modern code
While I haven't looked at *all* of the existing changes, glancing at an example one, the XINCREF and XDECREF macro -> static inline .h function change from https://github.com/python/cpython/commit/541497e6197268517b0d492856027774c43..., I don't have any concerns. Because I am confident that gcc and clang will behave well with this type of change. style than a C preprocessor macro. I expect the largest visible impact may be that a profiler may now attribute CPU cycles takes by these code snippets to the function from the .h file rather than directly to the functions the macro expanded in in the past due to additional debug symbol info attribution. Just more data. Consider that a win. I do not believe any compiler will generate poor code from this on any meaningful system and compiler configuration (-O0 or -Og code seems irrelevant, nothing ships that way). Regardless, pablogsal appears to be doing the homework to double check which seems like a good plan before we release 3.8. :) -gps
Le jeu. 15 nov. 2018 à 01:06, Gregory P. Smith <greg@krypto.org> a écrit :
I expect the largest visible impact may be that a profiler may now attribute CPU cycles takes by these code snippets to the function from the .h file rather than directly to the functions the macro expanded in in the past due to additional debug symbol info attribution. Just more data. Consider that a win.
Oh. That's very interesting. I just tried gdb and I confirm that gdb understands well inlined function. When I debug Python, gdb moves into Py_INCREF() or Py_DECREF() when I use "next". I also tried perf record/perf report: if I annotate a function (assembler code of the function), perf shows me the C code of inlined Py_INCREF and Py_DECREF! That's nice! Victor
Gregory P. Smith schrieb am 15.11.18 um 01:03:
From my point of view: A static inline function is a much nicer modern code style than a C preprocessor macro.
It's also slower to compile, given that function inlining happens at a much later point in the compiler pipeline than macro expansion. The C compiler won't even get to see macros in fact, whereas whether to inline a function or not is a dedicated decision during the optimisation phase based on metrics collected in earlier stages. For something as ubiquitous as Py_INCREF/Py_DECREF, it might even be visible in the compilation times. Oh, BTW, I don't know if this was mentioned in the discussion before, but transitive inlining can easily be impacted by the switch from a macro to an inline function. Since inlining happens long before the final CPU code generation, the C compiler needs to uses heuristics for estimating the eventual "code weight" of an inline function, and then sums up all weights within a calling function to decide whether to also inline that calling function into the transitive callers or not. Now imagine that you have an inline function that executes several Py_INCREF/Py_DECREF call cycles, and the C compiler happens to slightly overestimate the weights of these two. Then it might end up deciding against inlining the function now, whereas it previously might have decided for it since it was able to see the exact source code expanded from the macros. I think that's what Raymond meant with his concerns regarding changing macros into inline functions. C compilers might be smart enough to always inline CPython's new inline functions themselves, but the style change can still have unexpected transitive impacts on code that uses them. I agree with Raymond that as long as there is no clear gain in this code churn, we should not underestimate the risk of degarding code on user side. Stefan
On Sun, Nov 18, 2018 at 8:52 AM Stefan Behnel <stefan_ml@behnel.de> wrote:
Gregory P. Smith schrieb am 15.11.18 um 01:03:
From my point of view: A static inline function is a much nicer modern code style than a C preprocessor macro.
It's also slower to compile, given that function inlining happens at a much later point in the compiler pipeline than macro expansion. The C compiler won't even get to see macros in fact, whereas whether to inline a function or not is a dedicated decision during the optimisation phase based on metrics collected in earlier stages. For something as ubiquitous as Py_INCREF/Py_DECREF, it might even be visible in the compilation times.
Have you measured this? I had the opposite intuition, that macros on average will be slower to compile because they increase the amount of code that the frontend has to process. But I've never checked... -n -- Nathaniel J. Smith -- https://vorpus.org
On Sun, 18 Nov 2018 13:53:54 -0800 Nathaniel Smith <njs@pobox.com> wrote:
On Sun, Nov 18, 2018 at 8:52 AM Stefan Behnel <stefan_ml@behnel.de> wrote:
Gregory P. Smith schrieb am 15.11.18 um 01:03:
From my point of view: A static inline function is a much nicer modern code style than a C preprocessor macro.
It's also slower to compile, given that function inlining happens at a much later point in the compiler pipeline than macro expansion. The C compiler won't even get to see macros in fact, whereas whether to inline a function or not is a dedicated decision during the optimisation phase based on metrics collected in earlier stages. For something as ubiquitous as Py_INCREF/Py_DECREF, it might even be visible in the compilation times.
Have you measured this? I had the opposite intuition, that macros on average will be slower to compile because they increase the amount of code that the frontend has to process. But I've never checked...
It will certainly depend on how much code the macro expands to. Py_INCREF is an extremely simple macro, so expanding everywhere doesn't sound like a problem. On the other hand, modern "macros" that are C++ templates can inline vast amounts of code at the call site, and that's a common cause of slow C++ compiles. Regards Antoine.
Le dim. 18 nov. 2018 à 17:54, Stefan Behnel <stefan_ml@behnel.de> a écrit :
It's also slower to compile, given that function inlining happens at a much later point in the compiler pipeline than macro expansion. The C compiler won't even get to see macros in fact, whereas whether to inline a function or not is a dedicated decision during the optimisation phase based on metrics collected in earlier stages. For something as ubiquitous as Py_INCREF/Py_DECREF, it might even be visible in the compilation times.
I ran a benchmark: there is no significant slowdown (+4 seconds, 6% slower, in the worst case). https://bugs.python.org/issue35059#msg330316
Now imagine that you have an inline function that executes several Py_INCREF/Py_DECREF call cycles, and the C compiler happens to slightly overestimate the weights of these two. Then it might end up deciding against inlining the function now, whereas it previously might have decided for it since it was able to see the exact source code expanded from the macros. I think that's what Raymond meant with his concerns regarding changing macros into inline functions. C compilers might be smart enough to always inline CPython's new inline functions themselves, but the style change can still have unexpected transitive impacts on code that uses them.
I ran the performance benchmark suite to compare C macros to static inline functions: there is no significant impact on performance. https://bugs.python.org/issue35059#msg330302
I agree with Raymond that as long as there is no clear gain in this code churn, we should not underestimate the risk of degarding code on user side.
I don't understand how what you mean with "degarding code on user side". If you are talking about performance, again, my changes have no significant impact on performance (not on compilation time nor runtime performance).
"there is no clear gain in this code churn"
There are multiple advantages: * Better development and debugging experience: tools understand inlined functions much better than C macros: gdb, Linux perf, etc. * Better API: arguments now have a type and the function has a return type. In practice, some macros still cast their argument to PyObject* to not introduce new compiler warnings in Python 3.8. For example, even if Py_INCREF() is documented (*) as a function expecting PyObject*, it accepts any pointer type (PyTupleObject*, PyUnicodeObject*, etc.). Technically, it also accepts PyObject** which is a bug, but that's a different story ;-) * Much better code, just plain regular C. C macros are ugly: "do { ... } while (0)" workaround, additional parenthesis around each argument, strange "expr1, expr2" syntax of "macro expression" which returns a value (inline function just uses regular "return" and ";" at the end of instructions), strange indentation, etc. * No more "macro pitfals": https://gcc.gnu.org/onlinedocs/cpp/Macro-Pitfalls.html * Local variables no longer need a magic name to avoid risk of name conflict, and have a clearly defined scope. Py_DECREF() and _Py_XINCREF() no longer need a local variable since it's argument already has a clearly defined type: PyObject*. I introduced a new variable in _Py_Dealloc() to fix a possible race condition. Previously, the variable was probably avoided because it's tricky use variables in macros. * #ifdef can now be used inside the inline function: it makes the code easier to understand. * etc. Are you aware that Python had macros like: #define _Py_REF_DEBUG_COMMA , #define _Py_CHECK_REFCNT(OP) /* a semicolon */; I let you judge the quality of this macro: #define _Py_NewReference(op) ( \ _Py_INC_TPALLOCS(op) _Py_COUNT_ALLOCS_COMMA \ _Py_INC_REFTOTAL _Py_REF_DEBUG_COMMA \ Py_REFCNT(op) = 1) Is it an expression? Can it be used in "if (test) _Py_NewReference(op);"? It doesn't use the "do { ... } while (0)" protection against macro pitfals. (*) Py_INCREF doc: https://docs.python.org/dev/c-api/refcounting.html#c.Py_INCREF Victor
participants (6)
-
Antoine Pitrou -
Gregory P. Smith -
Nathaniel Smith -
Raymond Hettinger -
Stefan Behnel -
Victor Stinner