GNU logs - #57266, boring messages

Message sent to bug-gnu-emacs@HIDDEN:

Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Stefan Monnier <monnier@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Wed, 17 Aug 2022 21:20:02 +0000
Resent-Message-ID: <handler.57266.B.166077115022973 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: 57266 <at> debbugs.gnu.org
From: Stefan Monnier <monnier@HIDDEN>
Date: Wed, 17 Aug 2022 17:18:44 -0400
Message-ID: <jwvzgg2zhzf.fsf@HIDDEN>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Received-SPF: pass client-ip=132.204.25.50;
 envelope-from=monnier@HIDDEN; helo=mailscanner.iro.umontreal.ca
X-Spam_score_int: -42
X-Spam_score: -4.3
X-Spam_bar: ----
X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3,
 SPF_HELO_NONE=0.001, SPF_PASS=-0.001,
 T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-Spam-Score: -1.3 (-)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -2.3 (--)

--=-=-=
Content-Type: text/plain

Tags: patch

Based on the recent discussion around counting lines, I proposed the
patch below.  Part of it aims to just document the way in which
the `w->base_line_number` cache is maintained, but it goes further,
fixing the problem with format-mode-line and narrowing I discovered
and simplifying some of the code based on the new understanding of how
the cache is supposed to work.


        Stefan


2022-08-17  Stefan Monnier  <monnier@HIDDEN>

    * src/xdisp.c (BASE_LINE_NUMBER_VALID_P): New macro.
    (try_scrolling): Use it.  Ignore `just_this_one_p` because it was
    too pessimistic.
    (redisplay_window): Remove `buffer_unchanged_p`, not used any more.
    Check BASE_LINE_NUMBER_VALID_P once and for all at the beginning and
    don't rely on other side-information that is only correlated with
    BASE_LINE_NUMBER_VALID_P to decide when to flush this cache.
    (decode_mode_spec): Check `BASE_LINE_NUMBER_VALID_P` before using
    the cache.

    * src/window.c (set_window_buffer): Flush the `base_line_number` cache.


In GNU Emacs 29.0.50 (build 1, i686-pc-linux-gnu, GTK+ Version 3.24.34, cairo version 1.16.0)
 of 2022-08-15 built on lechazo
Repository revision: 5564f72d5b3e2f162f52d935c077f6ea15fb60b7
Repository branch: work
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux bookworm/sid

Configured using:
 'configure -C --enable-checking --enable-check-lisp-object-type --with-modules --with-cairo --with-tiff=ifavailable
 'CFLAGS=-Wall -g3 -Og -Wno-pointer-sign'
 PKG_CONFIG_PATH=/home/monnier/lib/pkgconfig'


--=-=-=
Content-Type: text/patch; charset=iso-8859-1
Content-Disposition: attachment; filename=base_line.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/window.c b/src/window.c
index c8fcb3a607f..436d4e342d7 100644
--- a/src/window.c
+++ b/src/window.c
@@ -4093,6 +4093,8 @@ set_window_buffer (Lisp_Object window, Lisp_Object bu=
ffer,
 			     buffer);
       w->start_at_line_beg =3D false;
       w->force_start =3D false;
+      /* Flush the base_line cache since it applied to another buffer.  */
+      w->base_line_number =3D 0;
     }
=20
   wset_redisplay (w);
