Subject: Re: bug#56682: Fix the long lines font locking related slowdowns

From: Eli Zaretskii <[email-masked]>
Date: Sat, 23 Jul 2022 18:43:09 +0300

> From: Gerd Möllmann <[email-masked]>
> Cc: [email-masked],  [email-masked],  [email-masked]
> Date: Sat, 23 Jul 2022 16:33:28 +0200
> 
> Gerd Möllmann <[email-masked]> writes:
> 
> > Eli Zaretskii <[email-masked]> writes:
> >
> >> And I've found a serious sink of CPU cycles under truncate-lines, and
> >> installed a fix on the feature branch.  Gerd, if you have time to
> >> eyeball the fix and comment on it, I'd appreciate.  It's commit
> >> 350e97d on the branch.  (I can post a more detailed explanation of
> >> what I did and why, if that would help, because the code and the
> >> functions it calls are somewhat tricky.)
> >
> > I'll look at it and come back.
> 
> modified   src/xdisp.c
> @@ -7153,10 +7153,10 @@ forward_to_next_line_start (struct it *it, bool *skipped_p,
>  	  || ((pos = Fnext_single_property_change (make_fixnum (start),
>  						   Qdisplay, Qnil,
>  						   make_fixnum (limit)),
> -	       NILP (pos))
> +	       (NILP (pos) || XFIXNAT (pos) == limit))
>  	      && next_overlay_change (start) == ZV))
>  	{
> -	  if (!it->bidi_p)
> +	  if (!it->bidi_p || !bidi_it_prev)
>  	    {
>  	      IT_CHARPOS (*it) = limit;
>  	      IT_BYTEPOS (*it) = bytepos;
> 
> I understand the first diff, which makes a lot of sense

Thanks, this was the main part.  From what I see, this code never
really worked correctly, since Emacs 21, because
Fnext_single_property_change can never return nil when called like
that.  IOW, we never took the shortcut there, and probably didn't
notice it because 500-character lines are rare.

> but I'm afraid I don't know enough about bidi to be of any help.

The idea is that since the value of BIDI_IT_PREV is never used by the
caller when ON_NEWLINE_P is zero, we don't need to compute it when
ON_NEWLINE_P is zero, something that would have precluded us from
taking the shortcut.  Not taking the shortcut is very expensive when
we have very long lines, of course.

Btw, one quirk of the code in forward_to_next_line_start (which took
me some time to take in and convince myself it's okay) is that it
returns non-zero when it didn't find any new newlines till ZV.  This
is not very clean, but I guess we don't care in that case, since the
iterator will be moved to ZV, and from there there's not much we need
to do...