Emacs development

Paul Eggert | 26 Jun 2012 16:53

inline build_string performance

Trunk bzr 108742 changed build_string from a standard
extern function to a static inline function:

static inline Lisp_Object
build_string (const char *str)
{
  return make_string (str, strlen (str));
}

This is not an unalloyed win, since it bloats the
size of the Emacs executable, as callers to build_string
often now have two function calls (strlen + make_string),
not one (just build_string).  In my environment
(Fedora 15 x86-64, GCC 4.7.1) this patch adds 5704 bytes (0.25%)
to the size of the Emacs text.  Presumably this puts
a bit more pressure on the text cache.

Has a performance analysis been done on this change
showing that the code bloat is worth it?

While we're on the subject, I suspect that
we'll get more of a performance win by having a
function 'build_unibyte_string' in the common case
where build_string is invoked on text that is known
to be unibyte, as that avoids a separate pass through
the string.  I haven't had time to investigate this,
though.

Andreas Schwab | 26 Jun 2012 17:17

Re: inline build_string performance

Paul Eggert <eggert <at> cs.ucla.edu> writes:

> Trunk bzr 108742 changed build_string from a standard
> extern function to a static inline function:
>
> static inline Lisp_Object
> build_string (const char *str)
> {
>   return make_string (str, strlen (str));
> }
>
> This is not an unalloyed win, since it bloats the
> size of the Emacs executable, as callers to build_string
> often now have two function calls (strlen + make_string),
> not one (just build_string).

On the plus side the argument of build_string is often a constant which
causes the strlen call to be optimized out completely.

Andreas.

--

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

Dmitry Antipov | 26 Jun 2012 18:29

Picon

Re: inline build_string performance

On 06/26/2012 06:53 PM, Paul Eggert wrote:

> This is not an unalloyed win, since it bloats the
> size of the Emacs executable, as callers to build_string
> often now have two function calls (strlen + make_string),
> not one (just build_string).

The core idea of this stuff is to eliminate calls to strlen when an
argument of build_string is a compile-time constant - the compiler
should just replace call to strlen with constant integer.

As of r108741, my binary image has 336 calls to build_string and
26 calls to make_string; r108742 has 34 and 328, respectively, and
~130 calls of make_string are performed with constant 2nd argument
(i.e. strlen was totally optimized away).

> In my environment
> (Fedora 15 x86-64, GCC 4.7.1) this patch adds 5704 bytes (0.25%)
> to the size of the Emacs text.  Presumably this puts
> a bit more pressure on the text cache.

I have just 3664 bytes (GCC 4.7.1 with -O3, stripped):

[dima <at> notebook src]$ readelf -S emacs.r108741  | grep -A1 .text
   [14] .text             PROGBITS         000000000040bf20  0000bf20
        0000000000217dc4  0000000000000000  AX       0     0     16
[dima <at> notebook src]$ readelf -S emacs.r108742  | grep -A1 .text
   [14] .text             PROGBITS         000000000040bf20  0000bf20
        0000000000218c14  0000000000000000  AX       0     0     16

(Continue reading)

Paul Eggert | 26 Jun 2012 18:42

Re: inline build_string performance

On 06/26/2012 09:29 AM, Dmitry Antipov wrote:
> 
> The core idea of this stuff is to eliminate calls to strlen when an
> argument of build_string is a compile-time constant

That sounds worthwhile for critical paths.

How about reverting the build_string change, and defining a new
inline function build_literal intended for when the argument is a
string literal and for when speed is more important than conserving
code space?  That would give us speed where speed matters and where
we know it'll be faster, while avoiding code bloat otherwise.
build_string and  build_literal would have identical semantics,
but different performance properties.

Dmitry Antipov | 26 Jun 2012 19:33

Picon

Re: inline build_string performance

On 06/26/2012 08:42 PM, Paul Eggert wrote:

> That sounds worthwhile for critical paths.
>
> How about reverting the build_string change, and defining a new
> inline function build_literal intended for when the argument is a
> string literal and for when speed is more important than conserving
> code space?  That would give us speed where speed matters and where
> we know it'll be faster, while avoiding code bloat otherwise.
> build_string and  build_literal would have identical semantics,
> but different performance properties.

This may be implemented without reverting previous stuff. It will also gives
other (non-GCC) compilers a chance to demonstrate their optimization skills.

Dmitry

=== modified file 'src/alloc.c'
--- src/alloc.c	2012-06-26 14:41:01 +0000
+++ src/alloc.c	2012-06-26 17:26:02 +0000
 <at>  <at>  -2533,6 +2533,25  <at>  <at> 
   return string;
 }

+/* Fast version used when both STR and NBYTES are compile-time
+   constants, and all characters from STR are ASCII.  */
+
+Lisp_Object

