Start writing inlines rather than macros?

Hi there. The discussion on http://bugs.python.org/issue20440 started me thinking that much of this bikeshedding could be avoided if we weren't constrained to writing macros for all of this stuff. For example, a Py_INLINE(PyObject *) Py_Incref(PyObject *obj) { Py_INCREF(obj); return obj; } could be used in a Py_Assign() function, if a new reference were wanted: Py_INLINE(void) Py_Assign(PyObject ** target, PyObject *obj) { PyObject *tmp = *target; *target = tmp; Py_DECREF(tmp); } So that you could then safely write code like Py_Assign(&MyVar, Py_Incref(obj)); This would also allow you to stop writing various super macros to try to cater to all possible permutations. Now, Larry Hastings pointed out that we support C89 which doesn't support Inlines. Rather than suggesting here that we update that compatibility requirement, how about adding a Py_INLINE() macro.? This would be like Py_LOCAL_INLINE() except that it would drop the "static" keyword, unless inline isn't supported: #if defined(_MSC_VER) #define Py_INLINE(type) __inline type #elif defined(USE_INLINE) #define Py_INLINE(type) inline type #else #define Py_INLINE(type) static type #endif The only question is with the last line. How many platforms actually _do_not_ have inlines? Would writing stuff like this be considered problematic for those platforms? Note, that I'm not suggesting replacing macros all over the place, only for new code. Another question is: Is "static inline" in any practical way different from "inline"? It would be great to get rid of macros in code. It would be great for debugging too! K

Hi, 2014-02-27 11:22 GMT+01:00 Kristján Valur Jónsson <kristjan@ccpgames.com>:
In practice, recent versions of GCC and Clang are used. On Windows, it's Visual Studio 2010. I'm pretty sure that these compilers support inline functions. I'm also in favor of using inline functions instead of long macros using ugly hacks like "instr1,instr2" syntax where instr1 used assert(). See for example unicodeobject.c to have an idea of what horrible macros mean. I'm in favor of dropping C89 support and require at least C99. There is now C11, it's time to drop the old C89. http://en.wikipedia.org/wiki/C11_%28C_standard_revision%29 Victor

well, requiring C99 is another discussion which I'm not so keen on instigating :) As you point out, most of our target platforms probably do support inline already. My question is more of the nature: What about those that don't support inline, is there any harm in defaulting to "static" in that case and leave the inlining to the optimizer on those platforms? K

On 27/02/14 13:06, Kristján Valur Jónsson wrote:
I agree, modern compilers will inline quite aggressively, so declaring a function static is as good as declaring it inline, provided the function is small. Static functions are a lot easier to read and maintain than LOUD_BUT_UNTYPED_MACRO(x) :) Cheers, Mark.

On Thu, Feb 27, 2014 at 5:47 AM, Victor Stinner <victor.stinner@gmail.com>wrote:
The Visual Studio team has publicly stated they will never support C99, so dropping C89 blindly is going to alienate a big part of our user base unless we switch to C++ instead. I'm fine with trying to pull in C99 features, though, that we can somehow support in a backwards-compatible way with VS.

Brett Cannon <brett@python.org> wrote:
So you are saying that Python should use "the C that Visual Studio supports"? I believe Microsoft is not competent to define the C standard. If they cannot provide a compiler that is their bad. There are plenty of other standard-compliant compilers we can use, including Intel, clang and gcc (MinGW). Sturla

On Thu, Feb 27, 2014 at 12:22 PM, Sturla Molden <sturla.molden@gmail.com>wrote:
Well, C89 + Amendments which happens to be what all C compilers support, including VS.
I believe Microsoft is not competent to define the C standard.
Maybe, but they still control a large install base.
If they cannot provide a compiler that is their bad.
And unfortunately ours if we want Windows developers to be able to use CPython for things like embedding,
You manage to convince a majority of Windows developers to switch compilers and then I would be happy to promote we drop VS support and switch entirely to C99. Until then, though, this is like suggesting we cut off Windows XP because MS doesn't have long-term support anymore: it's a choice between being pragmatic for serving our install base or doing something to simplify our lives.

On Thu, 27 Feb 2014 17:22:37 +0000 (UTC) Sturla Molden <sturla.molden@gmail.com> wrote:
Other C compilers don't support all of C99, AFAIR. We sometimes get such bug reports from e.g. AIX users. I'd be all for accepting C99, ideally. But we must care about our users' constraints. Regards Antoine.

