all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: 12541@debbugs.gnu.org
Subject: bug#12541: Prefer plain 'static' to 'static inline'.
Date: Mon, 01 Oct 2012 19:14:00 +0200	[thread overview]
Message-ID: <83fw5yazdz.fsf@gnu.org> (raw)
In-Reply-To: <506893EF.8080604@cs.ucla.edu>

> Date: Sun, 30 Sep 2012 11:48:15 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 12541@debbugs.gnu.org
> 
> >> With the proposed change, the set of functions that are not always
> >> inlined expands to bidi_cache_iterator_state, bidi_char_at_pos, and
> >> bidi_fetch_char, and (if we also include functions that are partially
> >> inlined) bidi_cache_search and bidi_get_type.
> > 
> > Were they also not inlined before the change?
> 
> Most of these functions were inlined before the change.  However, as
> described above, bidi_char_at_pos was only partially inlined before
> the change.

It surprises me that bidi_char_at_pos is only partially inlined, or
not at all, since its body is just this:

  static inline int
  bidi_char_at_pos (ptrdiff_t bytepos, const unsigned char *s, bool unibyte)
  {
    if (s)
      {
	s += bytepos;
	if (unibyte)
	  return *s;
      }
    else
      s = BYTE_POS_ADDR (bytepos);
    return STRING_CHAR (s);
  }

Perhaps that's because STRING_CHAR could call a function?

Likewise with bidi_get_type.  This one was supposed to be as fast as
the C ctype macros, or close.  Handa-san told me that the uniprop
tables he implemented for this should really be fast.  Maybe again the
problem is that CHAR_TABLE_REF can call a function?

Anyway, it's disturbing that the functions you mention will no longer
be inlined, because they are at the lowest level of walking the buffer
and deciding what to do with the next character.  In particular,
bidi_get_type and bidi_fetch_char are invoked for every character we
display, even if there's no R2L characters anywhere in sight.  (By
contrast, bidi_cache_iterator_state and bidi_cache_search will only be
invoked if we bump into an R2L character.)

Nevertheless, I'm okay with removing the 'inline' qualifier from
bidi.c.  If the performance hit is significant, I'm sure we will hear
from users shortly.  Otherwise, I'll try to measure the performance
with and without 'inline' when I have time, and we can take it from
there.





  parent reply	other threads:[~2012-10-01 17:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-30  8:00 bug#12541: Prefer plain 'static' to 'static inline' Paul Eggert
2012-09-30  9:49 ` Eli Zaretskii
2012-09-30 14:18   ` Jason Rumney
2012-09-30 15:57     ` Eli Zaretskii
2012-09-30 17:58   ` Paul Eggert
2012-09-30 18:33     ` Eli Zaretskii
2012-09-30 18:48       ` Paul Eggert
2012-10-01  6:38         ` Paul Eggert
2012-10-01 17:14         ` Eli Zaretskii [this message]
2012-10-02  7:00           ` Paul Eggert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83fw5yazdz.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=12541@debbugs.gnu.org \
    --cc=eggert@cs.ucla.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.