(Continue reading)

Paul Eggert | 26 Jun 2012 19:37

Re: inline build_string performance

On 06/26/2012 10:33 AM, Dmitry Antipov wrote:
> This may be implemented without reverting previous stuff.

Sure, but if we we use build_literal for literals, and
build_string for non-literals, there's a performance
downside for having build_string be inline (namely,
it bloats the code), but no performance upside.

Perhaps I'm not quite following your idea, as
I thought the signature would be build_literal ("foo"),
whereas your build_literal also wants the string length.

Dmitry Antipov | 26 Jun 2012 19:58

Picon

Re: inline build_string performance

On 06/26/2012 09:37 PM, Paul Eggert wrote:

> Sure, but if we we use build_literal for literals, and
> build_string for non-literals, there's a performance
> downside for having build_string be inline (namely,
> it bloats the code), but no performance upside.

I don't understand this - build_string is a macro which expands to
build_literal (S, strlen (S)) if S is a compile-time constant,
and to make_string (S, strlen (S)) otherwise.  Note that it's possible
to have a nasty bug if someone calls build_literal with non-ASCII
string constant - it's definitely impossible to determine whether
a string constant is non-ASCII at compile time.

> Perhaps I'm not quite following your idea, as
> I thought the signature would be build_literal ("foo"),
> whereas your build_literal also wants the string length.

This assumes something like:

#define build_literal(str) __build_literal (str, strlen (str))

and clever compiler which can optimize away calls to strlen
if STR is a compile-time constant.

Another approach is to refactor all code like:

x = build_string ("test");

to:

(Continue reading)

Paul Eggert | 26 Jun 2012 20:46

Re: inline build_string performance

On 06/26/2012 10:58 AM, Dmitry Antipov wrote:
> build_string is a macro which expands to
> build_literal (S, strlen (S)) if S is a compile-time constant,
> and to make_string (S, strlen (S)) otherwise.

Yes, but in the latter case there is code bloat
but no compensating performance benefit.

How about the following instead?

=== modified file 'src/alloc.c'
--- src/alloc.c	2012-06-26 14:41:01 +0000
+++ src/alloc.c	2012-06-26 18:43:25 +0000
 <at>  <at>  -2496,6 +2496,20  <at>  <at> 
 }

 
+/* Make a string from the data at STR, treating it as multibyte if the
+   data warrants.  */
+
+Lisp_Object
+build_string_1 (const char *str)
+{
+  return make_string (str, strlen (str));
+}
+
+/* Make the build_string macro visible to GDB.  */
+extern Lisp_Object (build_string) (const char *) EXTERNALLY_VISIBLE;
+Lisp_Object (build_string) (const char *s) { return build_string (s); }
+

(Continue reading)

Eli Zaretskii | 26 Jun 2012 22:41

Picon

Re: inline build_string performance

> Date: Tue, 26 Jun 2012 11:46:34 -0700
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Cc: Emacs Development <Emacs-devel <at> gnu.org>
> 
> How about the following instead?

Another change that sacrifices maintainability (and the sanity of the
maintainers) for 0.0003% of speedup.

Thien-Thi Nguyen | 26 Jun 2012 20:51

Re: inline build_string performance

() Dmitry Antipov <dmantipov <at> yandex.ru>
() Tue, 26 Jun 2012 21:58:34 +0400

   This assumes something like:

   #define build_literal(str) __build_literal (str, strlen (str))

If the argument ‘str’ is a literal string, how about:

  /* -*- c -*- */
  #define build_literal(str)  make_string (str, sizeof (str) - 1)

Or is ‘sizeof’ somehow unsuitable for the job?

Stefan Monnier | 27 Jun 2012 02:39

Picon

Re: inline build_string performance

Let's stop the madness.  This is all mostly irrelevant.  The only think
that's important here is code cleanliness.
So let's make things as simple and clear as possible, please.

        Stefan

Eli Zaretskii | 27 Jun 2012 05:02

Picon

Re: inline build_string performance

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Date: Tue, 26 Jun 2012 20:39:58 -0400
> Cc: Dmitry Antipov <dmantipov <at> yandex.ru>,
> 	Emacs Development <Emacs-devel <at> gnu.org>
> 
> Let's stop the madness.  This is all mostly irrelevant.  The only think
> that's important here is code cleanliness.
> So let's make things as simple and clear as possible, please.

110% agreement.  Each change that spreads information about the
internals between several places adds to the obstacles one must
negotiate to make non-trivial changes in Emacs internals without
introducing bugs.  The bar is already too high.  If we want more
developers to come on board, we should make such changes only if they
give significant tangible wins.

Return

Return to gmane.emacs.devel.

Advertisement

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane