bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Andrey Listopadov]

I'm using emacs with `truncate-lines` set to `t` in my init.el.  After
the merge of #56393 I've rebuilt Emacs, and started testing the fix.
I've noticed, that when lines are truncated, opening dictionary.json
from the #45898 and pressing C-e doesn't move you to the end of the
line, but stops somewhere inside it.  E.g. with the default window width
of Emacs -Q, I can see the following portion of the line

←, "hu": ["index"]}, "examples": [["en C la indicoj aperas inter ortajkrampoj, →

Arrows shown in the fringe indicate that there are more to the line on
the right side.

As a side question, does these optimizations work with truncated lines?
As far as I can tell Emacs is not as responsive as when the lines are
wrapped, but I don't know how to measure this precisely.

In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.17.4)
 of 2022-07-21 built on toolbox
Repository revision: 4a4fcf628e1e4c8db47cd62fa5617b662fa8b5d6
Repository branch: master
System Description: Fedora Linux 36 (Workstation Edition)

Configured using:
 'configure --without-compress-install --with-native-compilation
 --with-pgtk --with-mailutils --prefix=/home/alist/.local'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY
PDUMPER PGTK PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS XIM
GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(comp comp-cstr warnings rx cl-seq cl-macs gv cl-extra bytecomp
byte-compile cconv help-mode pp shadow sort mail-extr emacsbug message
mailcap yank-media puny dired dired-loaddefs rfc822 mml mml-sec
password-cache epa derived epg rfc6068 epg-config gnus-util
text-property-search time-date subr-x mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs cl-lib
sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils rmc
iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/pgtk-win pgtk-win term/common-win
pgtk-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list
replace newcomment text-mode lisp-mode prog-mode register page tab-bar
menu-bar rfn-eshadow isearch easymenu timer select scroll-bar mouse
jit-lock font-lock syntax font-core term/tty-colors frame minibuffer
nadvice seq simple cl-generic indonesian philippine cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite emoji-zwj charscript charprop
case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
gtk pgtk lcms2 multi-tty make-network-process native-compile emacs)

Memory information:
((conses 16 82646 9587)
 (symbols 48 7091 6)
 (strings 32 19214 1773)
 (string-bytes 1 656752)
 (vectors 16 15347)
 (vector-slots 8 284088 17444)
 (floats 8 173 66)
 (intervals 56 698 7)
 (buffers 992 13))

--
Andrey Listopadov

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gregory Heytings]

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


>
> I'm using emacs with `truncate-lines` set to `t` in my init.el.  After 
> the merge of #56393 I've rebuilt Emacs, and started testing the fix. 
> I've noticed, that when lines are truncated, opening dictionary.json 
> from the #45898 and pressing C-e doesn't move you to the end of the 
> line, but stops somewhere inside it.  E.g. with the default window width 
> of Emacs -Q, I can see the following portion of the line
>
> ←, "hu": ["index"]}, "examples": [["en C la indicoj aperas interortajkrampoj, →
>
> Arrows shown in the fringe indicate that there are more to the line on 
> the right side.
>

You would have seen the same bug before the merge, so it's entirely 
unrelated to the merge.  It is present at least in Emacs 24, 25, 26, 27 
and 28.

>
> As a side question, does these optimizations work with truncated lines? 
> As far as I can tell Emacs is not as responsive as when the lines are 
> wrapped, but I don't know how to measure this precisely.
>

They do work with truncated lines, but I suspect that truncated lines 
require specific fixes to work efficiently.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> Cc: 56683@debbugs.gnu.org
> Date: Thu, 21 Jul 2022 19:45:19 +0000
> From: Gregory Heytings <gregory@heytings.org>
> 
> > I'm using emacs with `truncate-lines` set to `t` in my init.el.  After 
> > the merge of #56393 I've rebuilt Emacs, and started testing the fix. 
> > I've noticed, that when lines are truncated, opening dictionary.json 
> > from the #45898 and pressing C-e doesn't move you to the end of the 
> > line, but stops somewhere inside it.  E.g. with the default window width 
> > of Emacs -Q, I can see the following portion of the line
> >
> > ←, "hu": ["index"]}, "examples": [["en C la indicoj aperas interortajkrampoj, →
> >
> > Arrows shown in the fringe indicate that there are more to the line on 
> > the right side.
> >
> 
> You would have seen the same bug before the merge, so it's entirely 
> unrelated to the merge.  It is present at least in Emacs 24, 25, 26, 27 
> and 28.

Is this related to the below (from xdisp.c)?

  #define DISP_INFINITY 10000000

Maybe we should make "infinity" somewhat larger?  dictionary.json has
a single line that is 18922365 characters long, so it's "more than
infinity".

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gregory Heytings]

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


>
> Is this related to the below (from xdisp.c)?
>
> #define DISP_INFINITY 10000000
>
> Maybe we should make "infinity" somewhat larger?  dictionary.json has a 
> single line that is 18922365 characters long, so it's "more than 
> infinity".
>

It is indeed, setting DISP_INFINITY to 1000000000 "fixes" that bug. 
But... how infinite is infinite?  (A near-philosophical question 😉) 
With a somewhat larger file (that one is only 20 MB) we'd have to increase 
it again.  If that's a hard-coded limit, I guess it's a sign that we 
should advise against using toggle-truncate-lines in buffer with long 
lines.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Andrey Listopadov]

Gregory Heytings <gregory@heytings.org> writes:

> You would have seen the same bug before the merge, so it's entirely
> unrelated to the merge.  It is present at least in Emacs 24, 25, 26,
> 27 and 28.

Oh, well.  Then it seems that I've simply never noticed this before,
perhaps because Emacs couldn't handle such long lines.  Feel free to
close this then, if this is intended/known.

> They do work with truncated lines, but I suspect that truncated lines
> require specific fixes to work efficiently.

Thanks!  I've asked because It's important to me that Emacs is
responsive if I open a log file with really long lines. Truncated lines
make such logs much easier to read, as I can compare lines above and
below the very long one, without scrolling.

--
Andrey Listopadov

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gregory Heytings]

