Redundant if clause in xdisp.c?

Redundant if clause in xdisp.c?

[Eli Zaretskii]

Unless I'm missing something, both branches of the if clause below
(from xdisp.c:redisplay_internal) are identical, except for the
comments:

  		  if (Z - CHARPOS (tlendpos) == ZV)
		    {
		      /* This line ends at end of (accessible part of)
			 buffer.  There is no newline to count.  */
		      delta = (Z
			       - CHARPOS (tlendpos)
			       - MATRIX_ROW_START_CHARPOS (row));
		      delta_bytes = (Z_BYTE
				     - BYTEPOS (tlendpos)
				     - MATRIX_ROW_START_BYTEPOS (row));
		    }
  		  else
		    {
		      /* This line ends in a newline.  Must take
			 account of the newline and the rest of the
			 text that follows.  */
		      delta = (Z
			       - CHARPOS (tlendpos)
			       - MATRIX_ROW_START_CHARPOS (row));
		      delta_bytes = (Z_BYTE
				     - BYTEPOS (tlendpos)
				     - MATRIX_ROW_START_BYTEPOS (row));
		    }

  		  increment_matrix_positions (w->current_matrix,
					      this_line_vpos + 1,
					      w->current_matrix->nrows,
					      delta, delta_bytes);

Okay to remove the condition and to collapse both branches into a
single code fragment?

Re: Redundant if clause in xdisp.c?

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> Unless I'm missing something, both branches of the if clause below
> (from xdisp.c:redisplay_internal) are identical, except for the
> comments: ...
>
> Okay to remove the condition and to collapse both branches into a
> single code fragment?

Yes, please.

Re: Redundant if clause in xdisp.c?

[Stefan Monnier]

> Okay to remove the condition and to collapse both branches into a
> single code fragment?

Looks like a good idea, yes.  BTW, before playing with the redisplay,
please just throw away keyboard.c's handling of self-insert-command,
forward-char and backward-char, as well as the corresponding
"direct-rendering fast-path" code.

These optimizations seem to be pointless nowadays: even when running
Emacs-23 on my cell-phone (FreeRunner) displaying on my desktop, they
don't make any noticeable difference.

I refrained from installing this change for Emacs-23.2, out of fear
bigfoot might come and bite me, but I'll definitely install it for
Emacs-24.


        Stefan

Re: Redundant if clause in xdisp.c?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Sat, 29 Aug 2009 11:50:39 -0400
> 
> > Okay to remove the condition and to collapse both branches into a
> > single code fragment?
> 
> Looks like a good idea, yes.

Done.

> BTW, before playing with the redisplay, please just throw away
> keyboard.c's handling of self-insert-command, forward-char and
> backward-char, as well as the corresponding "direct-rendering
> fast-path" code.

These optimizations are disabled for bidi redisplay anyway, so they
don't bother me.

> These optimizations seem to be pointless nowadays: even when running
> Emacs-23 on my cell-phone (FreeRunner) displaying on my desktop, they
> don't make any noticeable difference.

How did you turn them off to measure their influence, and with what
kind of stuff in your buffer(s)?

> I refrained from installing this change for Emacs-23.2, out of fear
> bigfoot might come and bite me, but I'll definitely install it for
> Emacs-24.

OK, but for now I'd prefer not to do any changes in my bidi sandbox
that are not strictly related to bidirectional editing.  Once we have
a way to commit these changes to CVS, fine with me, if we are sure
these optimizations don't matter anymore.

Re: Redundant if clause in xdisp.c?

[Stefan Monnier]

>> BTW, before playing with the redisplay, please just throw away
>> keyboard.c's handling of self-insert-command, forward-char and
>> backward-char, as well as the corresponding "direct-rendering
>> fast-path" code.

> These optimizations are disabled for bidi redisplay anyway, so they
> don't bother me.

Good.

>> These optimizations seem to be pointless nowadays: even when running
>> Emacs-23 on my cell-phone (FreeRunner) displaying on my desktop, they
>> don't make any noticeable difference.

> How did you turn them off to measure their influence,

I removed the corresponding code.

> and with what kind of stuff in your buffer(s)?

I tried it in *scratch* and in a Python-mode buffer (visiting
/usr/bin/zhone), doing random movement and typing (especially using the
auto-repeat to see when the redisplay keeps up and when it doesn't).

>> I refrained from installing this change for Emacs-23.2, out of fear
>> bigfoot might come and bite me, but I'll definitely install it for
>> Emacs-24.

> OK, but for now I'd prefer not to do any changes in my bidi sandbox
> that are not strictly related to bidirectional editing.  Once we have
> a way to commit these changes to CVS, fine with me, if we are sure
> these optimizations don't matter anymore.

Sure, I just wanted to make sure you don't waste time in these parts of
the code.


        Stefan

Re: Redundant if clause in xdisp.c?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 29 Aug 2009 17:21:42 -0400
> Cc: emacs-devel@gnu.org
> 
> >> These optimizations seem to be pointless nowadays: even when running
> >> Emacs-23 on my cell-phone (FreeRunner) displaying on my desktop, they
> >> don't make any noticeable difference.
> 
> > How did you turn them off to measure their influence,
> 
> I removed the corresponding code.

I think it's enough to set direction-reversed non-nil.

> > and with what kind of stuff in your buffer(s)?
> 
> I tried it in *scratch* and in a Python-mode buffer (visiting
> /usr/bin/zhone), doing random movement and typing (especially using the
> auto-repeat to see when the redisplay keeps up and when it doesn't).

GUI session or tty?  The price of disabling these optimizations might
be more visible on a tty via a relatively slow link.  I don't think we
should remove them before we try at least that.

> Sure, I just wanted to make sure you don't waste time in these parts of
> the code.

The first rule of optimization: don't do it.  The second rule of
optimization: don't do it yet.  I'm somewhere before the first and the
second one: I need to get the code work right before I make it faster
(if needed).