GNU bug report logs - #68234
[PATCH] Increase blink-matching-paren-distance by 300kb.

Previous Next

To reply to this bug, email your comments to 68234 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

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

From: Antero Mejr <antero <at> mailbox.org>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Increase blink-matching-paren-distance by 300kb.
Date: Wed, 03 Jan 2024 18:37:08 +0000

[Message part 1 (text/plain, inline)]

Avoids erroneous mismatched paren highlighting when highlighting large
expressions of around 10,000 lines.

This value was last increased in 2009, from 25->100kb. This patch
increases it from 100->400.

[0001-Increase-blink-matching-paren-distance-by-300kb.patch (text/x-patch, attachment)]

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

From: Antero Mejr <antero <at> mailbox.org>
To: 68234 <at> debbugs.gnu.org
Subject: Re: [PATCH] Increase blink-matching-paren-distance by 300kb.
Date: Wed, 03 Jan 2024 18:48:41 +0000

[Message part 1 (text/plain, inline)]

Messed up the code comment on the last patch, please use this patch
instead (also added Bug# to commit message).

[0001-Increase-blink-matching-paren-distance-by-300kb.patch (text/x-patch, attachment)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Antero Mejr <antero <at> mailbox.org>, 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Wed, 3 Jan 2024 20:50:41 -0800

Antero Mejr via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> writes:

> Avoids erroneous mismatched paren highlighting when highlighting large
> expressions of around 10,000 lines.
>
> This value was last increased in 2009, from 25->100kb. This patch
> increases it from 100->400.

Makes sense to me.

> From d55e8ce5d98389bc8bbe4dac97bc76bb957e791e Mon Sep 17 00:00:00 2001
> From: Antero Mejr <antero <at> mailbox.org>
> Date: Wed, 3 Jan 2024 18:29:43 +0000
> Subject: [PATCH] Increase blink-matching-paren-distance by 300kb.
>
> * lisp/simple.el (blink-matching-paren-distance): Increase from 100kb
> to 400kb.
> ---
>  lisp/simple.el | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/simple.el b/lisp/simple.el
> index 4f6d2ee12c3..177ea640506 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -9260,10 +9260,10 @@ It is also ignored if `show-paren-mode' is enabled."
>    :type 'boolean
>    :group 'paren-blinking)
>
> -(defcustom blink-matching-paren-distance (* 100 1024)
> +(defcustom blink-matching-paren-distance (* 400 1024)
>    "If non-nil, maximum distance to search backwards for matching open-paren.
>  If nil, search stops at the beginning of the accessible portion of the buffer."
> -  :version "23.2"                       ; 25->100k
> +  :version "23.2"                       ; 25->100k, 30->400k

This should read:

 +  :version "30.1"                       ; 100->400k

Or even just:

 +  :version "30.1"

>    :type '(choice (const nil) integer)
>    :group 'paren-blinking)
>
> --
> 2.43.0

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Antero Mejr <antero <at> mailbox.org>,
 Stefan Kangas <stefankangas <at> gmail.com>
Cc: 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Thu, 04 Jan 2024 13:31:17 +0200

> Date: Wed, 03 Jan 2024 18:48:41 +0000
> From:  Antero Mejr via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> >From 8875e658456074e92272bcbb6d31c8db47d4658a Mon Sep 17 00:00:00 2001
> From: Antero Mejr <antero <at> mailbox.org>
> Date: Wed, 3 Jan 2024 18:29:43 +0000
> Subject: [PATCH] Increase blink-matching-paren-distance by 300kb.
> 
> * lisp/simple.el (blink-matching-paren-distance): Increase from 100kb
> to 400kb.  (Bug#68234)
> ---
>  lisp/simple.el | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lisp/simple.el b/lisp/simple.el
> index 4f6d2ee12c3..d4a339f37dc 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -9260,10 +9260,10 @@ It is also ignored if `show-paren-mode' is enabled."
>    :type 'boolean
>    :group 'paren-blinking)
>  
> -(defcustom blink-matching-paren-distance (* 100 1024)
> +(defcustom blink-matching-paren-distance (* 400 1024)
>    "If non-nil, maximum distance to search backwards for matching open-paren.
>  If nil, search stops at the beginning of the accessible portion of the buffer."
> -  :version "23.2"                       ; 25->100k
> +  :version "23.2"                       ; 25->100k->400k
>    :type '(choice (const nil) integer)
>    :group 'paren-blinking)

Do people feel okay with increasing this value by a factor of 4?  This
blinking is supposed to be very fast, so if this will slow down
display, it's not TRT.

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

From: Antero Mejr <antero <at> mailbox.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Thu, 04 Jan 2024 19:22:52 +0000

[Message part 1 (text/plain, inline)]

Stefan Kangas <stefankangas <at> gmail.com> writes:

> This should read:
>
>  +  :version "30.1"                       ; 100->400k
>
> Or even just:
>
>  +  :version "30.1"
>

New patch attached, removed the comment.

[0001-Increase-blink-matching-paren-distance-by-300kb.patch (text/x-patch, attachment)]

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

From: Daniel Martín <mardani29 <at> yahoo.es>
To: Antero Mejr via "Bug reports for GNU Emacs, the Swiss army knife of text
 editors" <bug-gnu-emacs <at> gnu.org>
Cc: Antero Mejr <antero <at> mailbox.org>, 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Thu, 04 Jan 2024 22:03:19 +0100

Antero Mejr via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> writes:

> Avoids erroneous mismatched paren highlighting when highlighting large
> expressions of around 10,000 lines.
>

Do you have a practical example of those expressions that are 10,000
lines long?  I think they are rare enough that people affected by
possible paren mismatching with them could simply tweak
blink-matching-paren-distance in their customization file.

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

From: Antero Mejr <antero <at> mailbox.org>
To: Daniel Martín <mardani29 <at> yahoo.es>
Cc: 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Thu, 04 Jan 2024 21:36:09 +0000

Daniel Martín <mardani29 <at> yahoo.es> writes:

> Do you have a practical example of those expressions that are 10,000
> lines long?  I think they are rare enough that people affected by
> possible paren mismatching with them could simply tweak
> blink-matching-paren-distance in their customization file.

First time seeing it was an SXML expression for a website (about 6k
lines), second time was an allowlist filter (about 10k lines).

I don't think there is a noticable performance penalty in searching a
wider region, which is why I proposed increasing the default.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Antero Mejr <antero <at> mailbox.org>
Cc: 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Thu, 4 Jan 2024 15:08:35 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> -(defcustom blink-matching-paren-distance (* 100 1024)
>> +(defcustom blink-matching-paren-distance (* 400 1024)
>>    "If non-nil, maximum distance to search backwards for matching open-paren.
>>  If nil, search stops at the beginning of the accessible portion of the buffer."
>> -  :version "23.2"                       ; 25->100k
>> +  :version "23.2"                       ; 25->100k->400k
>>    :type '(choice (const nil) integer)
>>    :group 'paren-blinking)
>
> Do people feel okay with increasing this value by a factor of 4?  This
> blinking is supposed to be very fast, so if this will slow down
> display, it's not TRT.

I benchmarked this on my old ~2013 GNU/Linux machine using:

  (progn
  (goto-char (point-max))
  (insert "}")
  (let ((minibuffer-message-timeout 0)
        (blink-matching-delay 0)
        (repeat 20) fst)
    (dolist (distance (number-sequence 100 1000 100))
      (let* ((blink-matching-paren-distance (* distance 1024))
             (time (car (benchmark-run repeat (blink-matching-open)))))
        (message "distance %-4dKB takes %-.3fs (x%.1f)"
                 distance
                 (/ time repeat)
                 (/ time (setq fst (or fst time))))))))

In src/lisp.h (176 KB/5607 lines), I get:

  distance 100 KB takes 0.011s (x1.0)
  distance 200 KB takes 0.015s (x1.3)
  distance 300 KB takes 0.015s (x1.3)
  distance 400 KB takes 0.015s (x1.3) *
  [etc.]

In src/xterm.c (945 KB/32842 lines), I get:

  distance 100 KB takes 0.010s (x1.0)
  distance 200 KB takes 0.017s (x1.6)
  distance 300 KB takes 0.025s (x2.4)
  distance 400 KB takes 0.034s (x3.3) *
  distance 500 KB takes 0.042s (x4.1)
  distance 600 KB takes 0.051s (x5.0)
  distance 700 KB takes 0.060s (x5.8)
  distance 800 KB takes 0.068s (x6.7)
  distance 900 KB takes 0.078s (x7.6)
  distance 1000KB takes 0.079s (x7.7)

I also tried this in a ridiculous 23 MB header file that I found in the
Linux kernel tree, which my Emacs opens in fundamental-mode:

  distance 100 KB takes 0.005s (x1.0)
  distance 200 KB takes 0.005s (x1.0)
  distance 300 KB takes 0.008s (x1.5)
  distance 400 KB takes 0.010s (x2.0) *
  distance 500 KB takes 0.013s (x2.5)
  distance 600 KB takes 0.015s (x3.1)
  distance 700 KB takes 0.018s (x3.6)
  distance 800 KB takes 0.020s (x4.1)
  distance 900 KB takes 0.023s (x4.6)
  distance 1000KB takes 0.025s (x5.1)

Humans can't perceive delays smaller than 100 ms, so my conclusion here
is that setting this to 400 KB shouldn't normally lead to any noticeable
slowdown.  On the other hand, we get improved correctness, which sounds
like a win.

We also have to take into account the compound effect with other
features (such as third-party packages) that also may be slowing things
down.  However, this should only affect typing closing parens, so maybe
correctness could still be considered more important.

Of course, we could also double (instead of quadruple) this number, and
get most of the benefits still.

Eli, do you have any preference?

Note finally that the above examined files are unusually large (as is
often the case in our tree).

PS. Out of interest, in a recent copy of the Linux kernel the median
    *.[ch] file size is ~5 KB.  In emacs/src, we have instead a median
    *.[ch] file size of ~21KB.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: antero <at> mailbox.org, 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Fri, 05 Jan 2024 09:49:00 +0200

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Thu, 4 Jan 2024 15:08:35 -0800
> Cc: 68234 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Do people feel okay with increasing this value by a factor of 4?  This
> > blinking is supposed to be very fast, so if this will slow down
> > display, it's not TRT.
> 
> I benchmarked this on my old ~2013 GNU/Linux machine using:
> 
>   (progn
>   (goto-char (point-max))
>   (insert "}")
>   (let ((minibuffer-message-timeout 0)
>         (blink-matching-delay 0)
>         (repeat 20) fst)
>     (dolist (distance (number-sequence 100 1000 100))
>       (let* ((blink-matching-paren-distance (* distance 1024))
>              (time (car (benchmark-run repeat (blink-matching-open)))))
>         (message "distance %-4dKB takes %-.3fs (x%.1f)"
>                  distance
>                  (/ time repeat)
>                  (/ time (setq fst (or fst time))))))))
> 
> In src/lisp.h (176 KB/5607 lines), I get:
> 
>   distance 100 KB takes 0.011s (x1.0)
>   distance 200 KB takes 0.015s (x1.3)
>   distance 300 KB takes 0.015s (x1.3)
>   distance 400 KB takes 0.015s (x1.3) *
>   [etc.]
> 
> In src/xterm.c (945 KB/32842 lines), I get:
> 
>   distance 100 KB takes 0.010s (x1.0)
>   distance 200 KB takes 0.017s (x1.6)
>   distance 300 KB takes 0.025s (x2.4)
>   distance 400 KB takes 0.034s (x3.3) *
>   distance 500 KB takes 0.042s (x4.1)
>   distance 600 KB takes 0.051s (x5.0)
>   distance 700 KB takes 0.060s (x5.8)
>   distance 800 KB takes 0.068s (x6.7)
>   distance 900 KB takes 0.078s (x7.6)
>   distance 1000KB takes 0.079s (x7.7)

Based on these times, I don't recommend raising the threshold above
200K.

> I also tried this in a ridiculous 23 MB header file that I found in the
> Linux kernel tree, which my Emacs opens in fundamental-mode:

I don't think Fundamental mode is relevant: everything is
lightning-fast there.  I would be interested in modes like Python,
though, if this is relevant for them.

> Humans can't perceive delays smaller than 100 ms

That's not true: 50 ms is already perceptible.  Moreover, this
particular time is an add-on to the redisplay time after cursor
movement, so adding even a few tens of msec could easily pass the
perception threshold.  And finally, some people, including on this
list, use machines older and much slower than those from 10 years ago.

> Of course, we could also double (instead of quadruple) this number, and
> get most of the benefits still.
> 
> Eli, do you have any preference?

I think we can at most double it at this time.  This is a defcustom,
so people can always increase it in their local configurations if they
have much faster machines and need routinely to cope with such large
parenthetical expressions.

> Note finally that the above examined files are unusually large (as is
> often the case in our tree).

What matters is not the size of the file, but the size of a
parenthetical expression.

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

From: Antero Mejr <antero <at> mailbox.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Stefan Kangas <stefankangas <at> gmail.com>, 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Fri, 05 Jan 2024 20:50:45 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

> I think we can at most double it at this time.  This is a defcustom,
> so people can always increase it in their local configurations if they
> have much faster machines and need routinely to cope with such large
> parenthetical expressions.

If we're not increasing it that much, can we change the behavior to not
wrongly indicate mismatched parens for expressions where you can't be
sure? Patch is attached.

[0001-Don-t-show-mismatched-parens-if-search-region-is-nar.patch (text/x-patch, attachment)]

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

From: Antero Mejr <antero <at> mailbox.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Stefan Kangas <stefankangas <at> gmail.com>, 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Fri, 05 Jan 2024 21:10:10 +0000

[Message part 1 (text/plain, inline)]

Antero Mejr <antero <at> mailbox.org> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>> I think we can at most double it at this time.  This is a defcustom,
>> so people can always increase it in their local configurations if they
>> have much faster machines and need routinely to cope with such large
>> parenthetical expressions.
>
> If we're not increasing it that much, can we change the behavior to not
> wrongly indicate mismatched parens for expressions where you can't be
> sure? Patch is attached.

Updated version of the last patch, accounts for when
blink-matching-paren-distance is nil.

[0001-Don-t-show-mismatched-parens-if-search-region-is-nar.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Antero Mejr <antero <at> mailbox.org>
Cc: stefankangas <at> gmail.com, 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Sat, 06 Jan 2024 09:01:30 +0200

> From: Antero Mejr <antero <at> mailbox.org>
> Cc: Stefan Kangas <stefankangas <at> gmail.com>,  68234 <at> debbugs.gnu.org
> Date: Fri, 05 Jan 2024 20:50:45 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > I think we can at most double it at this time.  This is a defcustom,
> > so people can always increase it in their local configurations if they
> > have much faster machines and need routinely to cope with such large
> > parenthetical expressions.
> 
> If we're not increasing it that much, can we change the behavior to not
> wrongly indicate mismatched parens for expressions where you can't be
> sure? Patch is attached.

AFAIU, this will show nothing when the matching parenthesis is not
found?  If so, why is that a good idea?

In general, I don't understand why this rare situation should draw so
much attention.  Does this happen to you frequently?  If so, does it
happen with many files?  You could solve this either by changing the
value as a file-local variable or globally by customizing the limit in
your init files.  What I don't understand is why should we make
significant changes that will affect everyone, when the problem is
rare and affects only you or a small number of users?  User options
exist because no single value can ever be good for everyone or every
situation, so the right solution for such cases is to customize the
option.

Or what am I missing?

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Antero Mejr <antero <at> mailbox.org>
Cc: 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Sat, 6 Jan 2024 04:09:14 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

> AFAIU, this will show nothing when the matching parenthesis is not
> found?  If so, why is that a good idea?

I guess the point is to avoid false positives: Emacs confidently says
"no matching paren", but actually there is one.  We just didn't try hard
enough to find it.

On the other hand, perhaps we could just adjust the message to make it
more clear: "no matching parenthesis within 100000 characters" or
something like that (but only if the buffer is indeed longer).  (But
then I wouldn't bother with the (* 100 1024) part, and just do 100000 or
200000 outright to improve the display.  This is a nit though.)

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: antero <at> mailbox.org, 68234 <at> debbugs.gnu.org
Subject: Re: bug#68234: [PATCH] Increase blink-matching-paren-distance by
 300kb.
Date: Mon, 8 Jan 2024 21:56:05 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

> I think we can at most double it at this time.  This is a defcustom,
> so people can always increase it in their local configurations if they
> have much faster machines and need routinely to cope with such large
> parenthetical expressions.

Sounds good to me.

Note that performance only comes into play for ending parens, and if
there is no matching open paren.

>> Note finally that the above examined files are unusually large (as is
>> often the case in our tree).
>
> What matters is not the size of the file, but the size of a
> parenthetical expression.

That would be the best case.

The file size is the upper bound if there's no matching opening paren
(the worst case).

Previous Next

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