Inlining doesn't happen on OS X: big performance problem

Inlining doesn't happen on OS X: big performance problem

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]

Currently, gnulib converts "inline" to "static" on OS X. In particular,
config.h uses this logic from m4/extern-inline.m4:

#if ((__GNUC__ \
      ? defined __GNUC_STDC_INLINE__ && __GNUC_STDC_INLINE__ \
      : (199901L <= __STDC_VERSION__ \
         && !defined __HP_cc \
         && !(defined __SUNPRO_C && __STDC__))) \
     && !defined __APPLE__)
...
#elif (2 < __GNUC__ + (7 <= __GNUC_MINOR__) && !defined __STRICT_ANSI__ \
       && !defined __APPLE__)
...
#else
# define _GL_INLINE static _GL_UNUSED
# define _GL_EXTERN_INLINE static _GL_UNUSED
#endif

Using "static" for "inline" comes at a tremendous cost in performance.
Compiling with gcc 4.2.1 -O2, Emacs HEAD takes 113.9 seconds to
font-lock-fontify-buffer lisp.h ten times. If I replace "!defined
__APPLE__" with "1" above, Emacs compiles and runs perfectly well
(albeit with some warnings about missing declarations) and performs the
same task in only 36.8 seconds. Disassembly confirms that in the current
configuration, nothing is actually being inlined.

Can we please remove these "!defined __APPLE__" tests and start inlining
functions again in OS X?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Inlining doesn't happen on OS X: big performance problem

[Ryan Johnson]

On 19/09/2013 9:35 AM, Daniel Colascione wrote:
> Currently, gnulib converts "inline" to "static" on OS X. In particular,
> config.h uses this logic from m4/extern-inline.m4:
>
> #if ((__GNUC__ \
>        ? defined __GNUC_STDC_INLINE__ && __GNUC_STDC_INLINE__ \
>        : (199901L <= __STDC_VERSION__ \
>           && !defined __HP_cc \
>           && !(defined __SUNPRO_C && __STDC__))) \
>       && !defined __APPLE__)
> ...
> #elif (2 < __GNUC__ + (7 <= __GNUC_MINOR__) && !defined __STRICT_ANSI__ \
>         && !defined __APPLE__)
> ...
> #else
> # define _GL_INLINE static _GL_UNUSED
> # define _GL_EXTERN_INLINE static _GL_UNUSED
> #endif
>
> Using "static" for "inline" comes at a tremendous cost in performance.
> Compiling with gcc 4.2.1 -O2, Emacs HEAD takes 113.9 seconds to
> font-lock-fontify-buffer lisp.h ten times. If I replace "!defined
> __APPLE__" with "1" above, Emacs compiles and runs perfectly well
> (albeit with some warnings about missing declarations) and performs the
> same task in only 36.8 seconds. Disassembly confirms that in the current
> configuration, nothing is actually being inlined.
>
> Can we please remove these "!defined __APPLE__" tests and start inlining
> functions again in OS X?
>
Surely something that elaborate and costly was a conscious decision... 
have you looked through the revision history to see why this was added 
in the first place? (The hope is that the reason is no longer valid and 
the change can be reverted)

Ryan

Re: Inlining doesn't happen on OS X: big performance problem

[Paul Eggert]

Ryan Johnson wrote:
> Surely something that elaborate and costly was a conscious decision...

Yes, that's correct.  As noted in the code's comments,
the problem is that Apple's C library is incompatible with
C99; see the thread here:

http://lists.gnu.org/archive/html/bug-gnulib/2012-12/msg00023.html

This is registered as Darwin bug 12841334, whatever that is (I don't
have access to the Apple bug database).  As I understand it, it
it's a nontrivial bug, involving only the C library
headers but also clang (the default C compiler used by Apple).
I've corresponded with a developer at Apple about this (most recent
correspondence August 21), and as I understand it Apple plans to
fix the bug at some point, but the fix isn't public yet.  When it
is, I'd like to adjust Gnulib so that it avoids the problem on
fixed versions of Darwin.