Thanks for your comments.

>> They do work with truncated lines, but I suspect that truncated lines 
>> require specific fixes to work efficiently.
>
> Thanks!  I've asked because It's important to me that Emacs is 
> responsive if I open a log file with really long lines. Truncated lines 
> make such logs much easier to read, as I can compare lines above and 
> below the very long one, without scrolling.
>

How long is a "really long line" in a log file?  Do you think 1000 or 
10000 characters is a reasonable upper limit for your use case?

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Andrey Listopadov]

> How long is a "really long line" in a log file?  Do you think 1000 or
> 10000 characters is a reasonable upper limit for your use case?

It depends, occasionally there are lines with JSONs maybe one third of
the size of the mentioned dictionary.json.  But these are rare cases,
most of the time the longest lines are about 3000 characters wide, I
guess I have to measure it next time.  So probably a smaller amount than
10000 to enable optimizations is welcome.

One particularly interesting situation is when there are logs which span
such long lines on every line, e.g.:

   1 log-line-with-1800-chars→
   2 log-line-with-3000-chars→
   3 log-line-with-2000-chars→
   4 log-line-with-1500-chars→
     ...
1830 log-line-with-1500-chars

In this case, if I wasn't using truncation I would see only the first
line of the log, wrapped around and taking all screen space, but since
lines are truncated, I have ~70 very long lines displayed on screen.

