From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#57266: Maintaining the base_line_number cache Date: Wed, 17 Aug 2022 17:18:44 -0400 Message-ID: Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="37909"; mail-complaints-to="usenet@ciao.gmane.io" To: 57266@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Aug 17 23:20:27 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oOQSh-0009eI-7Q for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 17 Aug 2022 23:20:27 +0200 Original-Received: from localhost ([::1]:43994 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oOQSf-0000fU-RT for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 17 Aug 2022 17:20:26 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:35026) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oOQSI-0000ey-TT for bug-gnu-emacs@gnu.org; Wed, 17 Aug 2022 17:20:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:35198) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oOQSI-0006lM-L1 for bug-gnu-emacs@gnu.org; Wed, 17 Aug 2022 17:20:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oOQSI-0005zu-Ga for bug-gnu-emacs@gnu.org; Wed, 17 Aug 2022 17:20:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 17 Aug 2022 21:20:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 57266 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.166077115022973 (code B ref -1); Wed, 17 Aug 2022 21:20:02 +0000 Original-Received: (at submit) by debbugs.gnu.org; 17 Aug 2022 21:19:10 +0000 Original-Received: from localhost ([127.0.0.1]:53180 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oOQRR-0005yS-W1 for submit@debbugs.gnu.org; Wed, 17 Aug 2022 17:19:10 -0400 Original-Received: from lists.gnu.org ([209.51.188.17]:40176) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oOQRN-0005yI-Ay for submit@debbugs.gnu.org; Wed, 17 Aug 2022 17:19:08 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:54034) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oOQRN-00006X-4I for bug-gnu-emacs@gnu.org; Wed, 17 Aug 2022 17:19:05 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:20672) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oOQRK-0006d6-7M for bug-gnu-emacs@gnu.org; Wed, 17 Aug 2022 17:19:04 -0400 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 54E8B442597 for ; Wed, 17 Aug 2022 17:19:00 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 9F9C0442590 for ; Wed, 17 Aug 2022 17:18:53 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1660771133; bh=4+CPgY9UOQtHwMKpMt6SkwydHA+NayEIWDqdQsQJCqg=; h=From:To:Subject:Date:From; b=kGwVGzvJ1ygJyYkFl3vhYX7oHU0ruFj4xNAIv5Lisb+D56fRBKWwCcYmmZT4wMpVM zZVFDuYr+FINYxn3qEgVJzW+MSenq9rBqVh+t00XtXwPmuT7uSjt/EdHX9N1VQeoc7 L62kfAoDF85ZnMJnJ+YWsXyCp4lkMPsaCi4W7N0RTuIeyyPfSL69DqlpBmtR5OYi69 agUnjGhKRBiVmou4MlLgmgTmr0lN8SdYhjR8ZIc9nTSPJuyKHIK3pje4sEjByQ0z2p MSYe/H+92TlJxDhyzkivfTk30PFr8qfH1l1+NfWwnRB20w9fE7buxhoUoh76/JSiFH 24nKP4sPr5oQw== Original-Received: from lechazo (lechon.iro.umontreal.ca [132.204.27.242]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 86E401203E4 for ; Wed, 17 Aug 2022 17:18:53 -0400 (EDT) Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; 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-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:240114 Archived-At: --=-=-= 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 * 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); } --=-=-=--