In reviewing that correspondence I think that we could work
around some cases of the problem by replacing both instances of
'defined __APPLE__' with this:

    (defined __APPLE__ \                                                        
     && ! (defined _DONT_USE_CTYPE_INLINE_ \
           && ! (defined _FORTIFY_SOURCE && _FORTIFY_SOURCE)))

in m4/extern-inline.m4.  This would improve performance if you
compile Emacs with -D_DONT_USE_CTYPE_INLINE_ -D_FORTIFY_SOURCE=0.
If you're interested in building Emacs that way
I can make that change to Gnulib.

Re: Inlining doesn't happen on OS X: big performance problem

[Andreas Schwab]

Daniel Colascione <dancol@dancol.org> writes:

> Can we please remove these "!defined __APPLE__" tests and start inlining
> functions again in OS X?

Why doesn't the compiler inline static functions by itself?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Inlining doesn't happen on OS X: big performance problem

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

On 9/19/13 9:35 AM, Andreas Schwab wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
>> Can we please remove these "!defined __APPLE__" tests and start inlining
>> functions again in OS X?
> 
> Why doesn't the compiler inline static functions by itself?

Why should it?

Also, I don't see why we've been going through the tree and replacing
instances of "static inline" with "static": sure, it's possible for the
compiler to inline non-"inline" functions, and even functions marked
"inline" might not get inlined, but "static inline" is a good hint and
makes inlining much more likely. Why should we remove this hint?



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Inlining doesn't happen on OS X: big performance problem

[Andreas Schwab]

Daniel Colascione <dancol@dancol.org> writes:

> On 9/19/13 9:35 AM, Andreas Schwab wrote:
>> Daniel Colascione <dancol@dancol.org> writes:
>> 
>>> Can we please remove these "!defined __APPLE__" tests and start inlining
>>> functions again in OS X?
>> 
>> Why doesn't the compiler inline static functions by itself?
>
> Why should it?

There is no reason not to do it.

> Why should we remove this hint?

"Hint" is the essential hint.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Inlining doesn't happen on OS X: big performance problem

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

On 9/19/13 2:04 PM, Andreas Schwab wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
>> On 9/19/13 9:35 AM, Andreas Schwab wrote:
>>> Daniel Colascione <dancol@dancol.org> writes:
>>>
>>>> Can we please remove these "!defined __APPLE__" tests and start inlining
>>>> functions again in OS X?
>>>
>>> Why doesn't the compiler inline static functions by itself?
>>
>> Why should it?
> 
> There is no reason not to do it.

Sure there is: programs would be huge if compilers naively treated every
"static" as "inline, so they don't. Instead, compilers inline some
static functions, some of the time, and the heuristics they use for
deciding whether to do that are inscrutable and variable. Why should we
rely on these heuristics for good performance when we just write "static
inline" and make the decision ourselves?

If we've identified a bit of inlining as performance critical, there is
every reason to tell the compiler "this is important" instead of just
hoping that the compiler figures it out on its own. We shouldn't strip
these hints from existing code, at least.

>> Why should we remove this hint?
> 
> "Hint" is the essential hint.

What's wrong with leaving hints in the code? We still have "register"
all over the place, and "static inline" does much more than "register".


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Inlining doesn't happen on OS X: big performance problem

[Andreas Schwab]

Daniel Colascione <dancol@dancol.org> writes:

> Sure there is: programs would be huge if compilers naively treated every
> "static" as "inline, so they don't.

Just like if they trust the inline hint.  Your point is?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Inlining doesn't happen on OS X: big performance problem

[Paul Eggert]

On 09/19/13 13:58, Daniel Colascione wrote:
> I don't see why we've been going through the tree and replacing
> instances of "static inline" with "static"

See bug#12541; removing the "inline" helped performance slightly there.

