From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#57266: Maintaining the base_line_number cache Date: Mon, 22 Aug 2022 15:11:26 +0300 Message-ID: <834jy4bhqp.fsf@gnu.org> References: <831qteccli.fsf@gnu.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="34618"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 57266@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Aug 22 14:12:39 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 1oQ6II-0008qA-UB for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 22 Aug 2022 14:12:38 +0200 Original-Received: from localhost ([::1]:41710 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oQ6IH-0003RH-QI for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 22 Aug 2022 08:12:37 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:45580) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oQ6Hj-0003MR-CP for bug-gnu-emacs@gnu.org; Mon, 22 Aug 2022 08:12:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:48482) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oQ6Hj-0005tU-3I for bug-gnu-emacs@gnu.org; Mon, 22 Aug 2022 08:12:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oQ6Hi-00087g-T5 for bug-gnu-emacs@gnu.org; Mon, 22 Aug 2022 08:12:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 22 Aug 2022 12:12:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 57266 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 57266-submit@debbugs.gnu.org id=B57266.166117029331173 (code B ref 57266); Mon, 22 Aug 2022 12:12:02 +0000 Original-Received: (at 57266) by debbugs.gnu.org; 22 Aug 2022 12:11:33 +0000 Original-Received: from localhost ([127.0.0.1]:38229 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oQ6HE-00086i-U9 for submit@debbugs.gnu.org; Mon, 22 Aug 2022 08:11:33 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:50680) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oQ6HD-00086W-2c for 57266@debbugs.gnu.org; Mon, 22 Aug 2022 08:11:31 -0400 Original-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 ) 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==; Original-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 ) id 1oQ6H5-0005dV-8W; Mon, 22 Aug 2022 08:11:24 -0400 In-Reply-To: (message from Stefan Monnier on Mon, 22 Aug 2022 00:34:42 -0400) 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:240414 Archived-At: > From: Stefan Monnier > Cc: 57266@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.