GNU bug report logs - #48738
[PATCH] Optimized `describe-variable' predicate

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 48738 in the body.
You can then email your comments to 48738 AT debbugs.gnu.org in the normal way.

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

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

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: bug-gnu-emacs <at> gnu.org
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: [PATCH] Optimized `describe-variable' predicate
Date: Sun, 30 May 2021 06:27:16 +0200

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

Do not switch to the original buffer.  Use `buffer-local-value`
instead.  The buffer switching is more costly, which slows down
continuously updating completion UIs like Icomplete or Vertico if
there are many variables.

Ideally there would be a `buffer-local-boundp` predicate we could use
here.

In GNU Emacs 28.0.50 (build 12, x86_64-pc-linux-gnu, GTK+ Version
3.24.5, cairo version 1.16.0)
 of 2021-05-23 built on projects
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux 10 (buster)

[0001-describe-variable-Do-not-switch-to-the-original-buff.patch (text/x-diff, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: 48738 <at> debbugs.gnu.org
Subject: Re: bug#48738: [PATCH] Optimized `describe-variable' predicate
Date: Mon, 31 May 2021 06:55:21 +0200

Daniel Mendler <mail <at> daniel-mendler.de> writes:

> Do not switch to the original buffer.  Use `buffer-local-value`
> instead.  The buffer switching is more costly, which slows down
> continuously updating completion UIs like Icomplete or Vertico if
> there are many variables.

Thanks; applied to Emacs 28 with some changes -- I thought the
`ignore-errors' logic was kinda unclear, so I rewrote that bit using an
explicit `condition-case' checking for the error thrown in this case.

Adjusted patch included below; hopefully I didn't introduce any errors
here...

> Ideally there would be a `buffer-local-boundp` predicate we could use
> here.

Yes, that would be nice, I think.

And we probably have a lot of code in Emacs that uses
`with-current-buffer' instead of `buffer-local-value' that could be
similarly optimised (but I guess it mostly only makes sense to do so
when it's called in an intensive loop like here).

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 666583db72..c8f078cb85 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -1024,12 +1024,15 @@ describe-variable
                 (format-prompt "Describe variable" (and (symbolp v) v))
                 #'help--symbol-completion-table
                 (lambda (vv)
-                  ;; In case the variable only exists in the buffer
-                  ;; the command we switch back to that buffer before
-                  ;; we examine the variable.
-                  (with-current-buffer orig-buffer
-                    (or (get vv 'variable-documentation)
-                        (and (boundp vv) (not (keywordp vv))))))
+                  (or (get vv 'variable-documentation)
+                      (and (not (keywordp vv))
+                           ;; Since the variable may only exist in the
+                           ;; original buffer, we have to look for it
+                           ;; there.
+                           (condition-case nil
+                               (buffer-local-value vv orig-buffer)
+                             (:success t)
+                             (void-variable nil)))))
                 t nil nil
                 (if (symbolp v) (symbol-name v))))
      (list (if (equal val "")


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 48738 <at> debbugs.gnu.org
Subject: Re: bug#48738: [PATCH] Optimized `describe-variable' predicate
Date: Mon, 31 May 2021 07:00:06 +0200

On 5/31/21 6:55 AM, Lars Ingebrigtsen wrote:
> Thanks; applied to Emacs 28 with some changes -- I thought the
> `ignore-errors' logic was kinda unclear, so I rewrote that bit using an
> explicit `condition-case' checking for the error thrown in this case.

Makes sense. Thanks for applying the patch.

> Adjusted patch included below; hopefully I didn't introduce any errors
> here...
> 
>> Ideally there would be a `buffer-local-boundp` predicate we could use
>> here.
> 
> Yes, that would be nice, I think.
> 
> And we probably have a lot of code in Emacs that uses
> `with-current-buffer' instead of `buffer-local-value' that could be
> similarly optimised (but I guess it mostly only makes sense to do so
> when it's called in an intensive loop like here).

Yes, when I find such case I will report them or provide a patch.
However it really matters in very few cases. Here the predicate is
called 10k+ times when generating all completions.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: 48738 <at> debbugs.gnu.org
Subject: Re: bug#48738: [PATCH] Optimized `describe-variable' predicate
Date: Mon, 31 May 2021 07:06:20 +0200

Daniel Mendler <mail <at> daniel-mendler.de> writes:

> Yes, when I find such case I will report them or provide a patch.
> However it really matters in very few cases. Here the predicate is
> called 10k+ times when generating all completions.

Yup.

I think I'll just go ahead and add `buffer-local-boundp' now -- it'll
perhaps encourage further usage of `buffer-local-value'.  I remember
doing things like

(with-current-buffer foo
  (and (boundp 'zot) zot))

because there's no `buffer-local-boundp'.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: 48738 <at> debbugs.gnu.org
Subject: Re: bug#48738: [PATCH] Optimized `describe-variable' predicate
Date: Mon, 31 May 2021 07:13:03 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> I think I'll just go ahead and add `buffer-local-boundp' now -- it'll
> perhaps encourage further usage of `buffer-local-value'.  I remember
> doing things like
>
> (with-current-buffer foo
>   (and (boundp 'zot) zot))
>
> because there's no `buffer-local-boundp'.

local-variable-p and local-variable-if-set-p, but neither really do what
you want in this case.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 48738 <at> debbugs.gnu.org
Subject: Re: bug#48738: [PATCH] Optimized `describe-variable' predicate
Date: Mon, 31 May 2021 07:19:27 +0200

On 5/31/21 7:13 AM, Lars Ingebrigtsen wrote:
> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> 
>> I think I'll just go ahead and add `buffer-local-boundp' now -- it'll
>> perhaps encourage further usage of `buffer-local-value'.  I remember
>> doing things like
>>
>> (with-current-buffer foo
>>   (and (boundp 'zot) zot))
>>
>> because there's no `buffer-local-boundp'.
> 
> local-variable-p and local-variable-if-set-p, but neither really do what
> you want in this case.

Yes, I am aware of those. As you say, they don't do what we want here.
The question is which name convention to use here. I think having a
`buffer-local-boundp` makes sense.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: 48738 <at> debbugs.gnu.org
Subject: Re: bug#48738: [PATCH] Optimized `describe-variable' predicate
Date: Mon, 31 May 2021 07:23:51 +0200

Daniel Mendler <mail <at> daniel-mendler.de> writes:

> Yes, I am aware of those. As you say, they don't do what we want here.
> The question is which name convention to use here. I think having a
> `buffer-local-boundp` makes sense.

Added now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 48738 <at> debbugs.gnu.org
Subject: Re: bug#48738: [PATCH] Optimized `describe-variable' predicate
Date: Mon, 31 May 2021 07:30:23 +0200

On 5/31/21 7:23 AM, Lars Ingebrigtsen wrote:
> Daniel Mendler <mail <at> daniel-mendler.de> writes:
> 
>> Yes, I am aware of those. As you say, they don't do what we want here.
>> The question is which name convention to use here. I think having a
>> `buffer-local-boundp` makes sense.
> 
> Added now.

Thanks. While we are on it optimizing, does it make sense to make this a
c function? You can probably tell more about the overhead of condition-case?

However I think `buffer-local-boundp` is really only performance
critical in this single predicate in `describe-variable`. I cannot think
of another use case iterating over thousands of symbols and checking for
bound variables. So probably we are good now.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: 48738 <at> debbugs.gnu.org
Subject: Re: bug#48738: [PATCH] Optimized `describe-variable' predicate
Date: Mon, 31 May 2021 07:32:53 +0200

Daniel Mendler <mail <at> daniel-mendler.de> writes:

> Thanks. While we are on it optimizing, does it make sense to make this a
> c function? You can probably tell more about the overhead of condition-case?

Making it a C function might make sense, but condition-case is really
fast -- it's basically negligible, if I remember previous benchmarkings
of that correctly.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

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