I've taken dictionary.json, and used C-u 70 M-x duplicate-line, and it
does seem to perform well (or at least Emacs doesn't hang completely),
but there are some occasional slowdowns (Probably because of
show-paren-mode).

--
Andrey Listopadov

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> Date: Thu, 21 Jul 2022 20:15:55 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: andreyorst@gmail.com, 56683@debbugs.gnu.org
> 
> > Is this related to the below (from xdisp.c)?
> >
> > #define DISP_INFINITY 10000000
> >
> > Maybe we should make "infinity" somewhat larger?  dictionary.json has a 
> > single line that is 18922365 characters long, so it's "more than 
> > infinity".
> 
> It is indeed, setting DISP_INFINITY to 1000000000 "fixes" that bug. 
> But... how infinite is infinite?  (A near-philosophical question 😉) 
> With a somewhat larger file (that one is only 20 MB) we'd have to increase 
> it again.  If that's a hard-coded limit, I guess it's a sign that we 
> should advise against using toggle-truncate-lines in buffer with long 
> lines.

We could enlarge the value to INT_MAX, or even make the X-coordinate
members of the iterator structure have the type ptrdiff_t and then
DISP_INFINITY could be PTRDIFF_MAX (which would be undesirable in
general for performance reasons).  But that would not solve the
problem completely, because the value is in pixels.  So with the
largest possible line length, which is EMACS_INT_MAX characters long,
we'd need at least 7 or 8 times that for the X-coordinate values, and
we don't have that even in 64-bit builds.

If we want to advise against truncate-lines in such buffers, perhaps
toggle-truncate-lines should ask for confirmation in such cases, and
the documentation should mention the limitation.

Gerd, any ideas or comments?

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> Cc: 56683@debbugs.gnu.org
> Date: Thu, 21 Jul 2022 19:45:19 +0000
> From: Gregory Heytings <gregory@heytings.org>
> 
> > As a side question, does these optimizations work with truncated lines? 
> > As far as I can tell Emacs is not as responsive as when the lines are 
> > wrapped, but I don't know how to measure this precisely.
> 
> They do work with truncated lines, but I suspect that truncated lines 
> require specific fixes to work efficiently.

I think we should take look at this fragment from hscroll_window_tree:

	      /* Move iterator to pt starting at cursor_row->start in
		 a line with infinite width.  */
	      init_to_row_start (&it, w, cursor_row);
	      if (hscl)
		it.first_visible_x = window_hscroll_limited (w, it.f)
				     * FRAME_COLUMN_WIDTH (it.f);
	      it.last_visible_x = DISP_INFINITY;
	      move_it_in_display_line_to (&it, pt, -1, MOVE_TO_POS);
	      /* If the line ends in an overlay string with a newline,
		 we might infloop, because displaying the window will
		 want to put the cursor after the overlay, i.e. at X
		 coordinate of zero on the next screen line.  So we
		 use the buffer position prior to the overlay string
		 instead.  */
	      if (it.method == GET_FROM_STRING && pt > 1)
		{
		  init_to_row_start (&it, w, cursor_row);
		  if (hscl)
		    it.first_visible_x = (window_hscroll_limited (w, it.f)
					  * FRAME_COLUMN_WIDTH (it.f));
		  move_it_in_display_line_to (&it, pt - 1, -1, MOVE_TO_POS);
		}

Specifically, init_to_row_start can potentially start from a very far
away buffer position, especially in buffers with a single long line,
and AFAICT the subroutines called by init_to_row_start aren't
restricted to the "narrowing" for long lines, so they go to BOB in
this case.  The problem here is that this code handles automatic
hscrolling, so any changes to it need to be careful not to destroy the
use case of moving point horizontally to a position that is beyond the
right or left window edge (which is what auto-hscroll is for).

Alternatively, maybe automatic hscrolling should be disabled in such
buffers (or we should advise that), and then we should have convenient
commands to hscroll manually so as to bring point into the view.
(Maybe we even have such commands already, I don't know.)

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gregory Heytings]

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


Thanks again for your comments.

>
> It depends, occasionally there are lines with JSONs maybe one third of 
> the size of the mentioned dictionary.json.  But these are rare cases, 
> most of the time the longest lines are about 3000 characters wide, I 
> guess I have to measure it next time.  So probably a smaller amount than 
> 10000 to enable optimizations is welcome.
>

FTR, in buffers with lines that are "only" 3000 characters wide, long line 
optimizations are not turned on by default (see long-line-threshold, which 
is set to 10000 by default), such buffers should behave "well enough" 
without them.

>
> One particularly interesting situation is when there are logs which span 
> such long lines on every line, e.g.:
>
>   1 log-line-with-1800-chars→
>   2 log-line-with-3000-chars→
>   3 log-line-with-2000-chars→
>   4 log-line-with-1500-chars→
>     ...
> 1830 log-line-with-1500-chars
>
> In this case, if I wasn't using truncation I would see only the first 
> line of the log, wrapped around and taking all screen space, but since 
> lines are truncated, I have ~70 very long lines displayed on screen.
>

I understand.  It's an interesting use case.

>
> I've taken dictionary.json, and used C-u 70 M-x duplicate-line, and it 
> does seem to perform well (or at least Emacs doesn't hang completely), 
> but there are some occasional slowdowns (Probably because of 
> show-paren-mode).
>

At the moment I think it would be better to advise against using truncated 
lines for such pathological files.  As Eli just said, it would probably be 
rather complex to remove the hard-coded limit in the Emacs display engine.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Thu, 21 Jul 2022 20:15:55 +0000
>> From: Gregory Heytings <gregory@heytings.org>
>> cc: andreyorst@gmail.com, 56683@debbugs.gnu.org
>> 
>> > Is this related to the below (from xdisp.c)?
>> >
>> > #define DISP_INFINITY 10000000
>> >
>> > Maybe we should make "infinity" somewhat larger?  dictionary.json has a 
>> > single line that is 18922365 characters long, so it's "more than 
>> > infinity".
>> 
>> It is indeed, setting DISP_INFINITY to 1000000000 "fixes" that bug. 
>> But... how infinite is infinite?  (A near-philosophical question 😉)

Don't think too much about it, it didn't do good to Georg Cantor.  BTW,
Omega would be a really cool name for that INFINITY; the smallest
position beyond all finite x.

>> With a somewhat larger file (that one is only 20 MB) we'd have to increase 
>> it again.  If that's a hard-coded limit, I guess it's a sign that we 
>> should advise against using toggle-truncate-lines in buffer with long 
>> lines.
>
> We could enlarge the value to INT_MAX, or even make the X-coordinate
> members of the iterator structure have the type ptrdiff_t and then
> DISP_INFINITY could be PTRDIFF_MAX (which would be undesirable in
> general for performance reasons).  But that would not solve the
> problem completely, because the value is in pixels.  So with the
> largest possible line length, which is EMACS_INT_MAX characters long,
> we'd need at least 7 or 8 times that for the X-coordinate values, and
> we don't have that even in 64-bit builds.
>
> If we want to advise against truncate-lines in such buffers, perhaps
> toggle-truncate-lines should ask for confirmation in such cases, and
> the documentation should mention the limitation.
>
> Gerd, any ideas or comments?

I had to look around a bit, because I didn't remember that constant.  It
seems to have been introduced for horizontal scroll bars, I think.

If we really want infinity, how about setting last_visible_x to -1 in
that case.  But that requires checking/changing some places where
the iterator pisition is tested against last_visible_x.

I think that's okay.  WDYT?

Your "ticks" check kicks in should be burn to much time because of the
infinite last x, right?

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> Gerd, any ideas or comments?

Forgot one thing:  There seem to be a place in dispextern.h where
last_visible_x is set to the number 1000000.  That's odd.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> I think we should take look at this fragment from hscroll_window_tree:
>
> 	      /* Move iterator to pt starting at cursor_row->start in
> 		 a line with infinite width.  */
> 	      init_to_row_start (&it, w, cursor_row);
> 	      if (hscl)
> 		it.first_visible_x = window_hscroll_limited (w, it.f)
> 				     * FRAME_COLUMN_WIDTH (it.f);
> 	      it.last_visible_x = DISP_INFINITY;
> 	      move_it_in_display_line_to (&it, pt, -1, MOVE_TO_POS);
> 	      /* If the line ends in an overlay string with a newline,
> 		 we might infloop, because displaying the window will
> 		 want to put the cursor after the overlay, i.e. at X
> 		 coordinate of zero on the next screen line.  So we
> 		 use the buffer position prior to the overlay string
> 		 instead.  */
> 	      if (it.method == GET_FROM_STRING && pt > 1)
> 		{
> 		  init_to_row_start (&it, w, cursor_row);
> 		  if (hscl)
> 		    it.first_visible_x = (window_hscroll_limited (w, it.f)
> 					  * FRAME_COLUMN_WIDTH (it.f));
> 		  move_it_in_display_line_to (&it, pt - 1, -1, MOVE_TO_POS);
> 		}
>
> Specifically, init_to_row_start can potentially start from a very far
> away buffer position, especially in buffers with a single long line,

I'm not sure. Cursor_row comes from here:

	  if (w->cursor.vpos < bottom_row - w->desired_matrix->rows)
	    cursor_row = MATRIX_ROW (w->desired_matrix, w->cursor.vpos);
	  else
	    cursor_row = bottom_row - 1;

	  if (!cursor_row->enabled_p)
	    {
	      bottom_row = MATRIX_BOTTOM_TEXT_ROW (w->current_matrix, w);
	      if (w->cursor.vpos < bottom_row - w->current_matrix->rows)
		cursor_row = MATRIX_ROW (w->current_matrix, w->cursor.vpos);
	      else
		cursor_row = bottom_row - 1;
	    }

In my own words, either cursor_row has just been produced in the
desired_matrix, or it has been produced previously, and is now part of
the current matrix.

Doesn't that mean it was already subject to the iterator narrowing?

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Gregory Heytings <gregory@heytings.org>,  andreyorst@gmail.com,
>   56683@debbugs.gnu.org
> Date: Fri, 22 Jul 2022 08:51:56 +0200
> 
> If we really want infinity, how about setting last_visible_x to -1 in
> that case.  But that requires checking/changing some places where
> the iterator pisition is tested against last_visible_x.

I don't really see how this could help in general, because the
current_x member (what you call the "iterator position") will still
overflow at some point, and we can no longer do any layout decisions
for the X coordinate after that.

Or what am I missing?

> Your "ticks" check kicks in should be burn to much time because of the
> infinite last x, right?

Sorry, I don't think I understand what you are asking here.  Please
elaborate or rephrase.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Gregory Heytings <gregory@heytings.org>,  andreyorst@gmail.com,
>   56683@debbugs.gnu.org
> Date: Fri, 22 Jul 2022 08:53:33 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Gerd, any ideas or comments?
> 
> Forgot one thing:  There seem to be a place in dispextern.h where
> last_visible_x is set to the number 1000000.  That's odd.

That's a different kind of "infinity" ;-)

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> If we really want infinity, how about setting last_visible_x to -1 in
>> that case.  But that requires checking/changing some places where
>> the iterator pisition is tested against last_visible_x.
>
> I don't really see how this could help in general, because the
> current_x member (what you call the "iterator position") will still
> overflow at some point, and we can no longer do any layout decisions
> for the X coordinate after that.
>
> Or what am I missing?

You were asking for a good value of DISP_OMEGA, that's at least what I
understood.  My answer is -1, plus code changes.

That current_x can overflow is something completely different, IMO.  But
now I'm cautious enough to ask what do you mean?  Overflowing which
bound?  I read this as integer overflow, integer = the data type.

>
>> Your "ticks" check kicks in should be burn to much time because of the
>> infinite last x, right?
>
> Sorry, I don't think I understand what you are asking here.  Please
> elaborate or rephrase.

The thing you implemented in set_iterator_to_next we talked about.

  if (max_redisplay_ticks > 0)
    update_redisplay_ticks (1, it->w);

I don't remember the offical name ATM.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> Forgot one thing:  There seem to be a place in dispextern.h where
>> last_visible_x is set to the number 1000000.  That's odd.
>
> That's a different kind of "infinity" ;-)

Aleph1 :-O.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: andreyorst@gmail.com,  56683@debbugs.gnu.org,  gregory@heytings.org
> Date: Fri, 22 Jul 2022 09:53:33 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Forgot one thing:  There seem to be a place in dispextern.h where
> >> last_visible_x is set to the number 1000000.  That's odd.
> >
> > That's a different kind of "infinity" ;-)
> 
> Aleph1 :-O.

Well, what I meant was that it's used for other purposes than
DISP_INFINITY.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: andreyorst@gmail.com,  56683@debbugs.gnu.org,  gregory@heytings.org
> Date: Fri, 22 Jul 2022 09:52:16 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> >> If we really want infinity, how about setting last_visible_x to -1 in
> >> that case.  But that requires checking/changing some places where
> >> the iterator pisition is tested against last_visible_x.
> >
> > I don't really see how this could help in general, because the
> > current_x member (what you call the "iterator position") will still
> > overflow at some point, and we can no longer do any layout decisions
> > for the X coordinate after that.
> >
> > Or what am I missing?
> 
> You were asking for a good value of DISP_OMEGA, that's at least what I
> understood.  My answer is -1, plus code changes.
> 
> That current_x can overflow is something completely different, IMO.  But
> now I'm cautious enough to ask what do you mean?  Overflowing which
> bound?  I read this as integer overflow, integer = the data type.

Yes, integer overflow.  This code:

	      init_to_row_start (&it, w, cursor_row);
	      if (hscl)
		it.first_visible_x = window_hscroll_limited (w, it.f)
				     * FRAME_COLUMN_WIDTH (it.f);
	      it.last_visible_x = DISP_INFINITY;
	      move_it_in_display_line_to (&it, pt, -1, MOVE_TO_POS);

will keep producing glyphs inside move_it_in_display_line_to until
it gets to the position of point.  While producing glyphs,
it.current_x is incremented by the pixel-width of each produced
glyph.  After a large enough number of produced glyphs, it.current_x
will overflow INT_MAX.

> >> Your "ticks" check kicks in should be burn to much time because of the
> >> infinite last x, right?
> >
> > Sorry, I don't think I understand what you are asking here.  Please
> > elaborate or rephrase.
> 
> The thing you implemented in set_iterator_to_next we talked about.
> 
>   if (max_redisplay_ticks > 0)
>     update_redisplay_ticks (1, it->w);

I understood that part, but not the "should be burn to much time
because of the infinite last x" part, even if I replace "to" with
"too".

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>
> Yes, integer overflow.  This code:
>
> 	      init_to_row_start (&it, w, cursor_row);
> 	      if (hscl)
> 		it.first_visible_x = window_hscroll_limited (w, it.f)
> 				     * FRAME_COLUMN_WIDTH (it.f);
> 	      it.last_visible_x = DISP_INFINITY;
> 	      move_it_in_display_line_to (&it, pt, -1, MOVE_TO_POS);
>
> will keep producing glyphs inside move_it_in_display_line_to until
> it gets to the position of point.  While producing glyphs,
> it.current_x is incremented by the pixel-width of each produced
> glyph.  After a large enough number of produced glyphs, it.current_x
> will overflow INT_MAX.

Right.  I am not worried about that:

It takes a lot of iterations until the ints overflow.  The largest
positive 32-bit integer is very roughly 2 * 10^9.  With a font width of
of 10 pixels, that would mean 2 * 10^8 characters (200 Mb), and so on
with wider fonts.

Se that being said, what I orignally wanted to ask, was:

>> The thing you implemented in set_iterator_to_next we talked about.
>> 
>>   if (max_redisplay_ticks > 0)
>>     update_redisplay_ticks (1, it->w);
>
> I understood that part, but not the "should be burn to much time
> because of the infinite last x" part, even if I replace "to" with
> "too".

Sorry for the typo: s/be/we, i.e. "if we burn...".  What I meant to ask
is if the check above doesn't lead to a signal anyway long before we
reach the overflow.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: andreyorst@gmail.com,  56683@debbugs.gnu.org,  gregory@heytings.org
> Date: Fri, 22 Jul 2022 14:56:22 +0200
> 
> > will keep producing glyphs inside move_it_in_display_line_to until
> > it gets to the position of point.  While producing glyphs,
> > it.current_x is incremented by the pixel-width of each produced
> > glyph.  After a large enough number of produced glyphs, it.current_x
> > will overflow INT_MAX.
> 
> Right.  I am not worried about that:
> 
> It takes a lot of iterations until the ints overflow.  The largest
> positive 32-bit integer is very roughly 2 * 10^9.  With a font width of
> of 10 pixels, that would mean 2 * 10^8 characters (200 Mb), and so on
> with wider fonts.

We already have an example of a 19-MB file which is a single line.  It
is within an order of magnitude of the limit you described.  JSON
files in particular are known to be like that: long and without
newlines.

So I think the danger is real.

> >>   if (max_redisplay_ticks > 0)
> >>     update_redisplay_ticks (1, it->w);
> >
> > I understood that part, but not the "should be burn to much time
> > because of the infinite last x" part, even if I replace "to" with
> > "too".
> 
> Sorry for the typo: s/be/we, i.e. "if we burn...".  What I meant to ask
> is if the check above doesn't lead to a signal anyway long before we
> reach the overflow.

That feature is disabled by default.  If enabled, then yes, it will
most probably fire before that.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> That feature is disabled by default.  If enabled, then yes, it will
> most probably fire before that.

Could we enabled it automatically when we switch on iterator narrowing?

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: andreyorst@gmail.com,  56683@debbugs.gnu.org,  gregory@heytings.org
> Date: Fri, 22 Jul 2022 15:54:07 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > That feature is disabled by default.  If enabled, then yes, it will
> > most probably fire before that.
> 
> Could we enabled it automatically when we switch on iterator narrowing?

Technically, sure.  I'm not yet convinced we should, though, because
it isn't clear to me what problems, if any, will be left after the
narrowing idea is used as fully as practical.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gregory Heytings]

By the way, we should also decide what to do when the user enables 
truncate-lines in a buffer with "really too long" lines.  With 
DISP_INFINITY == 10000000 (its current value), and with a character pixel 
width of 20 (a not uncommon value nowadays with HiDPI screens), lines 
longer than 500000 (single-width) characters will not be displayed 
correctly.  With a larger character pixel width, or with characters 
occupying more than one column, that would be even less.

Given:

1. that there has not been a single bug report about the fact that 
truncate-line does not work correctly with "really too long" lines in the 
last 20 years,

2. that the limit above, caused by DISP_INFINITY, is not much more than 
10000, the default value for long-line-threshold, and

3. the remaining slowdowns caused by truncate-lines,

I'd be in favor of simply disabling truncate-lines when 
long_line_optimizations_p is set.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Andrey Listopadov]