I think it's at least worthwhile to investigate the use of inline/static functions over the current macros. It's been many years since I looked at them. I doubt they have gotten any easier to read or edit with all their backslashes. I do have one question though. Suppose you encounter a compiler that doesn't understand the inline keyword, so you choose the static declaration as Kristján suggested. The resulting Python executable should be functionally correct, but if the optimizer doesn't happen to inline a given static function you might be stuck with some bad performance across-the-board (if it never inlines, or doesn't inline where we really need it to), or only under some circumstances (as a hypothetical example, inlining in dictobject.c, but not in ceval.c). Is there a configurable way to tell if a compiler will inline functions which are declared static, and possibly under what conditions they might not? It might still be necessary to maintain macros for those platforms. Skip

On Thu, 27 Feb 2014 13:12:24 -0600 Skip Montanaro <skip@pobox.com> wrote:
I can assure you they haven't :-)
You're right. Since we only define macros where performance is critical (such as INCREF and DECREF), it would definitely have a very significant impact on performance.
Well, if we must maintain macros, let's maintain them everywhere and avoid the burden of two different implementations for the same thing. Regards Antoine.

On Thu, Feb 27, 2014 at 1:23 PM, Antoine Pitrou <solipsis@pitrou.net> wrote:
Well, if we must maintain macros, let's maintain them everywhere and avoid the burden of two different implementations for the same thing.
Would it be possible to generate the macro versions from the inline/static versions where necessary, as some sort of pre-compile step? S

Skip Montanaro writes:
To do that in general is a hard problem, unless you're a C compiler that already supports inlining. I'm not sure how easy it would be to define rules that support the inlines Python demands for performance, and yet makes function to macro conversion easy. I think it makes much more sense to look at rewriting the macros by hand so that they emulate the necessary amount of function semantics (eg, providing argument type-checking in debug builds, not mutating arguments unless passed by reference, evaluating arguments exactly once, etc). XEmacs has a lot of these for precisely the reason this thread started (plus the recognition at the time we started this practice that a number of our platforms could not be trusted to inline certain performance-critical routines). They're spelled using the same conventions as functions, so macros that don't obey those rules stick out. Then these function-like macros could be #ifdef'd with the inline functions, with some (automated) comparison testing on appropriate benchmarks by building with and without -dDONT_TRUST_INLINE. And then beta-testing by defaulting to #undef DONT_TRUST_INLINE. Then wait for cries of pain -- if none, remove the dead #ifdef branches in the last beta or so. Yes, this involves a certain amount of work, but I think it's bounded. There should be no extra maintenance of the #ifdef'd parts compared to the maintenance required to fix bugs resulting from change from macro semantics to inline semantics anyway, unless changing the macros is mistake-prone -- which will be recognized immediately.

It would be horrible to have to maintain both macros and functions. My suggestion would be to use functions for new code, and new use cases. We would stick with Py_INCREF() , and continue using that in obvious cases, (such as in the implementation of the new functions) but at the same time introduce a Py_IncRef() function returning a new reference, which can be used in new code where it is convenient. The new macros under discussion, Py_INCREF_AND_REPLACE_WITH_GUSTO() and all of them would then be redesigned in a more composable functional form. This way, only new code would be compiled with different efficiency on different platforms, thus avoiding introducing performance regressions. K

Skip Montanaro <skip@pobox.com> wrote:
I think that all modern compilers can handle "static inline" in header files. If you have a compiler that cannot, chances are that the platform is horribly outdated and this particular performance issue will be relatively benign compared to other ones. Stefan Krah

Hi, 2014-02-27 11:22 GMT+01:00 Kristján Valur Jónsson <kristjan@ccpgames.com>:
In practice, recent versions of GCC and Clang are used. On Windows, it's Visual Studio 2010. I'm pretty sure that these compilers support inline functions. I'm also in favor of using inline functions instead of long macros using ugly hacks like "instr1,instr2" syntax where instr1 used assert(). See for example unicodeobject.c to have an idea of what horrible macros mean. I'm in favor of dropping C89 support and require at least C99. There is now C11, it's time to drop the old C89. http://en.wikipedia.org/wiki/C11_%28C_standard_revision%29 Victor

well, requiring C99 is another discussion which I'm not so keen on instigating :) As you point out, most of our target platforms probably do support inline already. My question is more of the nature: What about those that don't support inline, is there any harm in defaulting to "static" in that case and leave the inlining to the optimizer on those platforms? K

On 27/02/14 13:06, Kristján Valur Jónsson wrote:
I agree, modern compilers will inline quite aggressively, so declaring a function static is as good as declaring it inline, provided the function is small. Static functions are a lot easier to read and maintain than LOUD_BUT_UNTYPED_MACRO(x) :) Cheers, Mark.

On Thu, Feb 27, 2014 at 5:47 AM, Victor Stinner <victor.stinner@gmail.com>wrote:
The Visual Studio team has publicly stated they will never support C99, so dropping C89 blindly is going to alienate a big part of our user base unless we switch to C++ instead. I'm fine with trying to pull in C99 features, though, that we can somehow support in a backwards-compatible way with VS.

Brett Cannon <brett@python.org> wrote:
So you are saying that Python should use "the C that Visual Studio supports"? I believe Microsoft is not competent to define the C standard. If they cannot provide a compiler that is their bad. There are plenty of other standard-compliant compilers we can use, including Intel, clang and gcc (MinGW). Sturla