diff --git a/src/xdisp.c b/src/xdisp.c
index 03c43be5bc0..ec7cc904e78 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -18341,6 +18341,14 @@ cursor_row_fully_visible_p (struct window *w, bool=
 force_p,
    `scroll-conservatively' and the Emacs manual.  */
 #define SCROLL_LIMIT 100
=20
+/* The freshness of the w->base_line_number cache is only ensured at every
+   redisplay cycle, so the cache can be used only if there's been
+   no relevant changes to the buffer since the last redisplay.  */
+#define BASE_LINE_NUMBER_VALID_P(w)                      \
+   (eassert (current_buffer =3D=3D XBUFFER ((w)->contents)), \
+    !current_buffer->clip_changed                        \
+    && BEG_UNCHANGED < (w)->base_line_number)
+
 static int
 try_scrolling (Lisp_Object window, bool just_this_one_p,
 	       intmax_t arg_scroll_conservatively, intmax_t scroll_step,
@@ -18640,9 +18648,10 @@ try_scrolling (Lisp_Object window, bool just_this_=
one_p,
   else
     {
       /* Maybe forget recorded base line for line number display.  */
-      if (!just_this_one_p
-	  || current_buffer->clip_changed
-	  || BEG_UNCHANGED < CHARPOS (startp))
+      /* FIXME: Why do we need this?  `try_scrolling` can only be called f=
rom
+         `redisplay_window` which should have flushed this cache already w=
hen
+         eeded.  */
+      if (!BASE_LINE_NUMBER_VALID_P (w))
 	w->base_line_number =3D 0;
=20
       /* If cursor ends up on a partially visible line,
@@ -19404,9 +19413,6 @@ redisplay_window (Lisp_Object window, bool just_thi=
s_one_p)
   /* Record it now because it's overwritten.  */
   bool current_matrix_up_to_date_p =3D false;
   bool used_current_matrix_p =3D false;
-  /* This is less strict than current_matrix_up_to_date_p.
-     It indicates that the buffer contents and narrowing are unchanged.  */
-  bool buffer_unchanged_p =3D false;
   bool temp_scroll_step =3D false;
   specpdl_ref count =3D SPECPDL_INDEX ();
   int rc;
@@ -19505,11 +19511,6 @@ redisplay_window (Lisp_Object window, bool just_th=
is_one_p)
=20
   specbind (Qinhibit_point_motion_hooks, Qt);
=20
-  buffer_unchanged_p
-    =3D (w->window_end_valid
-       && !current_buffer->clip_changed
-       && !window_outdated (w));
-
   /* When windows_or_buffers_changed is non-zero, we can't rely
      on the window end being valid, so set it to zero there.  */
   if (windows_or_buffers_changed)
@@ -19648,6 +19649,10 @@ redisplay_window (Lisp_Object window, bool just_th=
is_one_p)
 	}
     }
=20
+  if (!BASE_LINE_NUMBER_VALID_P (w))
+    /* Forget any recorded base line for line number display.  */
+    w->base_line_number =3D 0;
+
  force_start:
=20
   /* Handle case where place to start displaying has been specified,
@@ -19668,10 +19673,6 @@ redisplay_window (Lisp_Object window, bool just_th=
is_one_p)
       w->preserve_vscroll_p =3D false;
       w->window_end_valid =3D false;
=20
-      /* Forget any recorded base line for line number display.  */
-      if (!buffer_unchanged_p)
-	w->base_line_number =3D 0;
-
       /* Redisplay the mode line.  Select the buffer properly for that.
 	 Also, run the hook window-scroll-functions
 	 because we have scrolled.  */
@@ -20000,12 +20001,6 @@ redisplay_window (Lisp_Object window, bool just_th=
is_one_p)
=20
       if (w->cursor.vpos >=3D 0)
 	{
-	  if (!just_this_one_p
-	      || current_buffer->clip_changed
-	      || BEG_UNCHANGED < CHARPOS (startp))
-	    /* Forget any recorded base line for line number display.  */
-	    w->base_line_number =3D 0;
-
 	  if (!cursor_row_fully_visible_p (w, true, false, false))
 	    {
 	      clear_glyph_matrix (w->desired_matrix);
@@ -20076,10 +20071,6 @@ redisplay_window (Lisp_Object window, bool just_th=
is_one_p)
   debug_method_add (w, "recenter");
 #endif
=20
-  /* Forget any previously recorded base line for line number display.  */
-  if (!buffer_unchanged_p)
-    w->base_line_number =3D 0;
-
   /* Determine the window start relative to point.  */
   init_iterator (&it, w, PT, PT_BYTE, NULL, DEFAULT_FACE_ID);
   it.current_y =3D it.last_visible_y;
@@ -24211,6 +24202,13 @@ maybe_produce_line_number (struct it *it)
       if (!last_line)
 	{
 	  /* If possible, reuse data cached by line-number-mode.  */
+	  /* NOTE: We use `base_line_number` without checking
+	     BASE_LINE_NUMBER_VALID_P because we assume that `redisplay_window`
+	     has already flushed this cache for us when needed.
+	     NOTE=B2: IIUC checking BASE_LINE_NUMBER_VALID_P here would be
+	     overly pessimistic because it might say that the cache
+	     was invalid before entering `redisplay_window` yet the
+	     value has just been refreshed.  */
 	  if (it->w->base_line_number > 0
 	      && it->w->base_line_pos > 0
 	      && it->w->base_line_pos <=3D IT_CHARPOS (*it)
@@ -27949,6 +27947,17 @@ decode_mode_spec (struct window *w, register int c=
, int field_width,
 	startpos =3D marker_position (w->start);
 	startpos_byte =3D marker_byte_position (w->start);
 	height =3D WINDOW_TOTAL_LINES (w);
+
+ 	if (!BASE_LINE_NUMBER_VALID_P (w))
+ 	  /* NOTE: This check is necessary when we're called from
+ 	     `format-mode-line` but not when we're called from
+ 	     `redisplay_window`.
+ 	     FIXME: It's usually harmless in that case, except if you have
+ 	     several `%l` in your modeline, in which case it will cause the
+ 	     cache to to be recomputed as many times when it's out of
+ 	     date :-(  */
+ 	  w->base_line_number =3D 0;
+
 	/* We cannot cope with w->start being outside of the
 	   accessible portion of the buffer; in particular,
 	   display_count_lines call below might infloop if called with
@@ -27962,8 +27971,6 @@ decode_mode_spec (struct window *w, register int c,=
 int field_width,
 	  {
 	    startpos =3D BUF_BEGV (b);
 	    startpos_byte =3D BUF_BEGV_BYTE (b);
-	    w->base_line_pos =3D 0;
-	    w->base_line_number =3D 0;
 	  }
=20
 	/* If we decided that this buffer isn't suitable for line numbers,
@@ -28044,6 +28051,12 @@ decode_mode_spec (struct window *w, register int c=
, int field_width,
 		goto no_value;
 	      }
=20
+	    /* NOTE: if current_buffer->clip_changed is set or
+               if BEG_UNCHANGED is < position, this new cached value
+               may be unusable, because we can't reset BEG_UNCHANGED
+               or `clip_changed` from here (since they reflect the
+               changes since the last redisplay do they can only be reset
+               from `mark_window_display_accurate_1`).  */
 	    w->base_line_number =3D topline - nlines;
 	    w->base_line_pos =3D BYTE_TO_CHAR (position);
 	  }

--=-=-=--

Message sent:

Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-Mailer: MIME-tools 5.505 (Entity 5.505)
Content-Type: text/plain; charset=utf-8
X-Loop: help-debbugs@HIDDEN
From: help-debbugs@HIDDEN (GNU bug Tracking System)
To: Stefan Monnier <monnier@HIDDEN>
Subject: bug#57266: Acknowledgement (Maintaining the base_line_number cache)
Message-ID: <handler.57266.B.166077115022973.ack <at> debbugs.gnu.org>
References: <jwvzgg2zhzf.fsf@HIDDEN>
X-Gnu-PR-Message: ack 57266
X-Gnu-PR-Package: emacs
X-Gnu-PR-Keywords: patch
Reply-To: 57266 <at> debbugs.gnu.org
Date: Wed, 17 Aug 2022 21:20:02 +0000

Thank you for filing a new bug report with debbugs.gnu.org.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 bug-gnu-emacs@HIDDEN

If you wish to submit further information on this problem, please
send it to 57266 <at> debbugs.gnu.org.

Please do not send mail to help-debbugs@HIDDEN unless you wish
to report a problem with the Bug-tracking system.

--=20
57266: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D57266
GNU Bug Tracking System
Contact help-debbugs@HIDDEN with problems

Message sent to bug-gnu-emacs@HIDDEN:

Subject: bug#57266: Acknowledgement (Maintaining the base_line_number cache)
Resent-From: Stefan Monnier <monnier@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Wed, 17 Aug 2022 21:41:01 +0000
Resent-Message-ID: <handler.57266.B57266.166077240624938 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: 57266 <at> debbugs.gnu.org
From: Stefan Monnier <monnier@HIDDEN>
In-Reply-To: <handler.57266.B.166077115022973.ack <at> debbugs.gnu.org> (GNU bug
 Tracking System's message of "Wed, 17 Aug 2022 21:20:02 +0000")
Message-ID: <jwvr11ev9cu.fsf-monnier+emacs@HIDDEN>
References: <jwvzgg2zhzf.fsf@HIDDEN>
 <handler.57266.B.166077115022973.ack <at> debbugs.gnu.org>
Date: Wed, 17 Aug 2022 17:39:51 -0400
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Precedence: list
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>

--=-=-=
Content-Type: text/plain

Hmm... there was some serious thinko in my BASE_LINE_NUMBER_VALID_P.
Adjusted patch below,


        Stefan

--=-=-=
Content-Type: text/x-diff; charset=iso-8859-1
Content-Disposition: inline; filename=base_line.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/window.c b/src/window.c
index c8fcb3a607f..436d4e342d7 100644
--- a/src/window.c
+++ b/src/window.c
@@ -4093,6 +4093,8 @@ set_window_buffer (Lisp_Object window, Lisp_Object bu=
ffer,
 			     buffer);
       w->start_at_line_beg =3D false;
       w->force_start =3D false;
+      /* Flush the base_line cache since it applied to another buffer.  */
+      w->base_line_number =3D 0;
     }
=20
   wset_redisplay (w);
diff --git a/src/xdisp.c b/src/xdisp.c
index 03c43be5bc0..902cd276a43 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -18341,6 +18341,14 @@ cursor_row_fully_visible_p (struct window *w, bool=
 force_p,
    `scroll-conservatively' and the Emacs manual.  */
 #define SCROLL_LIMIT 100
=20
+/* The freshness of the w->base_line_number cache is only ensured at every
+   redisplay cycle, so the cache can be used only if there's been
+   no relevant changes to the buffer since the last redisplay.  */
+#define BASE_LINE_NUMBER_VALID_P(w)                      \
+   (eassert (current_buffer =3D=3D XBUFFER ((w)->contents)), \
+    !current_buffer->clip_changed                        \
+    && BEG_UNCHANGED >=3D (w)->base_line_pos)
+
 static int
 try_scrolling (Lisp_Object window, bool just_this_one_p,
 	       intmax_t arg_scroll_conservatively, intmax_t scroll_step,
@@ -18640,9 +18648,10 @@ try_scrolling (Lisp_Object window, bool just_this_=
one_p,
   else
     {
       /* Maybe forget recorded base line for line number display.  */
-      if (!just_this_one_p
-	  || current_buffer->clip_changed
-	  || BEG_UNCHANGED < CHARPOS (startp))
+      /* FIXME: Why do we need this?  `try_scrolling` can only be called f=
rom
+         `redisplay_window` which should have flushed this cache already w=
hen
+         eeded.  */
+      if (!BASE_LINE_NUMBER_VALID_P (w))
 	w->base_line_number =3D 0;
=20
       /* If cursor ends up on a partially visible line,
@@ -19404,9 +19413,6 @@ redisplay_window (Lisp_Object window, bool just_thi=
s_one_p)
   /* Record it now because it's overwritten.  */
   bool current_matrix_up_to_date_p =3D false;
   bool used_current_matrix_p =3D false;
-  /* This is less strict than current_matrix_up_to_date_p.
-     It indicates that the buffer contents and narrowing are unchanged.  */
-  bool buffer_unchanged_p =3D false;
   bool temp_scroll_step =3D false;
   specpdl_ref count =3D SPECPDL_INDEX ();
   int rc;
@@ -19505,11 +19511,6 @@ redisplay_window (Lisp_Object window, bool just_th=
is_one_p)
=20
   specbind (Qinhibit_point_motion_hooks, Qt);
=20
-  buffer_unchanged_p
-    =3D (w->window_end_valid
-       && !current_buffer->clip_changed
-       && !window_outdated (w));
-
   /* When windows_or_buffers_changed is non-zero, we can't rely
      on the window end being valid, so set it to zero there.  */
   if (windows_or_buffers_changed)
@@ -19648,6 +19649,10 @@ redisplay_window (Lisp_Object window, bool just_th=
is_one_p)
 	}
     }
=20
+  if (!BASE_LINE_NUMBER_VALID_P (w))
+    /* Forget any recorded base line for line number display.  */
+    w->base_line_number =3D 0;
+
  force_start:
=20
   /* Handle case where place to start displaying has been specified,
@@ -19668,10 +19673,6 @@ redisplay_window (Lisp_Object window, bool just_th=
is_one_p)
       w->preserve_vscroll_p =3D false;
       w->window_end_valid =3D false;
=20
-      /* Forget any recorded base line for line number display.  */
-      if (!buffer_unchanged_p)
-	w->base_line_number =3D 0;
-
       /* Redisplay the mode line.  Select the buffer properly for that.
 	 Also, run the hook window-scroll-functions
 	 because we have scrolled.  */
@@ -20000,12 +20001,6 @@ redisplay_window (Lisp_Object window, bool just_th=
is_one_p)
=20
       if (w->cursor.vpos >=3D 0)
 	{
-	  if (!just_this_one_p
-	      || current_buffer->clip_changed
-	      || BEG_UNCHANGED < CHARPOS (startp))
-	    /* Forget any recorded base line for line number display.  */
-	    w->base_line_number =3D 0;
-
 	  if (!cursor_row_fully_visible_p (w, true, false, false))
 	    {
 	      clear_glyph_matrix (w->desired_matrix);
@@ -20076,10 +20071,6 @@ redisplay_window (Lisp_Object window, bool just_th=
is_one_p)
   debug_method_add (w, "recenter");
 #endif
=20
-  /* Forget any previously recorded base line for line number display.  */
-  if (!buffer_unchanged_p)
-    w->base_line_number =3D 0;
-
   /* Determine the window start relative to point.  */
   init_iterator (&it, w, PT, PT_BYTE, NULL, DEFAULT_FACE_ID);
   it.current_y =3D it.last_visible_y;
@@ -24211,6 +24202,13 @@ maybe_produce_line_number (struct it *it)
       if (!last_line)
 	{
 	  /* If possible, reuse data cached by line-number-mode.  */
+	  /* NOTE: We use `base_line_number` without checking
+	     BASE_LINE_NUMBER_VALID_P because we assume that `redisplay_window`
+	     has already flushed this cache for us when needed.
+	     NOTE=B2: IIUC checking BASE_LINE_NUMBER_VALID_P here would be
+	     overly pessimistic because it might say that the cache
+	     was invalid before entering `redisplay_window` yet the
+	     value has just been refreshed.  */
 	  if (it->w->base_line_number > 0
 	      && it->w->base_line_pos > 0
 	      && it->w->base_line_pos <=3D IT_CHARPOS (*it)
@@ -27949,6 +27947,17 @@ decode_mode_spec (struct window *w, register int c=
, int field_width,
 	startpos =3D marker_position (w->start);
 	startpos_byte =3D marker_byte_position (w->start);
 	height =3D WINDOW_TOTAL_LINES (w);
+
+ 	if (!BASE_LINE_NUMBER_VALID_P (w))
+ 	  /* NOTE: This check is necessary when we're called from
+ 	     `format-mode-line` but not when we're called from
+ 	     `redisplay_window`.
+ 	     FIXME: It's usually harmless in that case, except if you have
+ 	     several `%l` in your modeline, in which case it will cause the
+ 	     cache to to be recomputed as many times when it's out of
+ 	     date :-(  */
+ 	  w->base_line_number =3D 0;
+
 	/* We cannot cope with w->start being outside of the
 	   accessible portion of the buffer; in particular,
 	   display_count_lines call below might infloop if called with
@@ -27962,8 +27971,6 @@ decode_mode_spec (struct window *w, register int c,=
 int field_width,
 	  {
 	    startpos =3D BUF_BEGV (b);
 	    startpos_byte =3D BUF_BEGV_BYTE (b);
-	    w->base_line_pos =3D 0;
-	    w->base_line_number =3D 0;
 	  }
=20
 	/* If we decided that this buffer isn't suitable for line numbers,
@@ -28044,6 +28051,12 @@ decode_mode_spec (struct window *w, register int c=
, int field_width,
 		goto no_value;
 	      }
=20
+	    /* NOTE: if current_buffer->clip_changed is set or
+               if BEG_UNCHANGED is < position, this new cached value
+               may be unusable, because we can't reset BEG_UNCHANGED
+               or `clip_changed` from here (since they reflect the
+               changes since the last redisplay do they can only be reset
+               from `mark_window_display_accurate_1`).  */
 	    w->base_line_number =3D topline - nlines;
 	    w->base_line_pos =3D BYTE_TO_CHAR (position);
 	  }

--=-=-=--

Message sent to bug-gnu-emacs@HIDDEN:

Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Eli Zaretskii <eliz@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Thu, 18 Aug 2022 06:04:01 +0000
Resent-Message-ID: <handler.57266.B57266.166080263210079 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: Stefan Monnier <monnier@HIDDEN>
Cc: 57266 <at> debbugs.gnu.org
Date: Thu, 18 Aug 2022 09:03:37 +0300
Message-Id: <831qteccli.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
In-Reply-To: <jwvzgg2zhzf.fsf@HIDDEN> (bug-gnu-emacs@HIDDEN)
References: <jwvzgg2zhzf.fsf@HIDDEN>
MIME-version: 1.0
Content-type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Precedence: list
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>

> Date: Wed, 17 Aug 2022 17:18:44 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@HIDDEN>
> 
> Based on the recent discussion around counting lines, I proposed the
> patch below.  Part of it aims to just document the way in which
> the `w->base_line_number` cache is maintained, but it goes further,
> fixing the problem with format-mode-line and narrowing I discovered
> and simplifying some of the code based on the new understanding of how
> the cache is supposed to work.

Thanks for working on this, but I think this is sometimes insufficient
and sometimes too bold for my palate: I'm not thrilled by the
perspective of investigating hard-to-debug bug reports about this
tricky issue, and will only agree to absolutely 110% safe changes.

All in all, I'd be much happier if you didn't attempt any "cleanups"
whose value is questionable in this area, only added the missing
invalidations.  (It is okay to replace identical or similar tests with
a macro, of course.)

Specifically:

> --- a/src/window.c
> +++ b/src/window.c
> @@ -4093,6 +4093,8 @@ set_window_buffer (Lisp_Object window, Lisp_Object buffer,
>  			     buffer);
>        w->start_at_line_beg = false;
>        w->force_start = false;
> +      /* Flush the base_line cache since it applied to another buffer.  */
> +      w->base_line_number = 0;
>      }

This is harmless, but IMO unnecessary, because wset_buffer already
takes care of that, via adjust_window_count.

> +/* The freshness of the w->base_line_number cache is only ensured at every
> +   redisplay cycle, so the cache can be used only if there's been
> +   no relevant changes to the buffer since the last redisplay.  */
> +#define BASE_LINE_NUMBER_VALID_P(w)                      \
> +   (eassert (current_buffer == XBUFFER ((w)->contents)), \
> +    !current_buffer->clip_changed                        \
> +    && BEG_UNCHANGED < (w)->base_line_number)

The assertion there is unnecessary: instead of using current_buffer,
you can use XBUFFER ((w)->contents).  It is also safer. Recently I
learned that in some corners of redisplay_window, the condition

  current_buffer == XBUFFER ((w)->contents)

is not necessarily tru.

Also, at least one place where you want to use this macro check
window_outdated, which this macro doesn't do.  If you know the reason
why that call is unnecessary, please explain it.

>        /* Maybe forget recorded base line for line number display.  */
> -      if (!just_this_one_p
> -	  || current_buffer->clip_changed
> -	  || BEG_UNCHANGED < CHARPOS (startp))
> +      /* FIXME: Why do we need this?  `try_scrolling` can only be called from
> +         `redisplay_window` which should have flushed this cache already when
> +         eeded.  */
> +      if (!BASE_LINE_NUMBER_VALID_P (w))
>  	w->base_line_number = 0;

About the FIXME: please analyze the control flow inside
redisplay_window and post your conclusions here, if not include them
in the log message.  Some of the optimizations that redisplay_window
attempts early on, if they succeed, do "goto done;", bypassing the
rest of the code, including try_scrolling and whatnot.  You need to
convince yourself and us (at least me) that deleting those other
places which invalidate the cache is indeed justified, because there's
no chance we will bypass the ones you leave in the code, and OTOH that
we don't unnecessarily invalidate the cache where we shouldn't.  The
control flow in redisplay_window is quite tricky.

> @@ -19404,9 +19413,6 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
>    /* Record it now because it's overwritten.  */
>    bool current_matrix_up_to_date_p = false;
>    bool used_current_matrix_p = false;
> -  /* This is less strict than current_matrix_up_to_date_p.
> -     It indicates that the buffer contents and narrowing are unchanged.  */
> -  bool buffer_unchanged_p = false;

Before you can delete this and its uses, please perform the
above-mentioned analysis.

> @@ -19505,11 +19511,6 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
>  
>    specbind (Qinhibit_point_motion_hooks, Qt);
>  
> -  buffer_unchanged_p
> -    = (w->window_end_valid
> -       && !current_buffer->clip_changed
> -       && !window_outdated (w));

Here you remove window_outdated without replacing it with anything.
Why?

> @@ -20000,12 +20001,6 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
>  
>        if (w->cursor.vpos >= 0)
>  	{
> -	  if (!just_this_one_p
> -	      || current_buffer->clip_changed
> -	      || BEG_UNCHANGED < CHARPOS (startp))
> -	    /* Forget any recorded base line for line number display.  */
> -	    w->base_line_number = 0;

Sorry, I don't buy your "reason" for removing just_this_one_p.  And
the deletion itself needs that analysis I mentioned.

> +	     NOTE²: IIUC checking BASE_LINE_NUMBER_VALID_P here would be

Please avoid using non-ASCII characters in C sources, as they aren't
visited with UTF-8 encoding by default.

> +	     overly pessimistic because it might say that the cache
> +	     was invalid before entering `redisplay_window` yet the
> +	     value has just been refreshed.  */
>  	  if (it->w->base_line_number > 0
>  	      && it->w->base_line_pos > 0
>  	      && it->w->base_line_pos <= IT_CHARPOS (*it)
> @@ -27949,6 +27947,17 @@ decode_mode_spec (struct window *w, register int c, int field_width,
>  	startpos = marker_position (w->start);
>  	startpos_byte = marker_byte_position (w->start);
>  	height = WINDOW_TOTAL_LINES (w);
> +
> + 	if (!BASE_LINE_NUMBER_VALID_P (w))

Here your macro assumes the buffer is current_buffer, whereas the rest
of the code doesn't.  See my comment about that assertion in the macro.

> @@ -27962,8 +27971,6 @@ decode_mode_spec (struct window *w, register int c, int field_width,
>  	  {
>  	    startpos = BUF_BEGV (b);
>  	    startpos_byte = BUF_BEGV_BYTE (b);
> -	    w->base_line_pos = 0;
> -	    w->base_line_number = 0;

Here you assume that current_buffer->clip_changed detects the
condition

	if (!(BUF_BEGV_BYTE (b) <= startpos_byte
	      && startpos_byte <= BUF_ZV_BYTE (b)))

But I see no particular reason to think that assumption is always
valid.

> +	    /* NOTE: if current_buffer->clip_changed is set or
> +               if BEG_UNCHANGED is < position, this new cached value
> +               may be unusable, because we can't reset BEG_UNCHANGED
> +               or `clip_changed` from here (since they reflect the
> +               changes since the last redisplay do they can only be reset
> +               from `mark_window_display_accurate_1`).  */

The part of the comment in parentheses has some sort of typo, because
it doesn't parse.

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Stefan Monnier <monnier@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Mon, 22 Aug 2022 04:35:02 +0000
Resent-Message-ID: <handler.57266.B57266.166114289513650 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 57266
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
To: Eli Zaretskii <eliz@HIDDEN>
Cc: 57266 <at> debbugs.gnu.org
Received: via spool by 57266-submit <at> debbugs.gnu.org id=B57266.166114289513650
          (code B ref 57266); Mon, 22 Aug 2022 04:35:02 +0000
Received: (at 57266) by debbugs.gnu.org; 22 Aug 2022 04:34:55 +0000
Received: from localhost ([127.0.0.1]:37328 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1oPz9K-0003Y4-3i
	for submit <at> debbugs.gnu.org; Mon, 22 Aug 2022 00:34:55 -0400
Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:27672)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <monnier@HIDDEN>) id 1oPz9I-0003Xr-72
 for 57266 <at> debbugs.gnu.org; Mon, 22 Aug 2022 00:34:53 -0400
Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1])
 by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id C464080758;
 Mon, 22 Aug 2022 00:34:46 -0400 (EDT)
Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1])
 by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 177FD80636;
 Mon, 22 Aug 2022 00:34:44 -0400 (EDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca;
 s=mail; t=1661142884;
 bh=LM0WwaqaWiiYn4CTQ/wi+JrChiqivOCHMyCd/FSQmzg=;
 h=From:To:Cc:Subject:In-Reply-To:References:Date:From;
 b=Zsw/z9gKTALgV3f/ACinBHbTv9OVQhrlDRAr2gLM8VfcN8fgf4v2GDEG3H85dXINX
 pHzh+Q4WuvcfxSpWhojpUKAtInsz7WlP8V8MuJ8RVF1JGW1AtQe5vWdP66TUiWFHQi
 zK0VZ5JT0cUUbid8vCghAJmQ6QEoq4D5A+/x1ICJpUiWoVcG6fsDTfd6r0mYZYv9if
 U4ok98pjxeJEQroPjbirrs+Xz/6dkqXC9N+hxGDJpcUjXn8lRD55cKvX+lDHbdnTxv
 uoDah+uMBNPW1Ft37uB+91QkTd8Jrg4KsBWfwZ2ZHLSdvGYAiZ30y8H065UHs+nsRQ
 JvjF5lIu8fWKg==
Received: from pastel (unknown [45.72.195.111])
 by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id BA4EC120470;
 Mon, 22 Aug 2022 00:34:43 -0400 (EDT)
From: Stefan Monnier <monnier@HIDDEN>
In-Reply-To: <831qteccli.fsf@HIDDEN> (Eli Zaretskii's message of "Thu, 18 Aug
 2022 09:03:37 +0300")
Message-ID: <jwv5yikexc9.fsf-monnier+emacs@HIDDEN>
References: <jwvzgg2zhzf.fsf@HIDDEN> <831qteccli.fsf@HIDDEN>
Date: Mon, 22 Aug 2022 00:34:42 -0400
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-SPAM-INFO: Spam detection results:  0
 ALL_TRUSTED                -1 Passed through trusted hosts only via SMTP
 AWL -0.058 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DKIM_SIGNED               0.1 Message has a DKIM or DK signature,
 not necessarily valid
 DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature
 DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's
 domain T_SCC_BODY_TEXT_LINE    -0.01 -
X-SPAM-LEVEL: 
X-Spam-Score: -2.3 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> All in all, I'd be much happier if you didn't attempt any "cleanups"
> whose value is questionable in this area, only added the missing
> invalidations.  (It is okay to replace identical or similar tests with
> a macro, of course.)

I think the `!just_this_one_p` is important enough since the test isn't
needed, and it otherwise prevents caching the base_line when a buffer is
displayed in more than 1 window, which can slow down redisplay noticeably
in such a case.

I added more comments to explain how the cache works.  It doesn't
explain why we tested `just_this_one_p`, tho, so maybe I'm still
missing something.  The Git history shows this test has been there ever
since 1993 but I haven't seen (nor figured) any explanation for it :-(
My best guess is that the original code basically only tried to be
"clever" when a single window needed refresh and otherwise redid
everything from scratch and that design also applied to the
base_line cache, without thinking too hard about whether it's
truly needed.

>> @@ -4093,6 +4093,8 @@ set_window_buffer (Lisp_Object window, Lisp_Object buffer,
>>  			     buffer);
>>        w->start_at_line_beg = false;
>>        w->force_start = false;
>> +      /* Flush the base_line cache since it applied to another buffer.  */
>> +      w->base_line_number = 0;
>>      }
>
> This is harmless, but IMO unnecessary, because wset_buffer already
> takes care of that, via adjust_window_count.

Indeed, thanks.  In the new patch I moved this from
`adjust_window_count` to `wset_buffer` (otherwise it's always executed
twice in `wset_buffer` and even three times in the other function that
calls `adjust_window_count`, since that one also calls `wset_buffer`).

>> +/* The freshness of the w->base_line_number cache is only ensured at every
>> +   redisplay cycle, so the cache can be used only if there's been
>> +   no relevant changes to the buffer since the last redisplay.  */
>> +#define BASE_LINE_NUMBER_VALID_P(w)                      \
>> +   (eassert (current_buffer == XBUFFER ((w)->contents)), \
>> +    !current_buffer->clip_changed                        \
>> +    && BEG_UNCHANGED < (w)->base_line_number)
>
> The assertion there is unnecessary: instead of using current_buffer,
> you can use XBUFFER ((w)->contents).  It is also safer.

Duh, indeed, thanks.

> Recently I learned that in some corners of redisplay_window, the
> condition
>
>   current_buffer == XBUFFER ((w)->contents)
>
> is not necessarily tru.

Indeed, this happens for example when calling `format-mode-line` from
a buffer different from the window's buffer.

> Also, at least one place where you want to use this macro check
> window_outdated, which this macro doesn't do.  If you know the reason
> why that call is unnecessary, please explain it.

It's not necessary because the only way the cache can be out of date
if when either clipping or buffer text or w->contents is changed and
that is already taken into account.

>>        /* Maybe forget recorded base line for line number display.  */
>> -      if (!just_this_one_p
>> -	  || current_buffer->clip_changed
>> -	  || BEG_UNCHANGED < CHARPOS (startp))
>> +      /* FIXME: Why do we need this?  `try_scrolling` can only be called from
>> +         `redisplay_window` which should have flushed this cache already when
>> +         eeded.  */
>> +      if (!BASE_LINE_NUMBER_VALID_P (w))
>>  	w->base_line_number = 0;
>
> About the FIXME: please analyze the control flow inside
> redisplay_window and post your conclusions here, if not include them
> in the log message.

The above code is in `try_scrolling` which is only called by
`redisplay_window` and my patch changes `redisplay_window` so it always
flushes the cache if needed, right from the start, so by the time we get
to `try_scrolling` we know the cache has already been flushed if needed
so this code is redundant.

> Some of the optimizations that redisplay_window attempts early on, if
> they succeed, do "goto done;", bypassing the rest of the code,
> including try_scrolling and whatnot.

The dependency goes the other way around.

>> @@ -20000,12 +20001,6 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
>>  
>>        if (w->cursor.vpos >= 0)
>>  	{
>> -	  if (!just_this_one_p
>> -	      || current_buffer->clip_changed
>> -	      || BEG_UNCHANGED < CHARPOS (startp))
>> -	    /* Forget any recorded base line for line number display.  */
>> -	    w->base_line_number = 0;
>
> Sorry, I don't buy your "reason" for removing just_this_one_p.  And
> the deletion itself needs that analysis I mentioned.

Maybe the !just_this_one_p was needed at some point in the past.
Now it's not any more, but I don't know when/where/how that changed.

>> @@ -27949,6 +27947,17 @@ decode_mode_spec (struct window *w, register int c, int field_width,
>>  	startpos = marker_position (w->start);
>>  	startpos_byte = marker_byte_position (w->start);
>>  	height = WINDOW_TOTAL_LINES (w);
>> +
>> + 	if (!BASE_LINE_NUMBER_VALID_P (w))
>
> Here your macro assumes the buffer is current_buffer, whereas the rest
> of the code doesn't.  See my comment about that assertion in the macro.

Indeed, thanks.

>> @@ -27962,8 +27971,6 @@ decode_mode_spec (struct window *w, register int c, int field_width,
>>  	  {
>>  	    startpos = BUF_BEGV (b);
>>  	    startpos_byte = BUF_BEGV_BYTE (b);
>> -	    w->base_line_pos = 0;
>> -	    w->base_line_number = 0;
>
> Here you assume that current_buffer->clip_changed detects the
> condition
>
> 	if (!(BUF_BEGV_BYTE (b) <= startpos_byte
> 	      && startpos_byte <= BUF_ZV_BYTE (b)))
>
> But I see no particular reason to think that assumption is always
> valid.

No, the reason those lines are redundant is that the base_line cache is
not affected by startpos: the value stored there is does not depend on startpos.
This said, in my patch I reverted this change because setting `w->base_line_pos = 0;`
has another side effect which is to reconsider whether to display line
numbers or not.

>> +	    /* NOTE: if current_buffer->clip_changed is set or
>> +               if BEG_UNCHANGED is < position, this new cached value
>> +               may be unusable, because we can't reset BEG_UNCHANGED
>> +               or `clip_changed` from here (since they reflect the
>> +               changes since the last redisplay do they can only be reset
>> +               from `mark_window_display_accurate_1`).  */
>
> The part of the comment in parentheses has some sort of typo, because
> it doesn't parse.

Indeed.  I think it's fixed now.


        Stefan


diff --git a/src/window.c b/src/window.c
index 2bce4c9723d..a6e3ad904f1 100644
--- a/src/window.c
+++ b/src/window.c
@@ -282,9 +282,6 @@ adjust_window_count (struct window *w, int arg)
 	b = b->base_buffer;
       b->window_count += arg;
       eassert (b->window_count >= 0);
-      /* These should be recalculated by redisplay code.  */
-      w->window_end_valid = false;
-      w->base_line_pos = 0;
     }
 }
 
@@ -301,6 +298,9 @@ wset_buffer (struct window *w, Lisp_Object val)
     eassert (MARKERP (w->start) && MARKERP (w->pointm));
   w->contents = val;
   adjust_window_count (w, 1);
+  /* These should be recalculated by redisplay code.  */
+  w->window_end_valid = false;
+  w->base_line_pos = 0;
 }
 
 static void
diff --git a/src/xdisp.c b/src/xdisp.c
index 03c43be5bc0..d9052c067f0 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -18341,6 +18341,39 @@ cursor_row_fully_visible_p (struct window *w, bool force_p,
    `scroll-conservatively' and the Emacs manual.  */
 #define SCROLL_LIMIT 100
 
+/* The freshness of the w->base_line_number cache is only ensured at every
+   redisplay cycle, so the cache can be used only if there's been
+   no relevant changes to the buffer since the last redisplay.  */
+static bool
+base_line_number_valid_p (struct window *w)
+{
+  /* The base_line cache stores a value which depends on:
+       A. w->contents (AKA `b`)
+       B. BUF_BEGV (b);
+       C. the contents of b->text
+     We flush the cache (in `wset_buffer`) when we set `w->contents`.
+     But for (B) and (C) we don't eagerly flush the cache when those get
+     changed, instead we rely on the `b->clip_changed` bit for (B) and on
+     BUF_BEG_UNCHANGED (b) for (C), and we check them here before making use
+     of the cache.
+     Both of those pieces of info are (re)set by redisplay.  This means that
+     after a modification which invalidates the cache, the cache will stay
+     invalid until the next redisplay, even if we recompute the value anew.
+     Worse: clip_changed and BUF_BEG_UNCHANGED only get reset at the end of a
+     redisplay cycle (after all the windows have been redisplayed), so the
+     base_lines we computed during redisplay are put into the cache
+     at a time when `base_line_number_valid_p` can return false.
+     In order to avoid throwing away this useful info, the cache is flushed
+     as needed at the beginning of redisplay (in `redisplay_window`) so that
+     during redisplay we can presume that the cache is valid (even if
+     `base_line_number_valid_p` is false).
+     It also means we can have a problem if the clipping or the b->text is
+     modified *during* redisplay.  */
+  struct buffer *b = XBUFFER ((w)->contents);
+  return !b->clip_changed
+         && BUF_BEG_UNCHANGED (b) >= (w)->base_line_pos;
+}
+
 static int
 try_scrolling (Lisp_Object window, bool just_this_one_p,
 	       intmax_t arg_scroll_conservatively, intmax_t scroll_step,
@@ -18639,12 +18672,6 @@ try_scrolling (Lisp_Object window, bool just_this_one_p,
     }
   else
     {
-      /* Maybe forget recorded base line for line number display.  */
-      if (!just_this_one_p
-	  || current_buffer->clip_changed
-	  || BEG_UNCHANGED < CHARPOS (startp))
-	w->base_line_number = 0;
-
       /* If cursor ends up on a partially visible line,
 	 treat that as being off the bottom of the screen.  */
       if (! cursor_row_fully_visible_p (w, extra_scroll_margin_lines <= 1,
@@ -19404,9 +19431,6 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
   /* Record it now because it's overwritten.  */
   bool current_matrix_up_to_date_p = false;
   bool used_current_matrix_p = false;
-  /* This is less strict than current_matrix_up_to_date_p.
-     It indicates that the buffer contents and narrowing are unchanged.  */
-  bool buffer_unchanged_p = false;
   bool temp_scroll_step = false;
   specpdl_ref count = SPECPDL_INDEX ();
   int rc;
@@ -19447,6 +19471,15 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
        cleverly elsewhere.  */
     w->must_be_updated_p = true;
 
+  if (!base_line_number_valid_p (w))
+    /* Forget any recorded base line for line number display.
+       We do it:
+       - after calling `reconsider_clip_changes` which may discover
+         that the clipping hasn't changed after all.
+       - early enough that we're sure it's done (the code below has
+         various `goto` which could otherwise skip this operation).  */
+    w->base_line_number = 0;
+
   if (MINI_WINDOW_P (w))
     {
       if (w == XWINDOW (echo_area_window)
@@ -19505,11 +19538,6 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
 
   specbind (Qinhibit_point_motion_hooks, Qt);
 
-  buffer_unchanged_p
-    = (w->window_end_valid
-       && !current_buffer->clip_changed
-       && !window_outdated (w));
-
   /* When windows_or_buffers_changed is non-zero, we can't rely
      on the window end being valid, so set it to zero there.  */
   if (windows_or_buffers_changed)
@@ -19668,10 +19696,6 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
       w->preserve_vscroll_p = false;
       w->window_end_valid = false;
 
-      /* Forget any recorded base line for line number display.  */
-      if (!buffer_unchanged_p)
-	w->base_line_number = 0;
-
       /* Redisplay the mode line.  Select the buffer properly for that.
 	 Also, run the hook window-scroll-functions
 	 because we have scrolled.  */
@@ -20000,12 +20024,6 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
 
       if (w->cursor.vpos >= 0)
 	{
-	  if (!just_this_one_p
-	      || current_buffer->clip_changed
-	      || BEG_UNCHANGED < CHARPOS (startp))
-	    /* Forget any recorded base line for line number display.  */
-	    w->base_line_number = 0;
-
 	  if (!cursor_row_fully_visible_p (w, true, false, false))
 	    {
 	      clear_glyph_matrix (w->desired_matrix);
@@ -20076,10 +20094,6 @@ redisplay_window (Lisp_Object window, bool just_this_one_p)
   debug_method_add (w, "recenter");
 #endif
 
-  /* Forget any previously recorded base line for line number display.  */
-  if (!buffer_unchanged_p)
-    w->base_line_number = 0;
-
   /* Determine the window start relative to point.  */
   init_iterator (&it, w, PT, PT_BYTE, NULL, DEFAULT_FACE_ID);
   it.current_y = it.last_visible_y;
@@ -24211,6 +24225,14 @@ maybe_produce_line_number (struct it *it)
       if (!last_line)
 	{
 	  /* If possible, reuse data cached by line-number-mode.  */
+	  /* NOTE: We use `base_line_number` without checking
+	     `base_line_number_valid_p` because we assume that
+             `redisplay_window` has already flushed this cache for us when
+             needed.
+	     Checking `base_line_number_valid_p` here would be
+	     overly pessimistic because it might say that the cache
+	     was invalid before entering `redisplay_window` yet the
+	     value has just been refreshed.  */
 	  if (it->w->base_line_number > 0
 	      && it->w->base_line_pos > 0
 	      && it->w->base_line_pos <= IT_CHARPOS (*it)
@@ -27523,10 +27545,24 @@ DEFUN ("format-mode-line", Fformat_mode_line, Sformat_mode_line,
 	= NILP (face) ? Qnil : list2 (Qface, face);
     }
 
+  if (!EQ (buffer, w->contents) || !base_line_number_valid_p (w))
+    /* decode_mode_spec/display_mode_element presume that the base_line
+       cache has been flushed if needed.  They don't flush it themselves,
+       because that could flush a value they just computed, e.g. when
+       %l appears twice in the mode line (or once in the mode line and
+       once in the frame title, ...).  */
+    w->base_line_number = 0;
+
   push_kboard (FRAME_KBOARD (it.f));
   display_mode_element (&it, 0, 0, 0, format, Qnil, false);
   pop_kboard ();
 
+  if (!EQ (buffer, w->contents) || !base_line_number_valid_p (w))
+    /* Flush any new base_line we may have computed while the clipping was
+       changed (or while operating on another buffer), since
+       `reconsider_clip_changes` may reset clip_changes to false later on.  */
+    w->base_line_number = 0;
+
   if (no_props)
     {
       len = MODE_LINE_NOPROP_LEN (string_start);
@@ -28044,6 +28080,12 @@ decode_mode_spec (struct window *w, register int c, int field_width,
 		goto no_value;
 	      }
 
+	    /* NOTE: if current_buffer->clip_changed is set or
+               if BEG_UNCHANGED is < position, this new cached value
+               may be unusable, because we can't reset BEG_UNCHANGED
+               or `clip_changed` from here (since they reflect the
+               changes since the last redisplay, they can only be reset
+               from `mark_window_display_accurate_1`).  */
 	    w->base_line_number = topline - nlines;
 	    w->base_line_pos = BYTE_TO_CHAR (position);
 	  }

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Eli Zaretskii <eliz@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Mon, 22 Aug 2022 12:12:02 +0000
Resent-Message-ID: <handler.57266.B57266.166117029331173 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 57266
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
To: Stefan Monnier <monnier@HIDDEN>
Cc: 57266 <at> debbugs.gnu.org
Received: via spool by 57266-submit <at> debbugs.gnu.org id=B57266.166117029331173
          (code B ref 57266); Mon, 22 Aug 2022 12:12:02 +0000
Received: (at 57266) by debbugs.gnu.org; 22 Aug 2022 12:11:33 +0000
Received: from localhost ([127.0.0.1]:38229 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1oQ6HE-00086i-U9
	for submit <at> debbugs.gnu.org; Mon, 22 Aug 2022 08:11:33 -0400
Received: from eggs.gnu.org ([209.51.188.92]:50680)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@HIDDEN>) id 1oQ6HD-00086W-2c
 for 57266 <at> debbugs.gnu.org; Mon, 22 Aug 2022 08:11:31 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e]:35122)
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <eliz@HIDDEN>)
 id 1oQ6H7-0005qZ-4p; Mon, 22 Aug 2022 08:11:25 -0400
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
 s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date:
 mime-version; bh=zHa4CDo0CSWnWl6tmROuQpfPBFrNthaVhv3BbwsMvio=; b=ey3E+veN00jI
 KHaIiD6Yy88lcKZa8cVRnTDvM6s3nvP5zCkbXiAAUD5T6Pq9DgA5BlLMj9HzxRKkX0Wp+z/4jqOUA
 rYOKn/5ZcyRvUEvxCJmRJ1nT8UBqwWmgewii/4fl+QjL57+3R2sDIZ9JjGJE3EoVmy1CNPepNAUMP
 dNEq0gJTAepSA+IwAIB4oxpa+UKJoVmh3D6kICZBZi9ejlguz7Nsk3cFBoMxiZf+p8PL0bDbXvBpr
 663FEMFggK/Wz/iG6OEVlHv7X+5MRgF8JHR6hRSZj/0H5fHxC0cA3pgkFd4Y5qIJEP4GAyDDvbP7/
 TFsH+K//+gbttnDV1+3kqw==;
Received: from [87.69.77.57] (port=1109 helo=home-c4e4a596f7)
 by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <eliz@HIDDEN>)
 id 1oQ6H5-0005dV-8W; Mon, 22 Aug 2022 08:11:24 -0400
Date: Mon, 22 Aug 2022 15:11:26 +0300
Message-Id: <834jy4bhqp.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
In-Reply-To: <jwv5yikexc9.fsf-monnier+emacs@HIDDEN> (message from Stefan
 Monnier on Mon, 22 Aug 2022 00:34:42 -0400)
References: <jwvzgg2zhzf.fsf@HIDDEN> <831qteccli.fsf@HIDDEN>
 <jwv5yikexc9.fsf-monnier+emacs@HIDDEN>
X-Spam-Score: -2.3 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> From: Stefan Monnier <monnier@HIDDEN>
> Cc: 57266 <at> debbugs.gnu.org
> Date: Mon, 22 Aug 2022 00:34:42 -0400
> 
> > All in all, I'd be much happier if you didn't attempt any "cleanups"
> > whose value is questionable in this area, only added the missing
> > invalidations.  (It is okay to replace identical or similar tests with
> > a macro, of course.)
> 
> I think the `!just_this_one_p` is important enough since the test isn't
> needed, and it otherwise prevents caching the base_line when a buffer is
> displayed in more than 1 window, which can slow down redisplay noticeably
> in such a case.

I didn't just mean just_this_one_p thingy, I meant all the changes
that attempt to somehow "clean up" the use of the line-number cache.
Why do we have to do that at all?  This stuff is not too complex, and
it works for ages!  Isn't the fact that we find more and more small
issues with the changes telling you something?

Please, just leave it alone, and only fix the actual problems.

> > Also, at least one place where you want to use this macro check
> > window_outdated, which this macro doesn't do.  If you know the reason
> > why that call is unnecessary, please explain it.
> 
> It's not necessary because the only way the cache can be out of date
> if when either clipping or buffer text or w->contents is changed and
> that is already taken into account.

What about the w->last_overlay_modified part, which window_outdated
takes into account?

> >>        /* Maybe forget recorded base line for line number display.  */
> >> -      if (!just_this_one_p
> >> -	  || current_buffer->clip_changed
> >> -	  || BEG_UNCHANGED < CHARPOS (startp))
> >> +      /* FIXME: Why do we need this?  `try_scrolling` can only be called from
> >> +         `redisplay_window` which should have flushed this cache already when
> >> +         eeded.  */
> >> +      if (!BASE_LINE_NUMBER_VALID_P (w))
> >>  	w->base_line_number = 0;
> >
> > About the FIXME: please analyze the control flow inside
> > redisplay_window and post your conclusions here, if not include them
> > in the log message.
> 
> The above code is in `try_scrolling` which is only called by
> `redisplay_window` and my patch changes `redisplay_window` so it always
> flushes the cache if needed, right from the start, so by the time we get
> to `try_scrolling` we know the cache has already been flushed if needed
> so this code is redundant.

The "if needed" part is the one I don't buy.  You moved it towards the
beginning of redisplay_window, which might mean it is sometimes done
prematurely and/or unnecessarily.  And the explanation is too
"hand-wavy" for my palate, sorry.

Once again, I'd prefer that we didn't touch working code, which
nowadays supports at least 3e features: the mode-line display, the
display-line-numbers mode, and the line-number-at-pos function and its
callers.

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Stefan Monnier <monnier@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Mon, 22 Aug 2022 12:42:01 +0000
Resent-Message-ID: <handler.57266.B57266.166117211518610 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 57266
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
To: Eli Zaretskii <eliz@HIDDEN>
Cc: 57266 <at> debbugs.gnu.org
Received: via spool by 57266-submit <at> debbugs.gnu.org id=B57266.166117211518610
          (code B ref 57266); Mon, 22 Aug 2022 12:42:01 +0000
Received: (at 57266) by debbugs.gnu.org; 22 Aug 2022 12:41:55 +0000
Received: from localhost ([127.0.0.1]:38269 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1oQ6kc-0004q5-Q8
	for submit <at> debbugs.gnu.org; Mon, 22 Aug 2022 08:41:55 -0400
Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:31375)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <monnier@HIDDEN>) id 1oQ6kY-0004pj-1i
 for 57266 <at> debbugs.gnu.org; Mon, 22 Aug 2022 08:41:53 -0400
Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1])
 by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 7193C100142;
 Mon, 22 Aug 2022 08:41:44 -0400 (EDT)
Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1])
 by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 0DD6E100091;
 Mon, 22 Aug 2022 08:41:43 -0400 (EDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca;
 s=mail; t=1661172103;
 bh=XFJV2Z66l8rnkirqPEBx1qt0vJkYur7BKwdrDVDaHn8=;
 h=From:To:Cc:Subject:In-Reply-To:References:Date:From;
 b=NLhBJ4Zn8ijBpvbTv7dLWDOg2wjdar6lPRWIFMB+Aryznt43F4qhjSksPC6+RboIW
 ZnVPTIutgi44x/4FKPflgk0yZvKZ3Q9ydC2MIGo6vc490EMdh/ge+IKhyhgvNT3rrt
 Iopnsxzj+/tFzf3D+XKocEO7hmZ8jQKLbqUS/Qn1FuCHgNkHB/GI5tpgl1j498W/F4
 oUZS0oeFHlbSs2/DXSFm+eFoHiAR1CxYeBzCL3BCEYNAeyEovPxCHndOmnJYsv3DeM
 BuFb75aLCsNwt5Jw1uaKnalQtuNsZUUMnNFNCq8Omemu1ybey/xpv+5+E0PkHwmjWR
 19Lj9ep0/uRUg==
Received: from pastel (unknown [45.72.195.111])
 by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id CBEF9120201;
 Mon, 22 Aug 2022 08:41:42 -0400 (EDT)
From: Stefan Monnier <monnier@HIDDEN>
In-Reply-To: <834jy4bhqp.fsf@HIDDEN> (Eli Zaretskii's message of "Mon, 22 Aug
 2022 15:11:26 +0300")
Message-ID: <jwvedx8cv4n.fsf-monnier+emacs@HIDDEN>
References: <jwvzgg2zhzf.fsf@HIDDEN> <831qteccli.fsf@HIDDEN>
 <jwv5yikexc9.fsf-monnier+emacs@HIDDEN> <834jy4bhqp.fsf@HIDDEN>
Date: Mon, 22 Aug 2022 08:41:41 -0400
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-SPAM-INFO: Spam detection results:  0
 ALL_TRUSTED                -1 Passed through trusted hosts only via SMTP
 AWL -0.049 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DKIM_SIGNED               0.1 Message has a DKIM or DK signature,
 not necessarily valid
 DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature
 DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's
 domain T_SCC_BODY_TEXT_LINE    -0.01 -
X-SPAM-LEVEL: 
X-Spam-Score: -2.3 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

>> I think the `!just_this_one_p` is important enough since the test isn't
>> needed, and it otherwise prevents caching the base_line when a buffer is
>> displayed in more than 1 window, which can slow down redisplay noticeably
>> in such a case.
>
> I didn't just mean just_this_one_p thingy, I meant all the changes
> that attempt to somehow "clean up" the use of the line-number cache.
> Why do we have to do that at all?  This stuff is not too complex, and
> it works for ages!  Isn't the fact that we find more and more small
> issues with the changes telling you something?

Yes, it's telling me that the code is a mess and I'm trying to fix it by
documenting what is actually needed.

>> It's not necessary because the only way the cache can be out of date
>> if when either clipping or buffer text or w->contents is changed and
>> that is already taken into account.
> What about the w->last_overlay_modified part, which window_outdated
> takes into account?

Overlays have no influence over the line number count, because the
base_line cache only counts real \n characters, not display lines.

> Once again, I'd prefer that we didn't touch working code, which

Code we don't dare to change is a problem we should fix.

> nowadays supports at least 3e features: the mode-line display, the
> display-line-numbers mode, and the line-number-at-pos function and its
> callers.

AFAIK it's not used (yet) in `line-number-at-pos`.


        Stefan

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Eli Zaretskii <eliz@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Mon, 22 Aug 2022 13:08:02 +0000
Resent-Message-ID: <handler.57266.B57266.166117364329868 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 57266
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
To: Stefan Monnier <monnier@HIDDEN>
Cc: 57266 <at> debbugs.gnu.org
Received: via spool by 57266-submit <at> debbugs.gnu.org id=B57266.166117364329868
          (code B ref 57266); Mon, 22 Aug 2022 13:08:02 +0000
Received: (at 57266) by debbugs.gnu.org; 22 Aug 2022 13:07:23 +0000
Received: from localhost ([127.0.0.1]:38305 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1oQ79H-0007lg-8E
	for submit <at> debbugs.gnu.org; Mon, 22 Aug 2022 09:07:23 -0400
Received: from eggs.gnu.org ([209.51.188.92]:36300)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@HIDDEN>) id 1oQ79E-0007lP-7W
 for 57266 <at> debbugs.gnu.org; Mon, 22 Aug 2022 09:07:22 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e]:39176)
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <eliz@HIDDEN>)
 id 1oQ798-00070C-VY; Mon, 22 Aug 2022 09:07:14 -0400
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
 s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date:
 mime-version; bh=Rgfbni1/jylNTx3U+NEFoqF/eNjA5ias/CBt4eHW6iA=; b=g0AmcghHrGxj
 iIe8Ffpu9/TUvMO0LKtWvnOJZpqz8oQ6GWCeV4JWJvm9LsTWqyy197+QSiYiwm2NM31Q43MCDl1GC
 W1GZqeaC9+F7woVyaY5xxD5jZurtf7H45dZQwLygqhNsIVwEmejT1t/P3STH7tSTj5xLpEFwRJtxP
 sBj+GklKcFjB7yaCJYbZQE9uZeObQbLTK3BrhwdlIpUqFN8fGjCqNtMmzi1oTnGbPL9WNORhadlNV
 UjhF6MzzLc6o5mOIRhlkpTPgIpQCNRNsen3DPh5a/R36NPd+fW/xfy9Li+9IcEB920dNw0Do3nlUg
 RSfF/8bnm96hdFBQ1WxIsw==;
Received: from [87.69.77.57] (port=4518 helo=home-c4e4a596f7)
 by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <eliz@HIDDEN>)
 id 1oQ798-0003b0-DU; Mon, 22 Aug 2022 09:07:14 -0400
Date: Mon, 22 Aug 2022 16:07:18 +0300
Message-Id: <83y1vga0l5.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
In-Reply-To: <jwvedx8cv4n.fsf-monnier+emacs@HIDDEN> (message from Stefan
 Monnier on Mon, 22 Aug 2022 08:41:41 -0400)
References: <jwvzgg2zhzf.fsf@HIDDEN> <831qteccli.fsf@HIDDEN>
 <jwv5yikexc9.fsf-monnier+emacs@HIDDEN> <834jy4bhqp.fsf@HIDDEN>
 <jwvedx8cv4n.fsf-monnier+emacs@HIDDEN>
X-Spam-Score: -2.3 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> From: Stefan Monnier <monnier@HIDDEN>
> Cc: 57266 <at> debbugs.gnu.org
> Date: Mon, 22 Aug 2022 08:41:41 -0400
> 
> > I didn't just mean just_this_one_p thingy, I meant all the changes
> > that attempt to somehow "clean up" the use of the line-number cache.
> > Why do we have to do that at all?  This stuff is not too complex, and
> > it works for ages!  Isn't the fact that we find more and more small
> > issues with the changes telling you something?
> 
> Yes, it's telling me that the code is a mess and I'm trying to fix it by
> documenting what is actually needed.

It's fine to fix and enhance the documentation, but please don't fix
the code that works.

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Stefan Monnier <monnier@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Mon, 22 Aug 2022 13:33:02 +0000
Resent-Message-ID: <handler.57266.B57266.166117518132357 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 57266
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
To: Eli Zaretskii <eliz@HIDDEN>
Cc: 57266 <at> debbugs.gnu.org
Received: via spool by 57266-submit <at> debbugs.gnu.org id=B57266.166117518132357
          (code B ref 57266); Mon, 22 Aug 2022 13:33:02 +0000
Received: (at 57266) by debbugs.gnu.org; 22 Aug 2022 13:33:01 +0000
Received: from localhost ([127.0.0.1]:38343 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1oQ7Y5-0008Pm-0i
	for submit <at> debbugs.gnu.org; Mon, 22 Aug 2022 09:33:01 -0400
Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:9400)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <monnier@HIDDEN>) id 1oQ7Y2-0008PZ-PZ
 for 57266 <at> debbugs.gnu.org; Mon, 22 Aug 2022 09:32:59 -0400
Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1])
 by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id E5EDE1001F3;
 Mon, 22 Aug 2022 09:32:52 -0400 (EDT)
Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1])
 by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 9B7D810010E;
 Mon, 22 Aug 2022 09:32:51 -0400 (EDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca;
 s=mail; t=1661175171;
 bh=fO4X8JMr4pjH/r1S/Ejz4e1Drnd9VI8odeQYFLczfAU=;
 h=From:To:Cc:Subject:In-Reply-To:References:Date:From;
 b=hELDuxOtJlwg0QspsN40MkYgnYn8rEFBRTS0Du9YeFxPLAtzeW9uXaTORbzWYLlpn
 IraqlGR+snreZPG0/KB2L/EdSuOWI32tYzOBCAlKJQolvVCzOv9BS1D2MsgvAcTw5T
 2E/O9pOqzRdh9OvfQSDGjnJ7QPJv7x+PWj910e34myGUPU9CznABKC8JO2z/hgaX6z
 okzYc62ZYYtGZmnn2MS/ru0CHMFszpyydNHskyo6MbbfXTDbelAYhbSHec//lx2g95
 hUJWEXzj4iRd0nBL+XD1ttOCwsqGd9Pt15HQW9PRRv5SGmkeOivzIP2ti/q0VVPuhA
 xIVKEmsreLbkA==
Received: from pastel (unknown [45.72.195.111])
 by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 7DB3C120193;
 Mon, 22 Aug 2022 09:32:51 -0400 (EDT)
From: Stefan Monnier <monnier@HIDDEN>
In-Reply-To: <83y1vga0l5.fsf@HIDDEN> (Eli Zaretskii's message of "Mon, 22 Aug
 2022 16:07:18 +0300")
Message-ID: <jwv8rngcsk6.fsf-monnier+emacs@HIDDEN>
References: <jwvzgg2zhzf.fsf@HIDDEN> <831qteccli.fsf@HIDDEN>
 <jwv5yikexc9.fsf-monnier+emacs@HIDDEN> <834jy4bhqp.fsf@HIDDEN>
 <jwvedx8cv4n.fsf-monnier+emacs@HIDDEN> <83y1vga0l5.fsf@HIDDEN>
Date: Mon, 22 Aug 2022 09:32:50 -0400
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-SPAM-INFO: Spam detection results:  0
 ALL_TRUSTED                -1 Passed through trusted hosts only via SMTP
 AWL -0.049 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DKIM_SIGNED               0.1 Message has a DKIM or DK signature,
 not necessarily valid
 DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature
 DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's
 domain T_SCC_BODY_TEXT_LINE    -0.01 -
X-SPAM-LEVEL: 
X-Spam-Score: -2.3 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

>> > I didn't just mean just_this_one_p thingy, I meant all the changes
>> > that attempt to somehow "clean up" the use of the line-number cache.
>> > Why do we have to do that at all?  This stuff is not too complex, and
>> > it works for ages!  Isn't the fact that we find more and more small
>> > issues with the changes telling you something?
>> 
>> Yes, it's telling me that the code is a mess and I'm trying to fix it by
>> documenting what is actually needed.
>
> It's fine to fix and enhance the documentation, but please don't fix
> the code that works.

If the code doesn't match the doc, then we'll never know if the doc
is wrong.


        Stefan

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Eli Zaretskii <eliz@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Mon, 22 Aug 2022 16:22:01 +0000
Resent-Message-ID: <handler.57266.B57266.166118530812967 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 57266
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
To: Stefan Monnier <monnier@HIDDEN>
Cc: 57266 <at> debbugs.gnu.org
Received: via spool by 57266-submit <at> debbugs.gnu.org id=B57266.166118530812967
          (code B ref 57266); Mon, 22 Aug 2022 16:22:01 +0000
Received: (at 57266) by debbugs.gnu.org; 22 Aug 2022 16:21:48 +0000
Received: from localhost ([127.0.0.1]:41589 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1oQABP-0003N5-UZ
	for submit <at> debbugs.gnu.org; Mon, 22 Aug 2022 12:21:48 -0400
Received: from eggs.gnu.org ([209.51.188.92]:44830)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@HIDDEN>) id 1oQABO-0003Ms-7b
 for 57266 <at> debbugs.gnu.org; Mon, 22 Aug 2022 12:21:46 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e]:56138)
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <eliz@HIDDEN>)
 id 1oQABI-0001Rt-6K; Mon, 22 Aug 2022 12:21:40 -0400
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
 s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date:
 mime-version; bh=Jjbz/rE7ic6c8rWa8UmFa29dC/h7MbFkNTl/uTnYhmo=; b=D9UyOCCTpKqD
 v8hMrtY9Usb0CnIb6R5pTmBnc12saW0nlWyH6mhofGM2Z6xuf1KNkKHPV9Kk++wsf4ccMPHH2wrqJ
 w7CcYjyfyGAA1Ivg5txym8ktxS9foLtANckzdeKwuD+OoUFzOEza8g0T99DA/rYelDK47uTjbo/uq
 yVxytt+tlDh5Oqx0OcR3L70WLsOa3vndexC4VVG4Xlybi5L8lwWF6LRWcg1CeXnY8+yZoMA64OAvj
 tGERZ2ccLyuligKe/vcSaLs5bGhyhhIbr55D1O0P3e1Ot0eR/ZbMXu2eOw99CdNhlFl/0upH26l5y
 mg+V7WWCvKBSqJs3w7V8RA==;
Received: from [87.69.77.57] (port=1124 helo=home-c4e4a596f7)
 by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <eliz@HIDDEN>)
 id 1oQABH-0003Mk-4i; Mon, 22 Aug 2022 12:21:39 -0400
Date: Mon, 22 Aug 2022 19:21:43 +0300
Message-Id: <83edx89rl4.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
In-Reply-To: <jwv8rngcsk6.fsf-monnier+emacs@HIDDEN> (message from Stefan
 Monnier on Mon, 22 Aug 2022 09:32:50 -0400)
References: <jwvzgg2zhzf.fsf@HIDDEN> <831qteccli.fsf@HIDDEN>
 <jwv5yikexc9.fsf-monnier+emacs@HIDDEN> <834jy4bhqp.fsf@HIDDEN>
 <jwvedx8cv4n.fsf-monnier+emacs@HIDDEN> <83y1vga0l5.fsf@HIDDEN>
 <jwv8rngcsk6.fsf-monnier+emacs@HIDDEN>
X-Spam-Score: -2.3 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> From: Stefan Monnier <monnier@HIDDEN>
> Cc: 57266 <at> debbugs.gnu.org
> Date: Mon, 22 Aug 2022 09:32:50 -0400
> 
> >> > I didn't just mean just_this_one_p thingy, I meant all the changes
> >> > that attempt to somehow "clean up" the use of the line-number cache.
> >> > Why do we have to do that at all?  This stuff is not too complex, and
> >> > it works for ages!  Isn't the fact that we find more and more small
> >> > issues with the changes telling you something?
> >> 
> >> Yes, it's telling me that the code is a mess and I'm trying to fix it by
> >> documenting what is actually needed.
> >
> > It's fine to fix and enhance the documentation, but please don't fix
> > the code that works.
> 
> If the code doesn't match the doc, then we'll never know if the doc
> is wrong.

Which is why I'm perfectly OK with documenting what the code does.

I'm also OK with adding a single function that makes the tests for
invalidating the cache, and then calling that function everywhere
where we now do such tests, not all of them identical.  And in that
function we can document everything we know about the cases where the
cache should be invalidated.

But please leave the tests themselves where they are done now, I see
no special reason to move them.  Because nothing is really wrong,
except the one case you found (and which we should fix, I agree).

Please!  I hope my requests mean something, given that I'm the only
one who works on bugs in these areas, and probably will keep doing
that for the observable future.

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Stefan Monnier <monnier@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Mon, 22 Aug 2022 18:03:01 +0000
Resent-Message-ID: <handler.57266.B57266.16611913756890 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 57266
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
To: Eli Zaretskii <eliz@HIDDEN>
Cc: 57266 <at> debbugs.gnu.org
Received: via spool by 57266-submit <at> debbugs.gnu.org id=B57266.16611913756890
          (code B ref 57266); Mon, 22 Aug 2022 18:03:01 +0000
Received: (at 57266) by debbugs.gnu.org; 22 Aug 2022 18:02:55 +0000
Received: from localhost ([127.0.0.1]:41704 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1oQBlH-0001n3-Dk
	for submit <at> debbugs.gnu.org; Mon, 22 Aug 2022 14:02:55 -0400
Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:57303)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <monnier@HIDDEN>) id 1oQBlG-0001mo-0Z
 for 57266 <at> debbugs.gnu.org; Mon, 22 Aug 2022 14:02:54 -0400
Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1])
 by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 5B6614412F6;
 Mon, 22 Aug 2022 14:02:47 -0400 (EDT)
Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1])
 by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id CA401441264;
 Mon, 22 Aug 2022 14:02:45 -0400 (EDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca;
 s=mail; t=1661191365;
 bh=+DB+/FTD9hIU+Bu+M66J2xe46lZZx6amqg7gk3j2Lrc=;
 h=From:To:Cc:Subject:In-Reply-To:References:Date:From;
 b=JKvt0wjkLwGmNEYpqJ7BetfI9agGX697k2wQz/2rQfkMJ99V0BevxMDrxAxD7KnZe
 /Y//vRMZX3HAyflNEr/VoBjikfA++2c6Mf+EIr5gUc2VLZzmKSjbMYlsXVqm7bt97j
 jthjk1rZ5IjtnT5jDmRgZO7mj6pOf1RAMhwE9qVbE4Afy44uC+nG+XMYBvDwIQn0t1
 FIJgVEDZo6Br1IIXAjqHuFDl0N+2gPkjzwJhhUlGQ3EyKaRr2I/Ue9dQViUy+R0w79
 PIZ0mCSZvRDqpQTDMp24lwdws42qDwnfZzWUmN+7S0pzTO87Pxxgem0BorrmBiO8Iu
 k9GqNhYBJ0/hg==
Received: from lechazo (lechon.iro.umontreal.ca [132.204.27.242])
 by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id A941112017B;
 Mon, 22 Aug 2022 14:02:45 -0400 (EDT)
From: Stefan Monnier <monnier@HIDDEN>
In-Reply-To: <83edx89rl4.fsf@HIDDEN> (Eli Zaretskii's message of "Mon, 22 Aug
 2022 19:21:43 +0300")
Message-ID: <jwvk070qi54.fsf-monnier+emacs@HIDDEN>
References: <jwvzgg2zhzf.fsf@HIDDEN> <831qteccli.fsf@HIDDEN>
 <jwv5yikexc9.fsf-monnier+emacs@HIDDEN> <834jy4bhqp.fsf@HIDDEN>
 <jwvedx8cv4n.fsf-monnier+emacs@HIDDEN> <83y1vga0l5.fsf@HIDDEN>
 <jwv8rngcsk6.fsf-monnier+emacs@HIDDEN> <83edx89rl4.fsf@HIDDEN>
Date: Mon, 22 Aug 2022 14:02:44 -0400
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-SPAM-INFO: Spam detection results:  0
 ALL_TRUSTED                -1 Passed through trusted hosts only via SMTP
 AWL 0.212 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DKIM_SIGNED               0.1 Message has a DKIM or DK signature,
 not necessarily valid
 DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature
 DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's
 domain T_SCC_BODY_TEXT_LINE    -0.01 -
X-SPAM-LEVEL: 
X-Spam-Score: -2.3 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

>> >> > I didn't just mean just_this_one_p thingy, I meant all the changes
>> >> > that attempt to somehow "clean up" the use of the line-number cache.
>> >> > Why do we have to do that at all?  This stuff is not too complex, and
>> >> > it works for ages!  Isn't the fact that we find more and more small
>> >> > issues with the changes telling you something?
>> >> 
>> >> Yes, it's telling me that the code is a mess and I'm trying to fix it by
>> >> documenting what is actually needed.
>> >
>> > It's fine to fix and enhance the documentation, but please don't fix
>> > the code that works.
>> 
>> If the code doesn't match the doc, then we'll never know if the doc
>> is wrong.
>
> Which is why I'm perfectly OK with documenting what the code does.

But I'm not.  Because I don't really know what the code does.
The doc I wrote documents the design I came up with based on my
understanding of what the code does.

I went through the trouble of figuring out a design of how things
*should* work, document it, and adjust the code to match the design.
So I'm not interested in pushing some half-assed version of it that
documents a design that doesn't match the reality, nor pushing partial
fixes/workarounds (for borderline cases that noone ever bumped into
anyway) while keeping the mess of unexplained hacks.

> Please!  I hope my requests mean something, given that I'm the only
> one who works on bugs in these areas, and probably will keep doing
> that for the observable future.

Indeed, my proposed patch is not a bug fix.
It's a code-maintenance patch.  It's meant to improve the code, not
the behavior.
[ I do think it will improve the behavior in some cases (by fixing a few
  corner case bugs, and more importantly by improving performance when
  displaying a buffer is several windows), but that is not the
  motivation, and there is undoubtedly the risk that it will introduce
  regressions (which will help us better understand the code).  ]


        Stefan

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Eli Zaretskii <eliz@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Mon, 22 Aug 2022 18:37:02 +0000
Resent-Message-ID: <handler.57266.B57266.166119336510026 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 57266
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
To: Stefan Monnier <monnier@HIDDEN>
Cc: 57266 <at> debbugs.gnu.org
Received: via spool by 57266-submit <at> debbugs.gnu.org id=B57266.166119336510026
          (code B ref 57266); Mon, 22 Aug 2022 18:37:02 +0000
Received: (at 57266) by debbugs.gnu.org; 22 Aug 2022 18:36:05 +0000
Received: from localhost ([127.0.0.1]:41721 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1oQCHN-0002bd-92
	for submit <at> debbugs.gnu.org; Mon, 22 Aug 2022 14:36:05 -0400
Received: from eggs.gnu.org ([209.51.188.92]:56976)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@HIDDEN>) id 1oQCHJ-0002b7-NI
 for 57266 <at> debbugs.gnu.org; Mon, 22 Aug 2022 14:36:04 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e]:48546)
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <eliz@HIDDEN>)
 id 1oQCHD-0003EX-Eh; Mon, 22 Aug 2022 14:35:55 -0400
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
 s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date:
 mime-version; bh=c9tURtlKx8MebDtYtIiNrnjk7yQDzbatr8uvurbl2wk=; b=e5z2beloOw+p
 MAYUZ3hQm126TVC+c17DL98yYrUbhtHvRYUAyTWzkSvWLwAoSF2fQGTDE89i9ie3TJXRfmOqSQr6k
 MoEU0PKlXt+KOH/QRPRwZpJtA002bpBRt8ev7Tsh4SVKaPJypANoEyCUvYPA8I7mKkWszTkGRNyGp
 VhVs3hos7vOxKntX2P2QKA7jCtk5WlyrnOzzfrXgsCBCajDEDTzh8XIFejk8TzTsDsNl2W0Gw1xP9
 vb1B/8nkSCWz3fUQzKRAvuQPamOBHmokCs8MXp8LrOxyk3nKEqkL/tJxbSVYfkR/YVKLuk+SI/2VL
 TD2NyJ4tNddNG3BjHCJlcg==;
Received: from [87.69.77.57] (port=1728 helo=home-c4e4a596f7)
 by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <eliz@HIDDEN>)
 id 1oQCH7-0007Dj-Ov; Mon, 22 Aug 2022 14:35:54 -0400
Date: Mon, 22 Aug 2022 21:35:54 +0300
Message-Id: <83a67w9ldh.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
In-Reply-To: <jwvk070qi54.fsf-monnier+emacs@HIDDEN> (message from Stefan
 Monnier on Mon, 22 Aug 2022 14:02:44 -0400)
References: <jwvzgg2zhzf.fsf@HIDDEN> <831qteccli.fsf@HIDDEN>
 <jwv5yikexc9.fsf-monnier+emacs@HIDDEN> <834jy4bhqp.fsf@HIDDEN>
 <jwvedx8cv4n.fsf-monnier+emacs@HIDDEN> <83y1vga0l5.fsf@HIDDEN>
 <jwv8rngcsk6.fsf-monnier+emacs@HIDDEN> <83edx89rl4.fsf@HIDDEN>
 <jwvk070qi54.fsf-monnier+emacs@HIDDEN>
X-Spam-Score: -2.3 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> From: Stefan Monnier <monnier@HIDDEN>
> Cc: 57266 <at> debbugs.gnu.org
> Date: Mon, 22 Aug 2022 14:02:44 -0400
> 
> > Which is why I'm perfectly OK with documenting what the code does.
> 
> But I'm not.  Because I don't really know what the code does.
> The doc I wrote documents the design I came up with based on my
> understanding of what the code does.

IME, our "understanding" what the display code does is frequently
flawed, and we discover that too late.

> I went through the trouble of figuring out a design of how things
> *should* work, document it, and adjust the code to match the design.
> So I'm not interested in pushing some half-assed version of it that
> documents a design that doesn't match the reality, nor pushing partial
> fixes/workarounds (for borderline cases that noone ever bumped into
> anyway) while keeping the mess of unexplained hacks.

I agree that if we were writing the display engine today, we'd do many
things differently and more cleanly.  But we are not at that place,
and the (marginal at best) advantages of having the code match your
understanding do not justify the risk of subtle and hard-to-debug
display glitches.

> > Please!  I hope my requests mean something, given that I'm the only
> > one who works on bugs in these areas, and probably will keep doing
> > that for the observable future.
> 
> Indeed, my proposed patch is not a bug fix.
> It's a code-maintenance patch.  It's meant to improve the code, not
> the behavior.

Sorry, I had (and still have from time to time) enough fallout from
similar experiments of yours around Emacs 25, which also aimed at
improving the code (by removing various reasons to perform more
thorough redisplay).  I'd rather not repeat that.

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Stefan Monnier <monnier@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Mon, 22 Aug 2022 19:58:02 +0000
Resent-Message-ID: <handler.57266.B57266.16611982392457 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 57266
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
To: Eli Zaretskii <eliz@HIDDEN>
Cc: 57266 <at> debbugs.gnu.org
Received: via spool by 57266-submit <at> debbugs.gnu.org id=B57266.16611982392457
          (code B ref 57266); Mon, 22 Aug 2022 19:58:02 +0000
Received: (at 57266) by debbugs.gnu.org; 22 Aug 2022 19:57:19 +0000
Received: from localhost ([127.0.0.1]:41822 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1oQDXz-0000dZ-7Z
	for submit <at> debbugs.gnu.org; Mon, 22 Aug 2022 15:57:19 -0400
Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:38651)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <monnier@HIDDEN>) id 1oQDXv-0000dK-RQ
 for 57266 <at> debbugs.gnu.org; Mon, 22 Aug 2022 15:57:18 -0400
Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1])
 by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 418CD80470;
 Mon, 22 Aug 2022 15:57:10 -0400 (EDT)
Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1])
 by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id D480A80723;
 Mon, 22 Aug 2022 15:57:08 -0400 (EDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca;
 s=mail; t=1661198228;
 bh=SWOCGvrF4Em0Qq1O/xQCutlpSN28zoqhQrdN1bZ1QzU=;
 h=From:To:Cc:Subject:In-Reply-To:References:Date:From;
 b=CsIFM3C3mDI5TEe3jTTfgAJZAz9Z1ljb/521NrYm9gHUJSApdLkbfctlxc6hEgxc+
 yK+ElN2kRPJmvAsQ0KV6JFvD6kOANmPodUKWBewedbAbov1+Gnekgk6RM2zonuF4D5
 rCFU9LJEqiI63O4Cnr5/M6z7lzgydoUsMGrX0KbIy6LW8SwbhUBZEWoONTVu/Ocy++
 uIxgjs22Ei5y0V9yokqfI0wPNoddw6cAv9rgYWfgk4+X/U7lhEJvy990kHdBfQ1S9R
 FrAEco6G3TxC7eNjcgPpJDQNqGBJeBvGkt/QzBOvYz6qBPEA0cWf5HZfqbyDyLZloV
 NC8zS2MEnz45g==
Received: from lechazo (lechon.iro.umontreal.ca [132.204.27.242])
 by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id BD5151202E2;
 Mon, 22 Aug 2022 15:57:08 -0400 (EDT)
From: Stefan Monnier <monnier@HIDDEN>
In-Reply-To: <83a67w9ldh.fsf@HIDDEN> (Eli Zaretskii's message of "Mon, 22 Aug
 2022 21:35:54 +0300")
Message-ID: <jwvedx8qcoy.fsf-monnier+emacs@HIDDEN>
References: <jwvzgg2zhzf.fsf@HIDDEN> <831qteccli.fsf@HIDDEN>
 <jwv5yikexc9.fsf-monnier+emacs@HIDDEN> <834jy4bhqp.fsf@HIDDEN>
 <jwvedx8cv4n.fsf-monnier+emacs@HIDDEN> <83y1vga0l5.fsf@HIDDEN>
 <jwv8rngcsk6.fsf-monnier+emacs@HIDDEN> <83edx89rl4.fsf@HIDDEN>
 <jwvk070qi54.fsf-monnier+emacs@HIDDEN> <83a67w9ldh.fsf@HIDDEN>
Date: Mon, 22 Aug 2022 15:57:06 -0400
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-SPAM-INFO: Spam detection results:  0
 ALL_TRUSTED                -1 Passed through trusted hosts only via SMTP
 AWL 0.194 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DKIM_SIGNED               0.1 Message has a DKIM or DK signature,
 not necessarily valid
 DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature
 DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's
 domain T_SCC_BODY_TEXT_LINE    -0.01 -
X-SPAM-LEVEL: 
X-Spam-Score: -2.3 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> Sorry, I had (and still have from time to time) enough fallout from
> similar experiments of yours around Emacs 25, which also aimed at
> improving the code (by removing various reasons to perform more
> thorough redisplay).  I'd rather not repeat that.

I guess you're talking about the `<foo>->redisplay` bits.
FWIW, the main motivation for those was not maintenance but:
- speed (in my daily use I often have a hundred frames and reducing the
  cases where the redisplay has to refresh them all made a significant
  difference to the overall responsiveness of Emacs).
- the addition of `pre-redisplay-functions` so that highlighting of a
  rectangular region could be implemented in ELisp (and at the same
  occasion, remove the ad-hoc handling of the `region` highlighting
  from the C code).


        Stefan

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#57266: Maintaining the base_line_number cache
Resent-From: Lars Ingebrigtsen <larsi@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Tue, 23 Aug 2022 14:23:01 +0000
Resent-Message-ID: <handler.57266.B57266.166126452925614 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 57266
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
To: Eli Zaretskii <eliz@HIDDEN>
Cc: 57266 <at> debbugs.gnu.org, Stefan Monnier <monnier@HIDDEN>
Received: via spool by 57266-submit <at> debbugs.gnu.org id=B57266.166126452925614
          (code B ref 57266); Tue, 23 Aug 2022 14:23:01 +0000
Received: (at 57266) by debbugs.gnu.org; 23 Aug 2022 14:22:09 +0000
Received: from localhost ([127.0.0.1]:44717 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1oQUnB-0006f4-AL
	for submit <at> debbugs.gnu.org; Tue, 23 Aug 2022 10:22:09 -0400
Received: from quimby.gnus.org ([95.216.78.240]:38862)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <larsi@HIDDEN>) id 1oQUn8-0006eZ-Mq
 for 57266 <at> debbugs.gnu.org; Tue, 23 Aug 2022 10:22:08 -0400
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnus.org;
 s=20200322; h=Content-Transfer-Encoding:Content-Type:MIME-Version:Message-ID
 :Date:References:In-Reply-To:Subject:Cc:To:From:Sender:Reply-To:Content-ID:
 Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc
 :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:
 List-Post:List-Owner:List-Archive;
 bh=SmKcCH6wVfAlMcEcjysnrDeHt7YBWDtCYvUpVQdQMfI=; b=YXd+dGDnPcRwaCs8Z/27Lr4d9v
 3mTM7dg3OQ7XDKTWK9pgdJnoQJJ7Hyjg+ntJkDSc+DEp4K0yKi86LuLobqyjVcJgT2iAoy8ocHfxc
 Yaj+pzCVS+sQLEF9R5vqS0gMqe8p66mH4vxYw2AIpLbF6NZ8msBm3HEwizffaaCkfGbU=;
Received: from [84.212.220.105] (helo=joga)
 by quimby.gnus.org with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.92) (envelope-from <larsi@HIDDEN>)
 id 1oQUmy-0000lJ-CS; Tue, 23 Aug 2022 16:21:58 +0200
From: Lars Ingebrigtsen <larsi@HIDDEN>
In-Reply-To: <jwvk070qi54.fsf-monnier+emacs@HIDDEN> (Stefan Monnier via's
 message of "Mon, 22 Aug 2022 14:02:44 -0400")
References: <jwvzgg2zhzf.fsf@HIDDEN> <831qteccli.fsf@HIDDEN>
 <jwv5yikexc9.fsf-monnier+emacs@HIDDEN> <834jy4bhqp.fsf@HIDDEN>
 <jwvedx8cv4n.fsf-monnier+emacs@HIDDEN> <83y1vga0l5.fsf@HIDDEN>
 <jwv8rngcsk6.fsf-monnier+emacs@HIDDEN> <83edx89rl4.fsf@HIDDEN>
 <jwvk070qi54.fsf-monnier+emacs@HIDDEN>
Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAABGdBTUEAALGPC/xhBQAAACBj
 SFJNAAB6JgAAgIQAAPoAAACA6AAAdTAAAOpgAAA6mAAAF3CculE8AAAAFVBMVEWUcpy2lqNPNEQt
 GFupX1e/hnv///9ng8G/AAAAAWJLR0QGYWa4fQAAAAd0SU1FB+YIFw4SDIqrZyoAAAFcSURBVDjL
 ldPNbsMgDABg8tN72Jp7ati9al4gRea+Vfb7v8pskimB0MNQD1G+2gbjGJNW0wKkh3beltkWAEw1
 6COtIV0JzPQ/APTkazVCROZahJdUXIloPY/0XQEAZFrOIMfG+AYiYQnNoACe3wI2djiC3YDwWQVE
 XgZTAYiMeQ1rTb8CNSUYuMkJWXMVEL5+RsYawNMSnGAQAHuVZJUItFepnp9ci0McX9KuvIkbkAfp
 Y97dXoG9bCuDQWEZOcQC0slvrEDni1LwJ4AVgkxQBtrDJ/PUSpEM2gR0BmlIuPBrkqnLQDPhhYMA
 nWEk8B5DVjxsAB7hDPJ5AIbpCOnCgYIGwKHGlEYEGSVAsu2Q/i+NfWmA7LuAJQYf0mMZgdv7HeSz
 71sBXN+7HWRKrlokltCI6LY2uHfmPplJfqbX9v6VcGa2N5n6j7X4vpxe9WC6TwX3OMDDzW5+zO4X
 iqyiS72eP0EAAAAldEVYdGRhdGU6Y3JlYXRlADIwMjItMDgtMjNUMTQ6MTg6MTIrMDA6MDACQ5Oh
 AAAAJXRFWHRkYXRlOm1vZGlmeQAyMDIyLTA4LTIzVDE0OjE4OjEyKzAwOjAwcx4rHQAAAABJRU5E
 rkJggg==
X-Now-Playing: No Bra's _Candy_: "Construction Worker"
Date: Tue, 23 Aug 2022 16:21:55 +0200
Message-ID: <8735dnrqf0.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-Spam-Report: Spam detection software, running on the system "quimby.gnus.org",
 has NOT identified this incoming email as spam.  The original
 message has been attached to this so you can view it or label
 similar future email.  If you have any questions, see
 @@CONTACT_ADDRESS@@ for details.
 Content preview:  Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army
 knife of text editors" <bug-gnu-emacs@HIDDEN> writes: > Indeed, my proposed
 patch is not a bug fix. > It's a code-maintenance patch. It's meant to improve
 the code, not > the behavior. 
 Content analysis details:   (-2.9 points, 5.0 required)
 pts rule name              description
 ---- ---------------------- --------------------------------------------------
 -1.0 ALL_TRUSTED            Passed through trusted hosts only via SMTP
 -1.9 BAYES_00               BODY: Bayes spam probability is 0 to 1%
 [score: 0.0000]
X-Spam-Score: -2.3 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@HIDDEN> writes:

> Indeed, my proposed patch is not a bug fix.
> It's a code-maintenance patch.  It's meant to improve the code, not
> the behavior.

My two cents: I was reading through parts of the newline (cache) code
last week while I was pondering whether to do the `pos-eol' functions or
not, and I have to say that I found it pretty impregnable.  So in a way
I'm relieved to learn that it's (partly) because the code doesn't make
as much sense as it should.  =F0=9F=98=80

Apparently Eli is the only person that understands the code at present,
and I understand his reluctance to change something that works.  But
making the code easier to understand would enable more people to
actually handle the code.  So I'm in favour of improving the code.

Last modified: Tue, 23 Aug 2022 14:30:02 UTC

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