> there has not been a single bug report about the fact that truncate-line does not work correctly with "really too long" lines in the last 20 years,

I imagine there's not that big of a percent of Emacs users who open files
with a really long lines (given the issues with these), and for all of those
there's an even smaller percent of those who use truncate-lines (and
even smaller percent of those who would submit a report, perhaps).

Overall, I'd like not to lose the ability to view long lines in such logs with truncated lines enabled, because as I've said earlier, it allows me to partially compare line to above and below ones, without scrolling. And I rarely even scroll to the right that much in such logs, as there's just way too much information to be viewed in a single horizontal line.

If, however nothing will prevent me from enabling truncating even if optimizations are in place, I'm OK with disabling truncating by default.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gregory Heytings]

>
> Overall, I'd like not to lose the ability to view long lines in such 
> logs with truncated lines enabled, because as I've said earlier, it 
> allows me to partially compare line to above and below ones, without 
> scrolling. And I rarely even scroll to the right that much in such logs, 
> as there's just way too much information to be viewed in a single 
> horizontal line.
>

As far as I understand, for your use case disabling truncate-lines when 
the buffer contains long lines would not change anything: you said that in 
your case the long lines are never longer than 10000 characters, which is 
the (default) limit of long-line-threshold.  Should you ever encounter a 
slightly longer line, you could always adapt long-line-threshold 
accordingly.  The point is that there's no way to make truncate-lines work 
with arbitrarily long lines, say a 1 GB long line.

