CHAR_TO_BYTE in bidi_resolve_explicit_1

CHAR_TO_BYTE in bidi_resolve_explicit_1

[Dmitry Antipov]

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

Eli,

I suspect that this call to CHAR_TO_BYTE in bidi_resolve_explicit_1
is redundant; can you check that?

Thanks,
Dmitry

[-- Attachment #2: bidi_resolve_explicit_1.patch --]
[-- Type: text/plain, Size: 457 bytes --]

=== modified file 'src/bidi.c'
--- src/bidi.c	2013-02-20 05:18:20 +0000
+++ src/bidi.c	2013-03-05 02:01:17 +0000
@@ -1360,8 +1360,8 @@
       else
 	{
 	  if (bidi_it->charpos < BEGV)
-	    bidi_it->charpos = BEGV;
-	  bidi_it->bytepos = CHAR_TO_BYTE (bidi_it->charpos);
+	    bidi_it->charpos = BEGV, bidi_it->bytepos = BEGV_BYTE;
+	  eassert (bidi_it->bytepos == CHAR_TO_BYTE (bidi_it->charpos));
 	}
     }
   /* Don't move at end of buffer/string.  */

Re: CHAR_TO_BYTE in bidi_resolve_explicit_1

[Eli Zaretskii]

> Date: Tue, 05 Mar 2013 06:05:44 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: Emacs development discussions <emacs-devel@gnu.org>
> 
> I suspect that this call to CHAR_TO_BYTE in bidi_resolve_explicit_1
> is redundant; can you check that?

We could use BEGV_BYTE when we limit charpos to BEGV, yes.  But
otherwise, the call to CHAR_TO_BYTE is not redundant, because nothing
else makes sure bidi_it->charpos is synchronized with bidi_it->bytepos
when the display engine re-seats the display iterator.

Did you see some evidence that the call to CHAR_TO_BYTE is redundant
in the general case?  The case of before BEGV is quite rare, so
eliminating CHAR_TO_BYTE there won't give you any speed up, and
CHAR_TO_BYTE is fast at BEGV anyway.

Re: CHAR_TO_BYTE in bidi_resolve_explicit_1

[Dmitry Antipov]

On 03/05/2013 07:51 AM, Eli Zaretskii wrote:

> We could use BEGV_BYTE when we limit charpos to BEGV, yes.  But
> otherwise, the call to CHAR_TO_BYTE is not redundant, because nothing
> else makes sure bidi_it->charpos is synchronized with bidi_it->bytepos
> when the display engine re-seats the display iterator.

AFAICS reseat routines uses bidi_init_it to make sure that BIDI stuff
is synced with the rest of redisplay, isn't it?

> Did you see some evidence that the call to CHAR_TO_BYTE is redundant
> in the general case?

I just tried the patch from the previous e-mail in the very basic editing
session against a few files (including Greek and R2L Arabic scripts), and
eassert was never triggered.

Dmitry

Re: CHAR_TO_BYTE in bidi_resolve_explicit_1

[Eli Zaretskii]

> Date: Tue, 05 Mar 2013 13:30:06 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: emacs-devel@gnu.org
> 
> On 03/05/2013 07:51 AM, Eli Zaretskii wrote:
> 
> > We could use BEGV_BYTE when we limit charpos to BEGV, yes.  But
> > otherwise, the call to CHAR_TO_BYTE is not redundant, because nothing
> > else makes sure bidi_it->charpos is synchronized with bidi_it->bytepos
> > when the display engine re-seats the display iterator.
> 
> AFAICS reseat routines uses bidi_init_it to make sure that BIDI stuff
> is synced with the rest of redisplay, isn't it?

Actually, I see that you are right.  I was speaking from my failing
memory of a situation that has long since changed.

So I installed a slightly more radical change than the one you
suggested.  Thanks.