GNU bug report logs - #56682
Fix the long lines font locking related slowdowns

Previous Next

Full log

Message #2323 received at 56682 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 56682 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked
 locked narrowing.
Date: Sat, 01 Apr 2023 14:26:50 +0000

Thanks for your review!

>
> They also change the code in non-trivial ways, and I'm not thrilled 
> about changing code in those parts at this late stage.
>

Two commits do change the code, yes, to fix (cursor motion command with 
C-n / C-p) bugs.  That code is only used for buffers with long lines.  It 
would be most regrettable to release Emacs 29 without these bugs fixed.

>
> Specific comments:
>
> afc2c6c13c:
>
> . Misses an optimization opportunity: where pos == BEGV, you can use 
> BEGV_BYTE instead of calling CHAR_TO_BYTE.
>

Okay, thanks, added in dce08cf05c.

>
> . I'm not sure I'm happy about calling find_newline in a loop where 
> previous code just made a trivial calculation.  Imagine a buffer with a 
> lot of short lines.  What problem exactly is being solved here, and how 
> does this change solve it?
>

The point is to find the buffer position of BOL.  But you're right, there 
is a missed optimization here, which I just added (also in dce08cf05c). 
Now the code searches for that position in [pos-500..pos], 
[pos-5500..pos-500], [pos-55500..pos-5500], [pos-555500..pos-55500], in 
that order, and buffers with lots of short lines (or more precisely: 
buffers with lots of short lines _and_ one or more long lines) are not 
negatively affected by that code anymore.

>
> 2093e010dc:
>
> . Why such a strange method of finding out whether we are on a TTY 
> frame?  The usual method is FRAME_WINDOW_P (XFRAME (w->frame)).
>

That's what I've been using since that function was introduced six months 
ago or so.  I admit I don't remember why that's what I chose.  If you tell 
me that using FRAME_WINDOW_P (XFRAME (w->frame)) has the same effect as 
terminal-live-p == t (and indeed after looking at the code ISTM that 
that's the case), I'll replace that.

>
> . The continuation glyph can be present not only on TTY frames, but also 
> on GUI frames when one or both of the fringes are disabled, so the test 
> needs to be augmented.  I think you need to use WINDOW_LEFT_FRINGE_WIDTH 
> and WINDOW_RIGHT_FRINGE_WIDTH.
>

Thanks, I did not realize that.  I just did that (again in dce08cf05c), 
but I'm not sure how WINDOW_LEFT_FRINGE_WIDTH should be used.  Using 
WINDOW_RIGHT_FRINGE_WIDTH seems enough, but I'm probably missing 
something.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.