(A side note: if you rarely scroll to the right that much, you can also 
truncate the lines with an external tool like "cut", either before you 
open the file in Emacs, or inside Emacs with C-x h M-| cut -c-1000 RET.)

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> Date: Mon, 25 Jul 2022 22:22:22 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Gerd Möllmann <gerd.moellmann@gmail.com>, 
>     andreyorst@gmail.com, 56683@debbugs.gnu.org
> 
> By the way, we should also decide what to do when the user enables 
> truncate-lines in a buffer with "really too long" lines.  With 
> DISP_INFINITY == 10000000 (its current value), and with a character pixel 
> width of 20 (a not uncommon value nowadays with HiDPI screens), lines 
> longer than 500000 (single-width) characters will not be displayed 
> correctly.  With a larger character pixel width, or with characters 
> occupying more than one column, that would be even less.
> 
> Given:
> 
> 1. that there has not been a single bug report about the fact that 
> truncate-line does not work correctly with "really too long" lines in the 
> last 20 years,

This might be because no one seriously visited such files in Emacs
until now.

> 2. that the limit above, caused by DISP_INFINITY, is not much more than 
> 10000, the default value for long-line-threshold, and
> 
> 3. the remaining slowdowns caused by truncate-lines,
> 
> I'd be in favor of simply disabling truncate-lines when 
> long_line_optimizations_p is set.