On Thu, Feb 27, 2014 at 12:22 PM, Sturla Molden <sturla.molden@gmail.com>wrote:
Well, C89 + Amendments which happens to be what all C compilers support, including VS.
I believe Microsoft is not competent to define the C standard.
Maybe, but they still control a large install base.
If they cannot provide a compiler that is their bad.
And unfortunately ours if we want Windows developers to be able to use CPython for things like embedding,
You manage to convince a majority of Windows developers to switch compilers and then I would be happy to promote we drop VS support and switch entirely to C99. Until then, though, this is like suggesting we cut off Windows XP because MS doesn't have long-term support anymore: it's a choice between being pragmatic for serving our install base or doing something to simplify our lives.

On Thu, 27 Feb 2014 17:22:37 +0000 (UTC) Sturla Molden <sturla.molden@gmail.com> wrote:
Other C compilers don't support all of C99, AFAIR. We sometimes get such bug reports from e.g. AIX users. I'd be all for accepting C99, ideally. But we must care about our users' constraints. Regards Antoine.

I think it's at least worthwhile to investigate the use of inline/static functions over the current macros. It's been many years since I looked at them. I doubt they have gotten any easier to read or edit with all their backslashes. I do have one question though. Suppose you encounter a compiler that doesn't understand the inline keyword, so you choose the static declaration as Kristján suggested. The resulting Python executable should be functionally correct, but if the optimizer doesn't happen to inline a given static function you might be stuck with some bad performance across-the-board (if it never inlines, or doesn't inline where we really need it to), or only under some circumstances (as a hypothetical example, inlining in dictobject.c, but not in ceval.c). Is there a configurable way to tell if a compiler will inline functions which are declared static, and possibly under what conditions they might not? It might still be necessary to maintain macros for those platforms. Skip

On Thu, 27 Feb 2014 13:12:24 -0600 Skip Montanaro <skip@pobox.com> wrote:
I can assure you they haven't :-)
You're right. Since we only define macros where performance is critical (such as INCREF and DECREF), it would definitely have a very significant impact on performance.
Well, if we must maintain macros, let's maintain them everywhere and avoid the burden of two different implementations for the same thing. Regards Antoine.

On Thu, Feb 27, 2014 at 1:23 PM, Antoine Pitrou <solipsis@pitrou.net> wrote:
Well, if we must maintain macros, let's maintain them everywhere and avoid the burden of two different implementations for the same thing.
Would it be possible to generate the macro versions from the inline/static versions where necessary, as some sort of pre-compile step? S

Skip Montanaro writes:
To do that in general is a hard problem, unless you're a C compiler that already supports inlining. I'm not sure how easy it would be to define rules that support the inlines Python demands for performance, and yet makes function to macro conversion easy. I think it makes much more sense to look at rewriting the macros by hand so that they emulate the necessary amount of function semantics (eg, providing argument type-checking in debug builds, not mutating arguments unless passed by reference, evaluating arguments exactly once, etc). XEmacs has a lot of these for precisely the reason this thread started (plus the recognition at the time we started this practice that a number of our platforms could not be trusted to inline certain performance-critical routines). They're spelled using the same conventions as functions, so macros that don't obey those rules stick out. Then these function-like macros could be #ifdef'd with the inline functions, with some (automated) comparison testing on appropriate benchmarks by building with and without -dDONT_TRUST_INLINE. And then beta-testing by defaulting to #undef DONT_TRUST_INLINE. Then wait for cries of pain -- if none, remove the dead #ifdef branches in the last beta or so. Yes, this involves a certain amount of work, but I think it's bounded. There should be no extra maintenance of the #ifdef'd parts compared to the maintenance required to fix bugs resulting from change from macro semantics to inline semantics anyway, unless changing the macros is mistake-prone -- which will be recognized immediately.

It would be horrible to have to maintain both macros and functions. My suggestion would be to use functions for new code, and new use cases. We would stick with Py_INCREF() , and continue using that in obvious cases, (such as in the implementation of the new functions) but at the same time introduce a Py_IncRef() function returning a new reference, which can be used in new code where it is convenient. The new macros under discussion, Py_INCREF_AND_REPLACE_WITH_GUSTO() and all of them would then be redesigned in a more composable functional form. This way, only new code would be compiled with different efficiency on different platforms, thus avoiding introducing performance regressions. K

Skip Montanaro <skip@pobox.com> wrote:
I think that all modern compilers can handle "static inline" in header files. If you have a compiler that cannot, chances are that the platform is horribly outdated and this particular performance issue will be relatively benign compared to other ones. Stefan Krah
participants (9)
-
Antoine Pitrou
-
Brett Cannon
-
Kristján Valur Jónsson
-
Mark Shannon
-
Skip Montanaro
-
Stefan Krah
-
Stephen J. Turnbull
-
Sturla Molden
-
Victor Stinner