More generally, these days "inline" is mostly a noise word for static
functions, just as "register" is a noise word for locals.  Modern
compilers inline static functions pretty well without "inline", just
as they allocate registers pretty well without "register", and it
saves maintenance hassle if developers don't have to waste their
time reading the "inline" noise and worrying about whether the
"inline" should be there.

It may be for some cases on some platforms that it's still helpful to
use the "inline" keyword on a static function, i.e., the performance benefit
outweighs the maintenance hassle.  These can be handled on a case by
case basis.  Adding "inline" might hurt performance on one platform
even if it helps on another, so some care should be taken.

Re: Inlining doesn't happen on OS X: big performance problem

[Óscar Fuentes]

Daniel Colascione <dancol@dancol.org> writes:

>>>> Why doesn't the compiler inline static functions by itself?
>>>
>>> Why should it?
>> 
>> There is no reason not to do it.
>
> Sure there is: programs would be huge if compilers naively treated every
> "static" as "inline, so they don't. Instead, compilers inline some
> static functions, some of the time, and the heuristics they use for
> deciding whether to do that are inscrutable and variable. Why should we
> rely on these heuristics for good performance when we just write "static
> inline" and make the decision ourselves?

The days when the programmer knew better than the compiler about
micro-optimization are long gone, even more so on cross-platform code.

[snip]

>>> Why should we remove this hint?
>> 
>> "Hint" is the essential hint.
>
> What's wrong with leaving hints in the code? We still have "register"
> all over the place, and "static inline" does much more than "register".

Nowadays "register" is as meaningless as "inline".

Re: Inlining doesn't happen on OS X: big performance problem

[Paul Eggert]

On 09/19/13 08:06, Paul Eggert wrote:
> I think that we could work
> around some cases of the problem

I installed an attempt to do that, as trunk
bzr 114401.

Re: Inlining doesn't happen on OS X: big performance problem

[Ryan Johnson]

On 19/09/2013 5:18 PM, Paul Eggert wrote:
> On 09/19/13 13:58, Daniel Colascione wrote:
>> I don't see why we've been going through the tree and replacing
>> instances of "static inline" with "static"
> See bug#12541; removing the "inline" helped performance slightly there.
>
> More generally, these days "inline" is mostly a noise word for static
> functions, just as "register" is a noise word for locals.  Modern
> compilers inline static functions pretty well without "inline", just
> as they allocate registers pretty well without "register", and it
> saves maintenance hassle if developers don't have to waste their
> time reading the "inline" noise and worrying about whether the
> "inline" should be there.
Instead, we get reports of emacs taking 3x longer to do something after 
"inline noise" is removed, leading some to wonder whether "inline" 
should be there after all...

(doesn't mean you want force the compiler to inline everything, but 
going to either extreme is going to hurt performance)

Ryan

Re: Inlining doesn't happen on OS X: big performance problem

[Paul Eggert]

Ryan Johnson wrote:
> we get reports of emacs taking 3x longer to do something after "inline noise" is removed,

No, the 3x issue is not about "static inline" versus "static".
It's about whether standard headers under Apple use
functions or macros, which is quite a different thing.

Re: Inlining doesn't happen on OS X: big performance problem

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

On 9/19/13 2:41 PM, Paul Eggert wrote:
> On 09/19/13 08:06, Paul Eggert wrote:
>> I think that we could work
>> around some cases of the problem
> 
> I installed an attempt to do that, as trunk
> bzr 114401.

Thanks. It works as long as you compile Emacs with the definitions you
mentioned --- in my case, CFLAGS='-O2 -D_FORTIFY_SOURCE=0
-D_DONT_USE_CTYPE_INLINE_=1 -g'.  Can we have Emacs compiled like this
by default on OS X?  I don't like the idea of Emacs being slow when
users run the usual "./configure && make && make install".


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Inlining doesn't happen on OS X: big performance problem

[Ryan Johnson]