Maybe optionally.  Or maybe displaying a message/warning suggesting
that.  I don't like disabling truncate-lines unconditionally in such
buffers, I prefer leaving that to the user.

And I think we should use INT_MAX instead of DISP_INFINITY, for the
purpose of auto-hscroll.  (There are several other uses of that macro,
but I didn't yet look at all of them to see if it would make sense to
increase DISP_INFINITY itself.)

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> Date: Tue, 26 Jul 2022 07:32:55 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Eli Zaretskii <eliz@gnu.org>, 
>     Gerd Möllmann <gerd.moellmann@gmail.com>, 
>     56683@debbugs.gnu.org
> 
> (A side note: if you rarely scroll to the right that much, you can also 
> truncate the lines with an external tool like "cut", either before you 
> open the file in Emacs, or inside Emacs with C-x h M-| cut -c-1000 RET.)

If you never scroll such windows too much to the right, the problems
with "finite infinity" won't show at all, AFAICT, so there's no need
to use 'cut' or its ilk.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gregory Heytings]

>
> Maybe optionally.  Or maybe displaying a message/warning suggesting 
> that.  I don't like disabling truncate-lines unconditionally in such 
> buffers, I prefer leaving that to the user.
>

The user would still have the choice to disable the automatic disabling by 
setting long-line-threshold to nil.

