GNU bug report logs - #59678
[PATCH] sh-script: use completion-table-with-cache to improve performance

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 59678 in the body.
You can then email your comments to 59678 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: yikai <at> z1k.dev
To: bug-gnu-emacs <at> gnu.org
Cc: Yikai Zhao <yikai <at> z1k.dev>
Subject: [PATCH] sh-script: use completion-table-with-cache to improve
 performance
Date: Tue, 29 Nov 2022 22:30:14 +0800

Hi,

I found that the auto-completion on .sh files (with company-mode) was pretty slow (takes several seconds).
It turns out that company-mode would call the capf function with 'metadata action for each candidate,
where each function call would need to list all the cmds. In this patch, I modified the code to use
completion-table-with-cache which fixes the problem for me.



---
 lisp/progmodes/sh-script.el | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 408ebfc0451..188b3b73125 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1688,19 +1688,17 @@ sh--vars-before-point
 ;; (defun sh--var-completion-table (string pred action)
 ;;   (complete-with-action action (sh--vars-before-point) string pred))
 
-(defun sh--cmd-completion-table (string pred action)
-  (let ((cmds
-         (append (when (fboundp 'imenu--make-index-alist)
-                   (mapcar #'car
-                           (condition-case nil
-                               (imenu--make-index-alist)
-                             (imenu-unavailable nil))))
-                 (mapcar (lambda (v) (concat v "="))
-                         (sh--vars-before-point))
-                 (locate-file-completion-table
-                  exec-path exec-suffixes string pred t)
-                 sh--completion-keywords)))
-    (complete-with-action action cmds string pred)))
+(defun sh--cmd-completion-table-gen (string)
+  (append (when (fboundp 'imenu--make-index-alist)
+            (mapcar #'car
+                    (condition-case nil
+                        (imenu--make-index-alist)
+                      (imenu-unavailable nil))))
+          (mapcar (lambda (v) (concat v "="))
+                  (sh--vars-before-point))
+          (locate-file-completion-table
+           exec-path exec-suffixes string nil t)
+          sh--completion-keywords))
 
 (defun sh-completion-at-point-function ()
   (save-excursion
@@ -1713,7 +1711,7 @@ sh-completion-at-point-function
         (list start end (sh--vars-before-point)
               :company-kind (lambda (_) 'variable)))
        ((sh-smie--keyword-p)
-        (list start end #'sh--cmd-completion-table
+        (list start end (completion-table-with-cache #'sh--cmd-completion-table-gen)
               :company-kind
               (lambda (s)
                 (cond
-- 
2.38.1

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: yikai <at> z1k.dev, Stefan Monnier <monnier <at> iro.umontreal.ca> 
Cc: 59678 <at> debbugs.gnu.org
Subject: Re: bug#59678: [PATCH] sh-script: use completion-table-with-cache to
 improve performance
Date: Thu, 01 Dec 2022 17:46:01 +0200

> Cc: Yikai Zhao <yikai <at> z1k.dev>
> From: yikai <at> z1k.dev
> Date: Tue, 29 Nov 2022 22:30:14 +0800
> 
> Hi,
> 
> I found that the auto-completion on .sh files (with company-mode) was pretty slow (takes several seconds).
> It turns out that company-mode would call the capf function with 'metadata action for each candidate,
> where each function call would need to list all the cmds. In this patch, I modified the code to use
> completion-table-with-cache which fixes the problem for me.

Stefan, any comments?

>  lisp/progmodes/sh-script.el | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
> index 408ebfc0451..188b3b73125 100644
> --- a/lisp/progmodes/sh-script.el
> +++ b/lisp/progmodes/sh-script.el
> @@ -1688,19 +1688,17 @@ sh--vars-before-point
>  ;; (defun sh--var-completion-table (string pred action)
>  ;;   (complete-with-action action (sh--vars-before-point) string pred))
>  
> -(defun sh--cmd-completion-table (string pred action)
> -  (let ((cmds
> -         (append (when (fboundp 'imenu--make-index-alist)
> -                   (mapcar #'car
> -                           (condition-case nil
> -                               (imenu--make-index-alist)
> -                             (imenu-unavailable nil))))
> -                 (mapcar (lambda (v) (concat v "="))
> -                         (sh--vars-before-point))
> -                 (locate-file-completion-table
> -                  exec-path exec-suffixes string pred t)
> -                 sh--completion-keywords)))
> -    (complete-with-action action cmds string pred)))
> +(defun sh--cmd-completion-table-gen (string)
> +  (append (when (fboundp 'imenu--make-index-alist)
> +            (mapcar #'car
> +                    (condition-case nil
> +                        (imenu--make-index-alist)
> +                      (imenu-unavailable nil))))
> +          (mapcar (lambda (v) (concat v "="))
> +                  (sh--vars-before-point))
> +          (locate-file-completion-table
> +           exec-path exec-suffixes string nil t)
> +          sh--completion-keywords))
>  
>  (defun sh-completion-at-point-function ()
>    (save-excursion
> @@ -1713,7 +1711,7 @@ sh-completion-at-point-function
>          (list start end (sh--vars-before-point)
>                :company-kind (lambda (_) 'variable)))
>         ((sh-smie--keyword-p)
> -        (list start end #'sh--cmd-completion-table
> +        (list start end (completion-table-with-cache #'sh--cmd-completion-table-gen)
>                :company-kind
>                (lambda (s)
>                  (cond
> -- 
> 2.38.1
> 
> 
> 
> 
> 

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yikai <at> z1k.dev, 59678 <at> debbugs.gnu.org
Subject: Re: bug#59678: [PATCH] sh-script: use completion-table-with-cache
 to improve performance
Date: Thu, 01 Dec 2022 15:30:04 -0500

>> I found that the auto-completion on .sh files (with company-mode) was
>> pretty slow (takes several seconds).  It turns out that company-mode
>> would call the capf function with 'metadata action for each
>> candidate, where each function call would need to list all the
>> cmds. In this patch, I modified the code to use
>> completion-table-with-cache which fixes the problem for me.
>
> Stefan, any comments?

Looks OK to me.  The description of the problem looks concerning, tho:
if Company really calls the completion table with `metadata` once per
candidate, then there's a bug in company.


        Stefan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: yikai <at> z1k.dev, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 59678-done <at> debbugs.gnu.org
Subject: Re: bug#59678: [PATCH] sh-script: use completion-table-with-cache to
 improve performance
Date: Fri, 02 Dec 2022 15:44:11 +0200

> Cc: Yikai Zhao <yikai <at> z1k.dev>
> From: yikai <at> z1k.dev
> Date: Tue, 29 Nov 2022 22:30:14 +0800
> 
> Hi,
> 
> I found that the auto-completion on .sh files (with company-mode) was pretty slow (takes several seconds).
> It turns out that company-mode would call the capf function with 'metadata action for each candidate,
> where each function call would need to list all the cmds. In this patch, I modified the code to use
> completion-table-with-cache which fixes the problem for me.

Thanks, installed on the emacs-29 branch.

In the future, please accompany your changes with ChangeLog-style commit of
messages (see CONTRIBUTE for the details).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: yikai <at> z1k.dev, 59678 <at> debbugs.gnu.org
Subject: Re: bug#59678: [PATCH] sh-script: use completion-table-with-cache
 to improve performance
Date: Fri, 02 Dec 2022 15:45:14 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: yikai <at> z1k.dev,  59678 <at> debbugs.gnu.org
> Date: Thu, 01 Dec 2022 15:30:04 -0500
> 
> >> I found that the auto-completion on .sh files (with company-mode) was
> >> pretty slow (takes several seconds).  It turns out that company-mode
> >> would call the capf function with 'metadata action for each
> >> candidate, where each function call would need to list all the
> >> cmds. In this patch, I modified the code to use
> >> completion-table-with-cache which fixes the problem for me.
> >
> > Stefan, any comments?
> 
> Looks OK to me.  The description of the problem looks concerning, tho:
> if Company really calls the completion table with `metadata` once per
> candidate, then there's a bug in company.

Thanks, I therefore installed the changes.

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

From: Yikai Zhao <yikai <at> z1k.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 59678-done <at> debbugs.gnu.org
Subject: Re: bug#59678: [PATCH] sh-script: use completion-table-with-cache to
 improve performance
Date: Sat, 3 Dec 2022 01:00:00 +0800

On Fri, Dec 2, 2022 at 9:44 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > Cc: Yikai Zhao <yikai <at> z1k.dev>
> > From: yikai <at> z1k.dev
> > Date: Tue, 29 Nov 2022 22:30:14 +0800
> >
> > Hi,
> >
> > I found that the auto-completion on .sh files (with company-mode) was pretty slow (takes several seconds).
> > It turns out that company-mode would call the capf function with 'metadata action for each candidate,
> > where each function call would need to list all the cmds. In this patch, I modified the code to use
> > completion-table-with-cache which fixes the problem for me.
>
> Thanks, installed on the emacs-29 branch.
>
> In the future, please accompany your changes with ChangeLog-style commit of
> messages (see CONTRIBUTE for the details).

Thanks. I will do that next time.

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

From: Yikai Zhao <yikai <at> z1k.dev>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 59678 <at> debbugs.gnu.org
Subject: Re: bug#59678: [PATCH] sh-script: use completion-table-with-cache to
 improve performance
Date: Sat, 3 Dec 2022 01:01:32 +0800

On Fri, Dec 2, 2022 at 4:30 AM Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
>
> >> I found that the auto-completion on .sh files (with company-mode) was
> >> pretty slow (takes several seconds).  It turns out that company-mode
> >> would call the capf function with 'metadata action for each
> >> candidate, where each function call would need to list all the
> >> cmds. In this patch, I modified the code to use
> >> completion-table-with-cache which fixes the problem for me.
> >
> > Stefan, any comments?
>
> Looks OK to me.  The description of the problem looks concerning, tho:
> if Company really calls the completion table with `metadata` once per
> candidate, then there's a bug in company.

Thanks!

I again confirmed that this is the current behavior.

I created an issue for company-mode to follow up.
https://github.com/company-mode/company-mode/issues/1351

>
>
>         Stefan
>

Previous Next

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