On 19/09/2013 6:57 PM, Paul Eggert wrote:
> Ryan Johnson wrote:
>> we get reports of emacs taking 3x longer to do something after "inline noise" is removed,
> No, the 3x issue is not about "static inline" versus "static".
> It's about whether standard headers under Apple use
> functions or macros, which is quite a different thing.
Oh, I must have misunderstood... I thought a bug in the Apple standard 
headers caused problems with inline functions. Sorry for the "inline 
noise"...

Ryan

Re: Inlining doesn't happen on OS X: big performance problem

[Paul Eggert]

Daniel Colascione wrote:
> Can we have Emacs compiled like this
> by default on OS X?

Sure, I gave it a shot in trunk bzr 114403.

Re: Inlining doesn't happen on OS X: big performance problem

[Jan Djärv]

Hello.

19 sep 2013 kl. 23:18 skrev Paul Eggert <eggert@cs.ucla.edu>:

> On 09/19/13 13:58, Daniel Colascione wrote:
>> I don't see why we've been going through the tree and replacing
>> instances of "static inline" with "static"
> 
> See bug#12541; removing the "inline" helped performance slightly there.
> 
> More generally, these days "inline" is mostly a noise word for static
> functions, just as "register" is a noise word for locals.  Modern
> compilers inline static functions pretty well without "inline", just
> as they allocate registers pretty well without "register", and it
> saves maintenance hassle if developers don't have to waste their
> time reading the "inline" noise and worrying about whether the
> "inline" should be there.

Please do not try to motivate the current Emacs inline handling with saved maintenance hassle.  Maintenance is now a magnitude worse than before due to the complicated inline handling.

Before:

1) See INLINE in code.
2) Check out INLINE define in config.h
3) Write your new INLINE function.

Now:

1) See SYSTEM_INLINE in code.
2) Find where it is defined, 2 places, either to INLINE (as _GL_INLINE) or EXTERN_INLINE
    (as _GL_EXTERN_INLINE).
3) Figure out if INLINE or EXTERN_INLINE applies to the case at hand.
4) Find definition of _GL_INLINE and _GL_EXTERN_INLINE.
5) Manually parse complicated conditionals to see which define applies.
6) Sigh when you realize that there are unique preprocessor defines of inline per file.
7) Don't use inline as it is too complicated and convince yourself that is is OK as machines gets
    faster and faster.

Also note that there are still 'static inline' in the w32-code, so if I saw that, I might come to the conclusion that 'static inline' is OK.

I'm sure there is some motivation for the inline mess we have now, but performance is not it (previously we had fast macros) and maintenance is absolutely not it either.

	Jan D.

Re: Inlining doesn't happen on OS X: big performance problem

[Dmitry Antipov]

On 09/20/2013 10:14 AM, Jan Djärv wrote:

> I'm sure there is some motivation for the inline mess we have now, but performance
> is not it (previously we had fast macros) and maintenance is absolutely not it either.

+1, this stuff looks overengineered. In particular, I've never seen
a project where each header should use it's own magic to declare inline
(FRAME_INLINE, BUFFER_INLINE, etc.).

Dmitry

Re: Inlining doesn't happen on OS X: big performance problem

[Stefan Monnier]

> +1, this stuff looks overengineered. In particular, I've never seen
> a project where each header should use it's own magic to declare inline
> (FRAME_INLINE, BUFFER_INLINE, etc.).

FWIW, that also baffles me,


        Stefan

Re: Inlining doesn't happen on OS X: big performance problem

[Paul Eggert]

Stefan Monnier wrote:
> FWIW, that also baffles me,

It's useful for projects that build multiple libraries, where there isn't
centralized control over what gets inlined where, and that's
where I picked up the style.  However, the style is indeed overkill for
Emacs.  I pushed a patch to use a simpler style, with just a
single INLINE macro rather than BUFFER_INLINE, LISP_INLINE, etc.,
as trunk bzr 114410.