>
> And I think we should use INT_MAX instead of DISP_INFINITY, for the 
> purpose of auto-hscroll.  (There are several other uses of that macro, 
> but I didn't yet look at all of them to see if it would make sense to 
> increase DISP_INFINITY itself.)
>

But you said earlier that this was undesirable because it would affect 
performance?

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> Date: Tue, 26 Jul 2022 12:17:33 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: gerd.moellmann@gmail.com, andreyorst@gmail.com, 56683@debbugs.gnu.org
> 
> > Maybe optionally.  Or maybe displaying a message/warning suggesting 
> > that.  I don't like disabling truncate-lines unconditionally in such 
> > buffers, I prefer leaving that to the user.
> 
> The user would still have the choice to disable the automatic disabling by 
> setting long-line-threshold to nil.

Yes, I understand.  I still don't like it very much.  But if many
users will complain, we might change that before Emacs 29 is released.

> > And I think we should use INT_MAX instead of DISP_INFINITY, for the 
> > purpose of auto-hscroll.  (There are several other uses of that macro, 
> > but I didn't yet look at all of them to see if it would make sense to 
> > increase DISP_INFINITY itself.)
> 
> But you said earlier that this was undesirable because it would affect 
> performance?

No, I said making the type of the struct member current_x a ptrdiff_t
or intmax_t will affect performance.  The above doesn't propose any
such changes, it just enlarges the "infinity" value.  With that value,
lines up to (/ (expt 2 31) 20) => 107374182 (i.e. about 107 million
characters) per line will still display correctly with truncate-lines
when hscrolling is required.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gregory Heytings]

>>> Maybe optionally.  Or maybe displaying a message/warning suggesting 
>>> that.  I don't like disabling truncate-lines unconditionally in such 
>>> buffers, I prefer leaving that to the user.
>>
>> The user would still have the choice to disable the automatic disabling 
>> by setting long-line-threshold to nil.
>
> Yes, I understand.  I still don't like it very much.  But if many users 
> will complain, we might change that before Emacs 29 is released.
>

Do I understand correctly that it's okay to do that on the feature (and 
later master) branch, and that it will be perhaps be revisited later?

>
> No, I said making the type of the struct member current_x a ptrdiff_t or 
> intmax_t will affect performance.  The above doesn't propose any such 
> changes, it just enlarges the "infinity" value.  With that value, lines 
> up to (/ (expt 2 31) 20) => 107374182 (i.e. about 107 million 
> characters) per line will still display correctly with truncate-lines 
> when hscrolling is required.
>

Sorry, I misunderstood what you said.  Now it's much clearer, indeed. 
But still, as you say, that puts a limit at around 100 MB, which isn't 
"that much" nowadays.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> Date: Tue, 26 Jul 2022 12:50:51 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: gerd.moellmann@gmail.com, andreyorst@gmail.com, 56683@debbugs.gnu.org
> 
> >>> Maybe optionally.  Or maybe displaying a message/warning suggesting 
> >>> that.  I don't like disabling truncate-lines unconditionally in such 
> >>> buffers, I prefer leaving that to the user.
> >>
> >> The user would still have the choice to disable the automatic disabling 
> >> by setting long-line-threshold to nil.
> >
> > Yes, I understand.  I still don't like it very much.  But if many users 
> > will complain, we might change that before Emacs 29 is released.
> 
> Do I understand correctly that it's okay to do that on the feature (and 
> later master) branch, and that it will be perhaps be revisited later?

I'd prefer not to do it just yet, and wait for more user feedback
(once the feature branch is merged, which I guess will be soon).

> > No, I said making the type of the struct member current_x a ptrdiff_t or 
> > intmax_t will affect performance.  The above doesn't propose any such 
> > changes, it just enlarges the "infinity" value.  With that value, lines 
> > up to (/ (expt 2 31) 20) => 107374182 (i.e. about 107 million 
> > characters) per line will still display correctly with truncate-lines 
> > when hscrolling is required.
> 
> Sorry, I misunderstood what you said.  Now it's much clearer, indeed. 
> But still, as you say, that puts a limit at around 100 MB, which isn't 
> "that much" nowadays.

Yes, it's not a solution, just a band-aid.  But it costs almost
nothing, and we get to push the problematic situations farther.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gregory Heytings]

>> Do I understand correctly that it's okay to do that on the feature (and 
>> later master) branch, and that it will be perhaps be revisited later?
>
> I'd prefer not to do it just yet, and wait for more user feedback (once 
> the feature branch is merged, which I guess will be soon).
>

Wouldn't it be easier to get useful user feedback if that automatic 
disabling was present on master?  If it's absent, what kind of user 
feedback can we expect to decide what is better for Emacs 29?

>> But still, as you say, that puts a limit at around 100 MB, which isn't 
>> "that much" nowadays.
>
> Yes, it's not a solution, just a band-aid.  But it costs almost nothing, 
> and we get to push the problematic situations farther.
>

It does indeed, but alas the fact that displaying such buffers is 
noticeably slower with truncate-lines remains.  I could perhaps take a 
look (after finalizing and merging the current branch), but I'm not really 
sure it's worth the price.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Eli Zaretskii]

> Date: Tue, 26 Jul 2022 13:25:30 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: gerd.moellmann@gmail.com, andreyorst@gmail.com, 56683@debbugs.gnu.org
> 
> >> Do I understand correctly that it's okay to do that on the feature (and 
> >> later master) branch, and that it will be perhaps be revisited later?
> >
> > I'd prefer not to do it just yet, and wait for more user feedback (once 
> > the feature branch is merged, which I guess will be soon).
> 
> Wouldn't it be easier to get useful user feedback if that automatic 
> disabling was present on master?  If it's absent, what kind of user 
> feedback can we expect to decide what is better for Emacs 29?

My feeling is that we didn't yet exhaust the potential for speedups by
ways other than by turning off features.  So the feedback I'd like to
get is more use cases where the current speedups are still not enough,
so that we could identify additional ones.  Once we get to the point
where either there's no more significant feedback or we see no way of
solving the reported issues by measures like those we took till now,
we will have to consider which features to give up and in what
situations.

The font-lock effect is a good case in point: if we were to decide to
turn it off when there are long lines, we'd have missed the speedup
you just installed.  I think there are more opportunities like that,
or at least there could be.

> It does indeed, but alas the fact that displaying such buffers is 
> noticeably slower with truncate-lines remains.  I could perhaps take a 
> look (after finalizing and merging the current branch), but I'm not really 
> sure it's worth the price.

We already display such buffers faster on the branch than on master,
and, the Isearch issue aside (I will describe what I found in a
minute), there could be more speedup opportunities for that.
truncate-lines is an important feature (we just heard someone
explaining one such use case), so I'd like to make it as performant as
we can before we decide whether it's really necessary to turn it off.

bug#56683: 29.0.50; long lines fix doesn't work correctly when lines are truncated

[Gregory Heytings]

>
> My feeling is that we didn't yet exhaust the potential for speedups by 
> ways other than by turning off features.
>

My feeling is that we did, if we take the truncate-lines case out of the 
picture.  My feeling is also that adding more optimizations for the 
truncate-lines case is not worth the price.

>
> The font-lock effect is a good case in point: if we were to decide to 
> turn it off when there are long lines, we'd have missed the speedup you 
> just installed.
>

The difference is that I told you that I would look at that problem, and 
that now, after having spent many hours on this, I highly doubt it's 
possible to do better, especially given that no matter what we do to 
improve the truncate-lines case, we'll hit another ceil very soon (the 
DISP_INFINITY one, even if you decide to enlarge it).  It seems to me that 
the most reasonable thing to do is to simply tell users that Emacs has a 
limit in that case (and again, it has one, no matter what we do).

As far as I can see, the only way to really speedup the truncate-lines 
case would be to introduce an entirely new narrowing method, in which the 
buffer would be narrowed in a rectangular way.  That is, instead of 
narrowing a buffer between BEGV and ZV, each line 1, ..., n around point 
would be narrowed between BEGV1..ZV1, ..., BEGVn..ZVn.

>
> truncate-lines is an important feature (we just heard someone explaining 
> one such use case),
>

In that specific use case the lines do not hit the long-line-threshold 
limit.