bug#41423: 27.0.91; tramp regression on pretest

bug#41423: 27.0.91; tramp regression on pretest

[rrandresf]

1. emacs -Q
2. Evaluate snippet below:
--8<---------------cut here---------------start------------->8---
(let ((default-directory "/ssh:user@5quince:~/")) (eshell))
--8<---------------cut here---------------end--------------->8---
3.  On the eshell buffer I do
--8<---------------cut here---------------start------------->8---
cd dev/c/h {hit TAB key for getting completion do its job and press Return key}
--8<---------------cut here---------------end--------------->8---

Observation: On emacs-26.3 it is inmediate. But on pretest it takes almost 3m to
complete same the action.

andres.ramirez
ps: just in case completion does cd dev/c/hildon/


In GNU Emacs 27.0.91 (build 1, armv7l-unknown-linux-gnueabihf, X toolkit, Xaw3d scroll bars)
System Description: Arch Linux ARM

Recent messages:
Loading em-basic...done
Loading em-cmpl...done
Loading em-dirs...done
Loading em-glob...done
Loading em-hist...done
Loading em-ls...done
Loading em-prompt...done
Loading em-script...done
Loading em-term...done
Loading em-unix...done
#<buffer *eshell*>
Configured using:
 'configure '--program-transform-name=s/^ctags$/ctags.emacs/'
 --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/usr/share --with-x-toolkit=lucid
 --mandir=/usr/share/man --pdfdir=/usr/share/doc/emacs --with-modules
 --with-xft --without-gconf --without-gsettings --with-imagemagick
 --without-xwidgets --without-pop --with-gameuser=:games
 --disable-build-details 'CFLAGS=-march=armv7-a -mfloat-abi=hard
 -mfpu=vfpv3-d16 -O2 -pipe -fstack-protector-strong -fno-plt'
 CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

Configured features:
XAW3D XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GLIB NOTIFY
INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS LUCID X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON
PDUMPER LCMS2 GMP

Important settings:
  value of $LC_COLLATE: C
  value of $LANG: C
  locale-coding-system: nil

Major mode: Eshell

Minor modes in effect:
  eshell-prompt-mode: t
  eshell-hist-mode: t
  eshell-pred-mode: t
  eshell-cmpl-mode: t
  eshell-proc-mode: t
  eshell-arg-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs text-property-search mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils pcmpl-unix em-unix
em-term term easymenu ehelp em-script em-prompt em-ls em-hist em-pred
em-glob em-dirs esh-var em-cmpl em-basic em-banner em-alias tramp-cache
tramp-sh esh-mode eshell esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg
esh-module esh-groups esh-util tramp tramp-loaddefs trampver
tramp-integration files-x tramp-compat shell pcomplete comint ansi-color
ring parse-time iso8601 time-date ls-lisp format-spec auth-source cl-seq
eieio eieio-core cl-macs eieio-loaddefs cl-loaddefs cl-lib
password-cache json subr-x map seq term/screen term/xterm xterm byte-opt
gv bytecomp byte-compile cconv disp-table tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify lcms2 dynamic-setting
font-render-setting x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 8 141067 10597)
 (symbols 24 9524 1)
 (strings 16 41154 2442)
 (string-bytes 1 1131300)
 (vectors 8 19393)
 (vector-slots 4 599625 10098)
 (floats 8 51 242)
 (intervals 28 265 162)
 (buffers 576 13))

bug#41423: additional info

[andrés ramírez]

More info.
It also delays if I replace cd by find-file on the eshell buffer. BTW
completion needs to happen for the delay to appear.

andres.ramirez

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest] (was: bug#41423: additional info)

[Michael Albinus]

andrés ramírez <rrandresf@gmail.com> writes:

Hi,

> It also delays if I replace cd by find-file on the eshell buffer. BTW
> completion needs to happen for the delay to appear.

I've bisected the emacs-27 branch for this. The first bad commit is

--8<---------------cut here---------------start------------->8---
commit 047c1b19353ff58d8cd45935c7b44c911b70e312 (HEAD, refs/bisect/bad)
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Tue Mar 19 23:41:20 2019 -0400

    * lisp/eshell/em-cmpl.el: Use completion-at-point i.s.o pcomplete
--8<---------------cut here---------------end--------------->8---

Since this is a serious regression, I'm marking this as blocking for
Emacs 27. (Sorry Eli, but you can always overrule me)

> andres.ramirez

Best regards, Michael.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Tim Vaughan via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Just a heads up: I believe I'm experiencing this bug under 27.1.  (Before finding this bug report I also did a bisection and found the same culprit commit that Andres did.).  Thus I expect it somehow manage to escape pretest.

Additionally, I've noticed that although the effect is severe only in tramp directories, there is a noticeable (~0.5s) pause when executing commands in local directories, provided autocompletion has been used to create the command string.  (The exact same command does not seem to cause the delay when typed in without invoking autocompletion.)

I'm not at all familiar with the eshell code, but I spent some time trying to identify the cause.  Weirdly the behaviour does not seem to occur when I instrument `eshell-send-input', making it a bit of a heisenbug.

Tim

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

The root of this bug is that `eshell-complete-commands-list' loops through 
all executables *on the remote server* with (while comps-in-path ...). 
That means typically 1000-2000 commands to check, one by one, hence the 3 
minutes delay.

The easy fix is to hit C-g, which stops this loop.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
> The root of this bug is that `eshell-complete-commands-list' loops 
> through all executables *on the remote server* with (while comps-in-path 
> ...). That means typically 1000-2000 commands to check, one by one, 
> hence the 3 minutes delay.
>
> The easy fix is to hit C-g, which stops this loop.
>

Another note: in fact this bug exists because 
`eshell-complete-commands-list' is, in this context, called in Emacs 27, 
but not in Emacs 26 and earlier.

The backtrace is:

* eshell-complete-commands-list()
   #f(compiled-function () #<bytecode 0x1e0009b1b5c5>)()
   pcomplete--here(#f(compiled-function () #<bytecode 0x1e0009b1b5c5>) nil nil nil)
   #f(compiled-function () #<bytecode 0x1fff46e348cd94>)()
   pcomplete-completions()
   pcomplete-completions-at-point()
   #f(compiled-function () #<bytecode 0xcbc6f2b2c706bdb>)()
   completion-in-region--postch()

In Emacs 26 `pcomplete--here' is called only once, with 
`pcomplete-entries' and `file-directory-p' in its `form' argument, and 
`pcomplete-completions' returns.

In Emacs 27 `pcomplete--here' is called twice with these arguments, and a 
third time with `eshell-complete-commands-list' as its `form' argument. 
It is this third call which takes about three minutes to complete.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

A last note: this bug exists because in Emacs 27 eshell uses 
`pcomplete-completions-at-point': TAB is bound to `completion-at-point' 
and `completion-at-point-functions' is `(pcomplete-completions-at-point 
t)'.

In Emacs 26 eshell used the (now obsolete) `pcomplete' function: TAB was 
bound to `eshell-pcomplete', which was defined as follows:

(defun eshell-pcomplete (&optional interactively)
    "Eshell wrapper for `pcomplete'."
    (interactive "p")
    (setq this-command 'pcomplete)
    (condition-case nil
        (if interactively
  	  (call-interactively 'pcomplete)
  	(pcomplete))
      (text-read-only (completion-at-point))))

IOW, pcomplete-completions-at-point was called only if `pcomplete' failed, 
and is now called by default.  (`completion-at-point-functions' was set to 
`(pcomplete-completions-at-point t)', as in Emacs 27.)

A simple fix is to eval

(setq completion-at-point-functions '(pcomplete t))

after starting eshell (or to put this in one of the eshell hooks), which 
will restore the previous default behavior.

I have no idea how `pcomplete-completions-at-point' should be adapted to 
avoid this bug.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Apparently my previous last note was not the last one ;-)

I still don't know how this bug should be fixed (except by using (setq 
completion-at-point-functions '(pcomplete t))), but here is a more 
detailed explanation of what is happening, at least how I understand it:

1. TAB calls completion-at-point
2. completion-at-point calls pcomplete-completions-at-point, which calls pcomplete/cd; this completes the directory name
3. completion-at-point let-binds completion-in-region-mode-predicate to a lambda, which contains pcomplete-completion-at-point
4. completion-at-point calls completion-in-region, which adds completion-in-region--postch to post-command-hook
5. post-command-hook calls completion-in-region--postch
6. completion-in-region--postch funcalls completion-in-region-mode-predicate
7. this calls pcomplete-completions-at-point a second time, which again calls pcomplete/cd (and adds a '/' after the directory name (?))
8. RET is pressed
9. post-command-hook still contains completion-in-region--postch: it is called again
10. completion-in-region--postch funcalls completion-in-region-mode-predicate again
11. this calls pcomplete-completions-at-point a third (!) time
12. at this point pcomplete-completions-at-point considers (for some reason, possibly because the last character of the input is '/' (?)) that it is a command that it must now complete
13. therefore instead of calling pcomplete/cd a third time, pcomplete-completions-at-point now calls eshell-complete-commands-list
14. this loops through all possible command names (on the local or remote host)
15. if all command names had had a common prefix, that prefix would have been inserted now, but this is not the case, so the effect of this loop (apart from a waste of time) is nil

The fact that default-directory is remote is not important here, the exact 
same steps take place when it is local, except that step 15 is executed 
much faster.

At step 15 it is possible to interrupt the loop with C-g.  At step 8 it is 
possible to remove completion-in-region--postch from post-command-hook for 
example by switching buffers with C-x C-b.

Why such a overly complicated mechanism is used, or what should be done to 
avoid this behavior, is beyond my understanding.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Michael Albinus]

[-- Attachment #1: Type: text/plain, Size: 2798 bytes --]

Gregory Heytings <ghe@sdf.org> writes:

Hi,

> Apparently my previous last note was not the last one ;-)
>
> I still don't know how this bug should be fixed (except by using (setq
> completion-at-point-functions '(pcomplete t))), but here is a more
> detailed explanation of what is happening, at least how I understand
> it:
>
> 1. TAB calls completion-at-point
> 2. completion-at-point calls pcomplete-completions-at-point, which
> calls pcomplete/cd; this completes the directory name
> 3. completion-at-point let-binds completion-in-region-mode-predicate
> to a lambda, which contains pcomplete-completion-at-point
> 4. completion-at-point calls completion-in-region, which adds
> completion-in-region--postch to post-command-hook
> 5. post-command-hook calls completion-in-region--postch
> 6. completion-in-region--postch funcalls completion-in-region-mode-predicate
> 7. this calls pcomplete-completions-at-point a second time, which
> again calls pcomplete/cd (and adds a '/' after the directory name (?))
> 8. RET is pressed
> 9. post-command-hook still contains completion-in-region--postch: it
> is called again
> 10. completion-in-region--postch funcalls
> completion-in-region-mode-predicate again
> 11. this calls pcomplete-completions-at-point a third (!) time
> 12. at this point pcomplete-completions-at-point considers (for some
> reason, possibly because the last character of the input is '/' (?))
> that it is a command that it must now complete
> 13. therefore instead of calling pcomplete/cd a third time,
> pcomplete-completions-at-point now calls eshell-complete-commands-list
> 14. this loops through all possible command names (on the local or remote host)
> 15. if all command names had had a common prefix, that prefix would
> have been inserted now, but this is not the case, so the effect of
> this loop (apart from a waste of time) is nil
>
> The fact that default-directory is remote is not important here, the
> exact same steps take place when it is local, except that step 15 is
> executed much faster.
>
> At step 15 it is possible to interrupt the loop with C-g.  At step 8
> it is possible to remove completion-in-region--postch from
> post-command-hook for example by switching buffers with C-x C-b.
>
> Why such a overly complicated mechanism is used, or what should be
> done to avoid this behavior, is beyond my understanding.

I don't know the completion machinery, so I'm adding Stefan who might
know better.

However, the remote case could be improved. Tramp uses caches. They
expire after a while (10 seconds per default), but this might be
improved. The appended patch disables Tramp cache expiry while being in
eshell-complete-commands-list, so completion might be faster once the
cache has been filled. Could you pls check?

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 816 bytes --]

*** /tmp/ediff046iSD	2020-08-29 14:36:21.788278641 +0200
--- /usr/local/src/emacs-27/lisp/eshell/em-cmpl.el	2020-08-29 14:22:38.781483169 +0200
***************
*** 399,405 ****

  (defun eshell-complete-commands-list ()
    "Generate list of applicable, visible commands."
!   (let ((filename (pcomplete-arg)) glob-name)
      (if (file-name-directory filename)
          (if eshell-force-execution
              (pcomplete-dirs-or-entries nil #'file-readable-p)
--- 399,405 ----

  (defun eshell-complete-commands-list ()
    "Generate list of applicable, visible commands."
!   (let ((filename (pcomplete-arg)) glob-name remote-file-name-inhibit-cache)
      (if (file-name-directory filename)
          (if eshell-force-execution
              (pcomplete-dirs-or-entries nil #'file-readable-p)

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Michael,

>
> However, the remote case could be improved. Tramp uses caches. They 
> expire after a while (10 seconds per default), but this might be 
> improved. The appended patch disables Tramp cache expiry while being in 
> eshell-complete-commands-list, so completion might be faster once the 
> cache has been filled. Could you pls check?
>

I just checked, and see no visible improvement.

That being said, I don't think this bug should be fixed on the eshell 
level.  It's a bug in the completion mechanisms, and 
`eshell-complete-commands-list' should simply not be called here.

Gregory

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Stefan Monnier]

>> 1. TAB calls completion-at-point
>> 2. completion-at-point calls pcomplete-completions-at-point, which
>> calls pcomplete/cd; this completes the directory name

Sounds like a bug: the `completion-at-point-functions` should not
perform the completion but should only return a description of the part
of the text that is the subject of completion, along with a description
(generally in the form of a function) of the set of elements from which
the possible completions can be taken.


        Stefan

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> 1. TAB calls completion-at-point
>> 2. completion-at-point calls pcomplete-completions-at-point, which calls pcomplete/cd; this completes the directory name
>
> Sounds like a bug: the `completion-at-point-functions` should not 
> perform the completion but should only return a description of the part 
> of the text that is the subject of completion, along with a description 
> (generally in the form of a function) of the set of elements from which 
> the possible completions can be taken.
>

Please don't take my words too literally.  "This completes the directory 
name" does not mean that `pcomplete/cd' performs the completion itself. 
Indeed it returns something with which the completion is performed.

The bug is clearly not there (at step 2), but later (in the fact that 
`completion-at-point' calls `pcomplete-completions-at-point' three times).

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Michael Albinus]

Gregory Heytings <ghe@sdf.org> writes:

> Hi Michael,

Hi Gregory,

>> However, the remote case could be improved. Tramp uses caches. They
>> expire after a while (10 seconds per default), but this might be
>> improved. The appended patch disables Tramp cache expiry while being
>> in eshell-complete-commands-list, so completion might be faster once
>> the cache has been filled. Could you pls check?
>>
>
> I just checked, and see no visible improvement.

The first time you try completion, there's no difference. The cache must
be filled. But all next times you complete, it shall be faster for
remote directoriues.

> That being said, I don't think this bug should be fixed on the eshell
> level.  It's a bug in the completion mechanisms, and
> `eshell-complete-commands-list' should simply not be called here.

I do not claim my patch is the solution. But it shall be useful, if
eshell-complete-commands-list is called somewhere.

> Gregory

Best regards, Michael.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
>> I just checked, and see no visible improvement.
>
> The first time you try completion, there's no difference. The cache must 
> be filled. But all next times you complete, it shall be faster for 
> remote directoriues.
>

Yes, this is what I did.  The first time there was no difference.  The 
second time I did not see any difference either, alas.  I just tried it 
again.

>
> I do not claim my patch is the solution. But it shall be useful, if 
> eshell-complete-commands-list is called somewhere.
>

A much better patch at this point is to just restore the previous default 
behavior (in Emacs 26 and earlier) by setting 
completion-at-point-functions to '(pcomplete t) instead of 
'(pcomplete-completions-at-point t), for example with:

(add-hook 'eshell-mode-hook (function (lambda () (setq completion-at-point-functions '(pcomplete t)))))

pcomplete is obsolete, but it is still there, and it works.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Michael Albinus]

Gregory Heytings <ghe@sdf.org> writes:

> A much better patch at this point is to just restore the previous
> default behavior (in Emacs 26 and earlier) by setting
> completion-at-point-functions to '(pcomplete t) instead of
> '(pcomplete-completions-at-point t), for example with:
>
> (add-hook 'eshell-mode-hook (function (lambda () (setq
> completion-at-point-functions '(pcomplete t)))))
>
> pcomplete is obsolete, but it is still there, and it works.

I guess Stefan had a reason when he applied
047c1b19353ff58d8cd45935c7b44c911b70e312 last year. Let's see his
analysis.

Best regards, Michael.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Stefan Monnier]

> Please don't take my words too literally.  "This completes the directory
> name" does not mean that `pcomplete/cd' performs the completion
> itself.  Indeed it returns something with which the completion is performed.

Hmm... any hope you could refine your description accordingly?

> The bug is clearly not there (at step 2), but later (in the fact that
> `completion-at-point' calls `pcomplete-completions-at-point' three times).

Calling `pcomplete-completions-at-point` several times is not
necessarily a problem.  E.g. it's normal to call it a second time after
completion to check whether we're still in the same completion area (in
order to detect when completion is "finished").


        Stefan

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
> Hmm... any hope you could refine your description accordingly?
>

I do not understand why I should explain to you how the code you wrote 
works.  Anyway, here we go (and I fear you will now tell me that my 
description is now too refined):

1. start emacs -Q
2. in an eshell buffer, type "<command> <first letters of a directory name> TAB" (command can be "cd", "ls", "rm", ...)
3. TAB calls completion-at-point
4. completion-at-point looks at completion-at-point-functions, whose value is (pcomplete-completions-at-point t), and calls pcomplete-completions-at-point
5. pcomplete-completions-at-point calls pcomplete-completions, which calls pcomplete/cd, which calls pcomplete--here, ...
6. pcomplete-completions returns a collection of completion candidates, and pcomplete-completions-at-point returns that collection together with a function pointer (to pcomplete-completions-at-point itself), a start position, an end position, and a property list
7. we are now back in completion-at-point, and enter the second case in its pcase
8. completion-at-point let-binds completion-in-region-mode-predicate to a lambda, which calls pcomplete-completion-at-point
9. completion-at-point then calls completion-in-region, which calls completion--in-region
10. completion--in-region enters completion-in-region-mode, which adds completion-in-region--postch to post-command-hook
11. completion--in-region calls completion--in-region-1, which calls completion--do-completion, which finally does the actual completion based on the collection of completion candidates returned at step 6
12. at this point the eshell buffer contains the completed directory name, with a trailing slash
13. completion-at-point is now finished, and post-command-hook is executed
14. post-command-hook calls completion-in-region--postch
15. completion-in-region--postch calls completion-in-region-mode-predicate (in fact, completion-in-region-mode--predicate which has been set to completion-in-region-mode-predicate when entering completion-in-region-mode at step 10)
16. this calls pcomplete-completions-at-point a second time, which calls pcomplete-completions, which calls pcomplete/cd, which calls pcomplete--here, ...
17. pcomplete/cd and pcomplete-completions return the exact same values as in step 6
18. pcomplete-completions-at-point returns almost the same value as in step 6 (the only difference is the value of the end position)
19. given that the value of the start position did not change, the lambda let-bound at step 8 returns t, and therefore completion-in-region--postch exits completion-in-region-mode entered at step 10
20. completion-in-region--postch is now finished, it did not change anything in the eshell buffer
21. RET is pressed
22. post-command-hook is executed, and still contains completion-in-region--postch, so it is called again
23. completion-in-region--postch calls completion-in-region-mode-predicate
24. this calls pcomplete-completions-at-point a third time, which calls pcomplete-completions
25. for some reason, pcomplete-completions considers that it must now complete a command name and not a directory name
26. therefore pcomplete-completions does not call pcomplete/cd but eshell-complete-commands-list
27. eshell-complete-commands-list loops through all possible command names 
28. if these command names had a common prefix, it would have been inserted in the eshell buffer (?), but this is not the case, so the effect of this loop (apart from a waste of time) is nil
29. when eshell-complete-commands-list has finished its job, eshell prints its next prompt

At step 27 it is possible to interrupt the loop with C-g.  At step 21 it 
is possible to remove completion-in-region--postch from post-command-hook, 
for example by switching buffers with C-x C-b.

>
> Calling `pcomplete-completions-at-point` several times is not 
> necessarily a problem.  E.g. it's normal to call it a second time after 
> completion to check whether we're still in the same completion area (in 
> order to detect when completion is "finished").
>

For some general case , I don't know (I admit I can't think of a case in 
which this would be useful, but I know my experience is limited).  To 
complete a directory name in a shell, I don't see why this should be the 
case.  The (now obsolete) mechanism calls pcomplete (which also calls 
pcomplete/cd) a single time, and it worked.  I don't see what steps 13-29 
could possibly do that would be useful, at least in the context of a 
shell.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Two corrections:

Step 19 should be:

19. given that the value of the start position did not change, the lambda let-bound at step 8 returns t, and therefore completion-in-region--postch does not exit completion-in-region-mode

And the last steps should be:

29. when eshell-complete-commands-list has finished its job, pcomplete-completions-at-point returns a value in which the start position has changed
30. therefore the lambda let-bound at step 8 returns nil, and therefore completion-in-region--postch exits completion-in-region-mode entered at step 10
31. this removes completion-in-region--postch from post-command-hook
32. eshell finally prints its next prompt

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Stefan Monnier]

> I do not understand why I should explain to you how the code you wrote
> works.

[ Because I remember ow it's supposed to work, but I don't know how it
  actually behaves in this specific case.  ]

> 19. given that the value of the start position did not change, the lambda
>     let-bound at step 8 returns t, and therefore completion-in-region--postch
>     does not exit completion-in-region-mode
> 20. completion-in-region--postch is now finished, it did not change
>     anything in the eshell buffer
> 21. RET is pressed
> 22. post-command-hook is executed, and still contains
>     completion-in-region--postch, so it is called again
> 23. completion-in-region--postch calls completion-in-region-mode-predicate
> 24. this calls pcomplete-completions-at-point a third time, which calls
>     pcomplete-completions

Looks OK so far.

> 25. for some reason, pcomplete-completions considers that it must now
>     complete a command name and not a directory name

I guess this is because after RET we're now at BOL so it looks like
a brand new command is starting.

> 26. therefore pcomplete-completions does not call pcomplete/cd but
>     eshell-complete-commands-list

I guess this is the culprit and this is where the time is spent.

`eshell-complete-commands-list` eagerly builds the list of possible
candidates and it takes a while whereas we should here return something
quickly and cheaply, e.g. by returning a function which will build and
return this list more lazily when completion is actually performed.

Of course, the slowdown will presumably still be seen when you do
actually want to complete a command name, so we should probably try and
figure out more precisely where the slowdown comes from and how to
avoid/reduce it.

I guess part of the slowdown comes from the fact that we don't just use
`file-name-all-completions` in each directory in PATH but we
additionally call `file-executable-p` (or `file-readable-p`) on every
command found, which I expect will take quite a while when it goes
through Tramp.

Still, I'm not completely sure where the time is spent because I'm not
sure which files/dirs will go through tramp.  AFAICT, `eshell-get-path`
will return the "local" $PATH rather than that of the remote host ... oh
wait, no I see that `tramp-eshell-directory-change` will set that to the
remote host's $PATH, so it should indeed work correctly (but slowly).

Maybe `tramp-eshell-directory-change` should tell
`eshell-complete-commands-list` to cache the list and also not to bother
checking `file-executable-p`?

> And the last steps should be:
>
> 29. when eshell-complete-commands-list has finished its job,
> pcomplete-completions-at-point returns a value in which the start position
> has changed
> 30. therefore the lambda let-bound at step 8 returns nil, and therefore
> completion-in-region--postch exits completion-in-region-mode entered at step
> 10
> 31. this removes completion-in-region--postch from post-command-hook
> 32. eshell finally prints its next prompt

Yes, these steps look fine.


        Stefan

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
>> 26. therefore pcomplete-completions does not call pcomplete/cd but 
>> eshell-complete-commands-list
>
> I guess this is the culprit
>

No, see below.

>
> and this is where the time is spent.
>

Yes, this is what I said.

>
> `eshell-complete-commands-list` eagerly builds the list of possible 
> candidates and it takes a while whereas we should here return something 
> quickly and cheaply, e.g. by returning a function which will build and 
> return this list more lazily when completion is actually performed.
>

No, the bug is in the completion mechanism, not in eshell.  I don't know 
exactly (because the mechanism is so complicated) where the completion 
functions should be fixed, but it is clear that there is no reason to call 
pcomplete-completions-at-point *three* times.

There is no reason to call pcomplete-completions-at-point when RET is 
pressed.

Typing "cd foo/ RET" does not call pcomplete-completions-at-point. 
Typing "cd foo TAB RET" should only call pcomplete-completions-at-point to 
add the trailing slash, and should not build a list of all possible 
commands.

>
> Of course, the slowdown will presumably still be seen when you do 
> actually want to complete a command name, so we should probably try and 
> figure out more precisely where the slowdown comes from and how to 
> avoid/reduce it.
>

That's another, separate issue, and it is not relevant here.  BTW, when 
you want to complete a command name, at least you have some characters in 
the prefix (or if you don't, you expect that listing all candidates will 
be slow).

Typing "cd TAB" works almost instantaneously (and prints "Sole 
completion"), even on a remote host.  Typing "ls TAB" works almost 
instantaneously, too, even on a remote host.  It prints "Complete, but not 
unique", and pressing TAB again lists the alternate completion candidates. 
Typing "l TAB" takes (on a remote host) five to ten seconds before 
printing all completion candidates, but again this is what a user expects.

Nobody expects that typing RET, when the completion has already been done, 
would take three minutes.

>
> Maybe `tramp-eshell-directory-change` should tell 
> `eshell-complete-commands-list` to cache the list and also not to bother 
> checking `file-executable-p`?
>

That's yet another, separate issue, that is not relevant here.  Indeed 
there are possible improvements in eshell and tramp, but this bug is in 
the completion mechanism, not in eshell or in tramp.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Eli Zaretskii]

On September 1, 2020 11:31:14 AM GMT+03:00, "Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> wrote:
>
>>
>>> 26. therefore pcomplete-completions does not call pcomplete/cd but 
>>> eshell-complete-commands-list
>>
>> I guess this is the culprit
>>
>
>No, see below.
>
>>
>> and this is where the time is spent.
>>
>
>Yes, this is what I said.
>
>>
>> `eshell-complete-commands-list` eagerly builds the list of possible 
>> candidates and it takes a while whereas we should here return
>something 
>> quickly and cheaply, e.g. by returning a function which will build
>and 
>> return this list more lazily when completion is actually performed.
>>
>
>No, the bug is in the completion mechanism, not in eshell.  I don't
>know 
>exactly (because the mechanism is so complicated) where the completion 
>functions should be fixed, but it is clear that there is no reason to
>call 
>pcomplete-completions-at-point *three* times.

Would it help to profile the completion process in this use case using the built-in Lisp profiler?

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
> Would it help to profile the completion process in this use case using 
> the built-in Lisp profiler?
>

No, because the bug is not about cpu or mem usage, but about doing 
something useless that just takes time (without eating much resources).

A much better way to profile this is to use:

(defvar pcomplete-completions-at-point-time 0)
(defun around-pcomplete-completions-at-point (fun)
   (message "calling pcomplete-completions-at-point")
   (setq pcomplete-completions-at-point-time (float-time))
   (let ((ret (funcall fun)))
     (message "returning from pcomplete-completions-at-point, call took %.2f seconds" (- (float-time) pcomplete-completions-at-point-time))
     ret))
(advice-add 'pcomplete-completions-at-point :around #'around-pcomplete-completions-at-point)

(let ((default-directory "/ssh:user@host:~/")) (eshell))

This will print:

calling pcomplete-completions-at-point
returning from pcomplete-completions-at-point, call took 0.00 seconds
calling pcomplete-completions-at-point
returning from pcomplete-completions-at-point, call took 0.00 seconds
calling pcomplete-completions-at-point
returning from pcomplete-completions-at-point, call took N seconds

The value of N depends on the speed of your connection.  On a fast 
connection it will be something around 50, on a slower one something 
around 100.  With a local directory (that is, without let-binding 
default-directory before entering eshell) it depends on your machine.  On 
a fast one it will be something around 0.10, on a slower one something 
around 0.50.

Note again that this third call to pcomplete-completions-at-point does 
nothing useful.  It just build a list of all possible commands, and throws 
it away.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Stefan Monnier]

>> `eshell-complete-commands-list` eagerly builds the list of possible
>> candidates and it takes a while whereas we should here return something
>> quickly and cheaply, e.g. by returning a function which will build and
>> return this list more lazily when completion is actually performed.
> No, the bug is in the completion mechanism, not in eshell.  I don't know
> exactly (because the mechanism is so complicated) where the completion
> functions should be fixed, but it is clear that there is no reason to call
> pcomplete-completions-at-point *three* times.

The reason why it does so, is that it wants to know when a "completion
session" terminates, e.g. to hide the *Completions* buffer or to run the
exit-function.

So it calls it:

- once to do the actual completion.
- once per command in post-command-hook to see if we're done.

Since in your example you have 2 commands (TAB and RET), that gives you
a total of 3.  This design relies on the fact that completion tables can
be lazy, so it should always be possible to make the
completion-at-point-function very cheap and harmless, so it's OK to call
it repeatedly (or even needlessly).

> There is no reason to call pcomplete-completions-at-point when RET
> is pressed.

If running that function is costly, it's a bug.

That's how completion-at-point-functions was designed.

If you want to change that design, be my guest, but it likely implies
changes to a fair bit of code, including outside Emacs.

>> Of course, the slowdown will presumably still be seen when you do actually
>> want to complete a command name, so we should probably try and figure out
>> more precisely where the slowdown comes from and how to avoid/reduce it.
> That's another, separate issue, and it is not relevant here.

Agreed.


        Stefan

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Stefan Monnier]

> This will print:
>
> calling pcomplete-completions-at-point
> returning from pcomplete-completions-at-point, call took 0.00 seconds
> calling pcomplete-completions-at-point
> returning from pcomplete-completions-at-point, call took 0.00 seconds
> calling pcomplete-completions-at-point
> returning from pcomplete-completions-at-point, call took N seconds
[...]
> Note again that this third call to pcomplete-completions-at-point does
> nothing useful.  It just build a list of all possible commands, and throws
> it away.

Exactly.

And the fix is to make this third call return in 0.00 seconds like the
others by making it return not the list of commands but a mere function
(which will return that list of commands only when called, but in the
present case it won't be called).


        Stefan

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Stefan Monnier]

> And the fix is to make this third call return in 0.00 seconds like the
> others by making it return not the list of commands but a mere function
> (which will return that list of commands only when called, but in the
> present case it won't be called).

See for example patch below (which shouldn't be applied as-is since the
body of the function ends up misindented).


        Stefan


diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
index 48c99acac3..e41afea9ef 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -399,11 +399,15 @@
 
 (defun eshell-complete-commands-list ()
   "Generate list of applicable, visible commands."
-  (let ((filename (pcomplete-arg)) glob-name)
+  ;; Building the commands list can take quite a while over Tramp
+  ;;  (bug#41423), so do it lazily.
+  (completion-table-dynamic
+   (lambda (filename)
     (if (file-name-directory filename)
         (if eshell-force-execution
             (pcomplete-dirs-or-entries nil #'file-readable-p)
           (pcomplete-executables))
+       (let (glob-name)
       (if (and (> (length filename) 0)
 	       (eq (aref filename 0) eshell-explicit-command-char))
 	  (setq filename (substring filename 1)
@@ -455,7 +459,7 @@
 			    (and eshell-show-lisp-alternatives
 				 (null completions)))
 			(all-completions filename obarray #'functionp))
-		   completions)))))))
+		      completions)))))))))
 
 (define-obsolete-function-alias 'eshell-pcomplete #'completion-at-point "27.1")
 

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
>> No, the bug is in the completion mechanism, not in eshell.  I don't 
>> know exactly (because the mechanism is so complicated) where the 
>> completion functions should be fixed, but it is clear that there is no 
>> reason to call pcomplete-completions-at-point *three* times.
>
> The reason why it does so, is that it wants to know when a "completion 
> session" terminates, e.g. to hide the *Completions* buffer or to run the 
> exit-function.
>
> So it calls it:
>
> - once to do the actual completion.
> - once per command in post-command-hook to see if we're done.
>
> Since in your example you have 2 commands (TAB and RET), that gives you 
> a total of 3.
>

Hmmm...  It seems to me that in this case, we're done after the first call 
to pcomplete-completions-at-point, so the second call to 
pcomplete-completions-at-point in post-command-hook should see this (or at 
least could see this), and remove completion-in-region--postch from 
post-command-hook.  RET would then not call pcomplete-completions-at-point 
anymore.

>
> This design relies on the fact that completion tables can be lazy, so it 
> should always be possible to make the completion-at-point-function very 
> cheap and harmless, so it's OK to call it repeatedly (or even 
> needlessly).
>

This is not at all documented AFAICS.  Given that it's a crucial aspect 
for your design to work, it should be.

>
>> There is no reason to call pcomplete-completions-at-point when RET is 
>> pressed.
>
> If running that function is costly, it's a bug.
>

It was not before you declared `pcomplete' obsolete and removed 
`eshell-pcomplete'.

>
> That's how completion-at-point-functions was designed.
>
> If you want to change that design, be my guest, but it likely implies 
> changes to a fair bit of code, including outside Emacs.
>

I don't want to change that design, but I ask myself why you documented 
`pcomplete-completions-at-point' as follows:

"Provide standard completion using pcomplete's completion tables. Same as 
`pcomplete' but using the standard completion UI."

It's NOT the same as `pcomplete', it relies on different conditions.  All 
completion functions called by `pcomplete-completions-at-point' should be 
checked and possibly changed with this new design.

Given this, why did you declared `pcomplete' obsolete (it would make sense 
to have both a simple mechanism for simple cases and a more complex one 
for more complex cases), and why did you remove `eshell-pcomplete'?

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
> See for example patch below (which shouldn't be applied as-is since the 
> body of the function ends up misindented).
>

Indeed this patch works.

But now my question is: whould it not be possible to do this (namely, 
returning a lazy completion table) in one of the pcomplete-* functions (in 
`pcomplete-completions-at-point' itself, or in `pcomplete-completions', 
or...), instead of doing this in the individual functions ultimately 
called by `pcomplete-completions-at-point'?

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Stefan Monnier]

>> Since in your example you have 2 commands (TAB and RET), that gives you
>> a total of 3.
> Hmmm...  It seems to me that in this case, we're done after the first call
> to pcomplete-completions-at-point,

Usually it depends on whether the completion inserts a separator
(e.g. a space in the case of Eshell/Pcomplete).  Otherwise, point is
still placed "within" (tho probably right at the END position) the
completion area and hence completion is not considered to be finished.
You can refine/tune the completion tables so that completion will
considered as finished in more cases, but that wouldn't solve the
problem in the general case anyway.

>> This design relies on the fact that completion tables can be lazy, so it
>> should always be possible to make the completion-at-point-function very
>> cheap and harmless, so it's OK to call it repeatedly (or even needlessly).
> This is not at all documented AFAICS.  Given that it's a crucial aspect for
> your design to work, it should be.

The lispref says the following:

    The functions on this hook should generally return quickly, since they
    may be called very often (e.g., from @code{post-command-hook}).
    Supplying a function for @var{collection} is strongly recommended if
    generating the list of completions is an expensive operation.  Emacs
    may internally call functions in @code{completion-at-point-functions}
    many times, but care about the value of @var{collection} for only some
    of these calls.  By supplying a function for @var{collection}, Emacs
    can defer generating completions until necessary.  You can use
    @code{completion-table-dynamic} to create a wrapper function:

>>> There is no reason to call pcomplete-completions-at-point when RET
>>> is pressed.
>> If running that function is costly, it's a bug.
> It was not before you declared `pcomplete' obsolete and removed
> `eshell-pcomplete'.

Indeed, making the Pcomplete infrastructure compatible with the standard
completion UI introduced some incompatibilities.

This was true before `pcomplete` was declared obsolete, tho: it has been
true since the introduction of `pcomplete-completions-at-point` (some
time around Emacs-24, IIRC).

> "Provide standard completion using pcomplete's completion tables. Same as
> `pcomplete' but using the standard completion UI."
> It's NOT the same as `pcomplete', it relies on different conditions.
> All completion functions called by `pcomplete-completions-at-point' should
> be checked and possibly changed with this new design.

In practice it works fairly well for most cases, but yes some functions
needed to be changed, and others remain, obviously.

> Given this, why did you declared `pcomplete' obsolete (it would make sense
> to have both a simple mechanism for simple cases and a more complex one for
> more complex cases), and why did you remove `eshell-pcomplete'?

There are several questions in there.  I deprecated `pcomplete` and
`eshell-pcomplete` because AFAIK they didn't have noticeably fewer bugs
than `completion-at-point` any more and neither did they offer any
feature available from `completion-at-point`.

W.r.t simple mechanism for simple cases, I'm not sure what that would
concretely look like in this particular case.

Some motivations for `pcomplete-completions-at-point`:
- make it possible to remove duplicate code that deals with the UI
  aspect of completion (i.e. the `pcomplete` command) rather than the
  core purpose of `pcomplete.el` which is to provide a way to specify
  which completion table applies where on a command line.
- let the `pcomplete` machinery work with the standard UI, which means
  it can also (mostly) obey `completion-styles`.
- let the `pcomplete` machinery work with other UIs such as
  `company-mode`.  I believe this last point is more important now.

> Indeed this patch works.

Thanks.

> But now my question is: whould it not be possible to do this (namely,
> returning a lazy completion table) in one of the pcomplete-* functions (in
> `pcomplete-completions-at-point' itself, or in `pcomplete-completions',
> or...), instead of doing this in the individual functions ultimately called
> by `pcomplete-completions-at-point'?

IIRC it wasn't really easy/possible, no.  E.g. in the patch I sent
I think there's a bug in that a leading * should change the START..END
returned by `pcomplete-completion-at-point-function` so the `glob-name`
computation should be done outside of the `completion-table-dynamic`.

It could have been made a bit simpler and cleaner I guess if I had
decided to "start over from scratch" and force a rewrite of every
pcomplete/<foo> function (basically every such function would have to
return the same kind of info as returned by
`completion-at-point-functions`).

It would not have had any code in common with `pcomplete` any more,
other than the underlying design (which I find very clever&elegant).


        Stefan

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Stefan,

Thank you very much for your detailed answer!

>
>>> This design relies on the fact that completion tables can be lazy, so 
>>> it should always be possible to make the completion-at-point-function 
>>> very cheap and harmless, so it's OK to call it repeatedly (or even 
>>> needlessly).
>>
>> This is not at all documented AFAICS.  Given that it's a crucial aspect 
>> for your design to work, it should be.
>
> The lispref says the following:
>
>    The functions on this hook should generally return quickly, since
>    they may be called very often (e.g., from @code{post-command-hook}).
>    Supplying a function for @var{collection} is strongly recommended if
>    generating the list of completions is an expensive operation.  Emacs
>    may internally call functions in @code{completion-at-point-functions}
>    many times, but care about the value of @var{collection} for only
>    some of these calls.  By supplying a function for @var{collection},
>    Emacs can defer generating completions until necessary.  You can use
>    @code{completion-table-dynamic} to create a wrapper function:
>

I see, thanks for the pointer.  I did not find this because I was 
searching for `pcomplete-completions-at-point' and 
`pcomplete-completions'.  It would make sense to put this pointer in 
pcomplete.el.

>
> W.r.t simple mechanism for simple cases, I'm not sure what that would 
> concretely look like in this particular case.
>

In this particular case, in Emacs 24-26, `eshell-pcomplete' called 
`pcomplete', which did the completion without triggering the machinery of 
`pcomplete-completion-at-point', that is, without entering a transient 
`completion-in-region-mode', without modifying `post-command-hook', and so 
forth.  In particular, `pcomplete/cd' was called a single time.  It seems 
to me (even now that I understand the design of 
`pcomplete-completion-at-point', and that I understand how the more 
complex mechanism can be made as efficient as the simple one) that this 
simple mechanism is often sufficient, that it is easier to understand, and 
that it should remain available.  OTOH I also understand the point that 
you want to avoid duplicating code.

>
> Some motivations for `pcomplete-completions-at-point`:
> - make it possible to remove duplicate code that deals with the UI aspect of completion (i.e. the `pcomplete` command) rather than the core purpose of `pcomplete.el` which is to provide a way to specify which completion table applies where on a command line.
> - let the `pcomplete` machinery work with the standard UI, which means it can also (mostly) obey `completion-styles`.
> - let the `pcomplete` machinery work with other UIs such as `company-mode`.  I believe this last point is more important now.
>

I see.  This makes perfect sense, thanks for the clarification.

What is still missing IMO is a general description/documentation of the 
various parts of the completion mechanisms (completion-at-point, 
completion-in-region, pcomplete, pcomplete-completion-at-point, 
comint-completion-at-point, icomplete, ...) in Emacs behave and interact. 
I was completely lost when I started working on this bug, things are a bit 
clearer now, but still not very clear.

>
>> Indeed this patch works.
>
> Thanks.
>
[...]
>
> In the patch I sent I think there's a bug in that a leading * should 
> change the START..END returned by 
> `pcomplete-completion-at-point-function` so the `glob-name` computation 
> should be done outside of the `completion-table-dynamic`.
>

*sigh* So this bug can still not be considered fixed?

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Michael Albinus]

Gregory Heytings <ghe@sdf.org> writes:

> Hi Stefan,

Hi,

> What is still missing IMO is a general description/documentation of
> the various parts of the completion mechanisms (completion-at-point,
> completion-in-region, pcomplete, pcomplete-completion-at-point,
> comint-completion-at-point, icomplete, ...) in Emacs behave and
> interact. I was completely lost when I started working on this bug,
> things are a bit clearer now, but still not very clear.

+1

Exactly this is the reason I've tried and failed to work on completion
problems. Several times.

Best regards, Michael.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Drew Adams]

>> What is still missing IMO is a general description/documentation of
>> the various parts of the completion mechanisms (completion-at-point,
>> completion-in-region, pcomplete, pcomplete-completion-at-point,
>> comint-completion-at-point, icomplete, ...) in Emacs behave and
>> interact. I was completely lost when I started working on this bug,
>> things are a bit clearer now, but still not very clear.
> 
> +1
> 
> Exactly this is the reason I've tried and failed to work on completion
> problems. Several times.

+1

And it's not just a lack of doc, I think.
The completion mechanism (e.g. minibuffer.el) is a labyrinth.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Stefan Monnier]

>> The lispref says the following:
>>
>>    The functions on this hook should generally return quickly, since
>>    they may be called very often (e.g., from @code{post-command-hook}).
>>    Supplying a function for @var{collection} is strongly recommended if
>>    generating the list of completions is an expensive operation.  Emacs
>>    may internally call functions in @code{completion-at-point-functions}
>>    many times, but care about the value of @var{collection} for only
>>    some of these calls.  By supplying a function for @var{collection},
>>    Emacs can defer generating completions until necessary.  You can use
>>    @code{completion-table-dynamic} to create a wrapper function:
>>
>
> I see, thanks for the pointer.  I did not find this because I was searching
> for `pcomplete-completions-at-point' and `pcomplete-completions'.  It would
> make sense to put this pointer in pcomplete.el.

Indeed, it might make sense to add a reminder that `pcomplete-here` is
the function where we "choose which completion table to use" and that
this choice should always be quick (so if the set of candidates can take
a while to compute, make sure the completion table computes it lazily).

> It seems to me (even now that I understand the design of
> `pcomplete-completion-at-point', and that I understand how the more
> complex mechanism can be made as efficient as the simple one) that
> this simple mechanism is often sufficient, that it is easier to
> understand, and that it should remain available.

You can definitely write such a "simple completion UI" on top of
`completion-at-point-functions`, and make sure it calls things like
`pcomplete-completion-at-point` only once.

Basically, take `completion-at-point`, throw out the "minor mode"
part and you're done.

> What is still missing IMO is a general description/documentation of the
> various parts of the completion mechanisms (completion-at-point,
> completion-in-region, pcomplete, pcomplete-completion-at-point,
> comint-completion-at-point, icomplete, ...) in Emacs behave and
> interact. I was completely lost when I started working on this bug, things
> are a bit clearer now, but still not very clear.

I find it hard to write such things because I'm too familiar with it to
know what's non-obvious.  Maybe you could try writing something that
you'd have found useful, and then we can collaboratively improve it?

>> In the patch I sent I think there's a bug in that a leading * should
>> change the START..END returned by `pcomplete-completion-at-point-function`
>> so the `glob-name` computation should be done outside of the
>> `completion-table-dynamic`.
> *sigh* So this bug can still not be considered fixed?

Yup :-(
I'll try to come up with something better if noone else beats me to it.


        Stefan

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Gregory Heytings via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>
>> What is still missing IMO is a general description/documentation of the 
>> various parts of the completion mechanisms (completion-at-point, 
>> completion-in-region, pcomplete, pcomplete-completion-at-point, 
>> comint-completion-at-point, icomplete, ...) in Emacs behave and 
>> interact. I was completely lost when I started working on this bug, 
>> things are a bit clearer now, but still not very clear.
>
> I find it hard to write such things because I'm too familiar with it to 
> know what's non-obvious.  Maybe you could try writing something that 
> you'd have found useful, and then we can collaboratively improve it?
>

Based on the time I already spent on this bug, I guess writing something 
like this will require a substantial amount of work, but if you are 
willing to help me, I'm willing to do that.

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Stefan Monnier]

>>> What is still missing IMO is a general description/documentation of the
>>> various parts of the completion mechanisms (completion-at-point,
>>> completion-in-region, pcomplete, pcomplete-completion-at-point,
>>> comint-completion-at-point, icomplete, ...) in Emacs behave and
>>> interact. I was completely lost when I started working on this bug,
>>> things are a bit clearer now, but still not very clear.
>> I find it hard to write such things because I'm too familiar with it to
>> know what's non-obvious.  Maybe you could try writing something that you'd
>> have found useful, and then we can collaboratively improve it?
> Based on the time I already spent on this bug, I guess writing something
> like this will require a substantial amount of work, but if you are willing
> to help me, I'm willing to do that.

That'd be great.  There's a chance others will chime in as well.


        Stefan

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[xristos--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On Wed, 2 Sep 2020 09:00:14 -0700 (PDT), 
Drew Adams <drew.adams@oracle.com> wrote:
> +1
>
> And it's not just a lack of doc, I think.
> The completion mechanism (e.g. minibuffer.el) is a labyrinth.

+1

I was bitten by this bug today (which is still unfixed afaict),
and since TRAMP/eshell is an important part of my daily routine I
went back to using pcomplete.

Having ran into similar completion heisenbugs in the past, I have
to say that I find the mechanism to be severely under-documented and
the code very hard to make sense of in reasonable time. A design document
that lays out the architecture and component interactions is sorely
needed. 

bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

[Stefan Monnier]

FWIW, I installed a variant of my earlier patch into `master`.
I suspect it has a few rough edges, so watch out for regressions, please
and complain to me (with as precise a recipe as you can) when you bump
into them,


        Stefan

bug#41423: Installing the fix for bug#41423 on emacs-27 (was: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest])

[Stefan Monnier]

Stefan Monnier [2021-01-31 21:45:39] wrote:
> FWIW, I installed a variant of my earlier patch into `master`.
> I suspect it has a few rough edges, so watch out for regressions, please
> and complain to me (with as precise a recipe as you can) when you bump
> into them,

I installed a further change which should fix the main issue I could
foresee.  I'm now reasonably happy with the patch (I think it's "right"
rather than a quick ad-hoc fix.  It deserves further improvements to add
some form of caching, but that's largely orthogonal to this bug report).

IIUC Michael had marked this bug as "blocking" for Emacs-27, so this
argues for installing it on the `emacs-27` branch.

The patch is not without risks seeing how it changes a function from
returning a list to returning a function and how it fundamentally
changes *when* the code is executed.  So I'm not sure whether it should
go on the `emacs-27` branch:

A- For non-Eshell users it is "obviously safe".
B- For Eshell-over-Tramp users it should be a clear improvement even if
   it turns out to introduce some unforeseen regressions in some cases.
C- Finally, Eshell-not-over-Tramp users should hopefully see no
   difference at all, which mostly means no improvement to make up for
   any risk of regression.

So, is the improvement in B worth the risk in C?


        Stefan

bug#41423: Installing the fix for bug#41423 on emacs-27

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

Hi,

> IIUC Michael had marked this bug as "blocking" for Emacs-27, so this
> argues for installing it on the `emacs-27` branch.
>
> The patch is not without risks seeing how it changes a function from
> returning a list to returning a function and how it fundamentally
> changes *when* the code is executed.  So I'm not sure whether it should
> go on the `emacs-27` branch:
>
> A- For non-Eshell users it is "obviously safe".
> B- For Eshell-over-Tramp users it should be a clear improvement even if
>    it turns out to introduce some unforeseen regressions in some cases.
> C- Finally, Eshell-not-over-Tramp users should hopefully see no
>    difference at all, which mostly means no improvement to make up for
>    any risk of regression.
>
> So, is the improvement in B worth the risk in C?

For me, the patch is not trivial enough that I can read it fluently. So
I propose to keep the bug open as blocking for Emacs 27, and backport
the patch to the emacs-27 branch only after the 27.2 release. Whether
there will be an Emacs 27.3 doesn't matter then.

>         Stefan

Best regards, Michael.

bug#41423: Installing the fix for bug#41423 on emacs-27 (was: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest])

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: andrés ramírez <rrandresf@gmail.com>,
>   41423@debbugs.gnu.org
> Date: Sun, 31 Jan 2021 23:36:09 -0500
> 
> IIUC Michael had marked this bug as "blocking" for Emacs-27, so this
> argues for installing it on the `emacs-27` branch.
> 
> The patch is not without risks seeing how it changes a function from
> returning a list to returning a function and how it fundamentally
> changes *when* the code is executed.  So I'm not sure whether it should
> go on the `emacs-27` branch:
> 
> A- For non-Eshell users it is "obviously safe".
> B- For Eshell-over-Tramp users it should be a clear improvement even if
>    it turns out to introduce some unforeseen regressions in some cases.
> C- Finally, Eshell-not-over-Tramp users should hopefully see no
>    difference at all, which mostly means no improvement to make up for
>    any risk of regression.
> 
> So, is the improvement in B worth the risk in C?

No, I don't think so.

bug#41423: Installing the fix for bug#41423 on emacs-27

[Stefan Monnier]

>> So, is the improvement in B worth the risk in C?
> No, I don't think so.

OK, thanks.  That saves me the trouble of trying to write a minimalist
version of the patch ;-)


        Stefan

bug#41423: Installing the fix for bug#41423 on emacs-27

[andrés ramírez]

Hi. Stefan, Lars, Eli, Michael, Guys.

>>>>> "Stefan" == Stefan Monnier <monnier@iro.umontreal.ca> writes:

    Stefan> OK, thanks.  That saves me the trouble of trying to write a minimalist version of the
    Stefan> patch ;-)

I am testing now:
,---- [  ]
| GNU Emacs 27.1.91 (build 1, armv7l-unknown-linux-gnueabihf, X toolkit, cairo version 1.17.4, Xaw3d scroll bars)
|  of 2021-01-31
`----

If You code the 'minimalist version of the patch'. I could try it on this
version. compiling master takes 1h on my lower end machine. But the
releases like:
--8<---------------cut here---------------start------------->8---
  https://alpha.gnu.org/gnu/emacs/pretest/emacs-27.1.91.tar.xz
--8<---------------cut here---------------end--------------->8---
have been prepared with:
--8<---------------cut here---------------start------------->8---
                      autoreconf -i -I m4 --force
                      make bootstrap
                     ./make-dist --snapshot --no-compress --no-changelog
--8<---------------cut here---------------end--------------->8---
After all this preparation compiling emacs just take me 20 minutes.

I think Lars was talking about having "Multi-OS Emacs buildbot". If that
happens. It would be excellent  having daily snapshot pre-packaged (like
the one in pretest You know with make bootstrap and the other two cli
commands ). That would simplify compiling emacs master on lower end
machines like my SBC {Single Board Computer}.

Best Regards

bug#41423: Installing the fix for bug#41423 on emacs-27

[Stefan Monnier]

> If You code the 'minimalist version of the patch'. I could try it on this
> version.

See patch below.  You don't need to rebuild Emacs either: just recompile
that one file and you're done.

> commands ).  That would simplify compiling emacs master on lower end
> machines like my SBC {Single Board Computer}.

I know what you mean (recompiling Emacs on my takes quite a while on
some of my machines (BananaPi, Mele-A1000, and Thinkpad X30), especially
because my build scripts enable all debug options, which makes it even
slower).


        Stefan


diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
index faf749d836..744be525ec 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -399,16 +399,21 @@
 
 (defun eshell-complete-commands-list ()
   "Generate list of applicable, visible commands."
-  (let ((filename (pcomplete-arg)) glob-name)
+  ;; Building the commands list can take quite a while, especially over Tramp
+  ;; (bug#41423), so do it lazily.
+  (let ((glob-name
+	 ;; When a command is specified using `eshell-explicit-command-char',
+         ;; that char is not part of the command and hence not part of what
+         ;; we complete.  Adjust `pcomplete-stub' accordingly!
+	 (if (and (> (length pcomplete-stub) 0)
+	          (eq (aref pcomplete-stub 0) eshell-explicit-command-char))
+	     (setq pcomplete-stub (substring pcomplete-stub 1)))))
+    (completion-table-dynamic
+     (lambda (filename)
     (if (file-name-directory filename)
         (if eshell-force-execution
             (pcomplete-dirs-or-entries nil #'file-readable-p)
           (pcomplete-executables))
-      (if (and (> (length filename) 0)
-	       (eq (aref filename 0) eshell-explicit-command-char))
-	  (setq filename (substring filename 1)
-		pcomplete-stub filename
-		glob-name t))
       (let* ((paths (eshell-get-path))
 	     (cwd (file-name-as-directory
 		   (expand-file-name default-directory)))
@@ -455,7 +460,7 @@
 			    (and eshell-show-lisp-alternatives
 				 (null completions)))
 			(all-completions filename obarray #'functionp))
-		   completions)))))))
+		      completions)))))))))
 
 (define-obsolete-function-alias 'eshell-pcomplete #'completion-at-point "27.1")
 

bug#41423: Installing the fix for bug#41423 on emacs-27

[andrés ramírez]

Hi. Stefan.

>>>>> "Stefan" == Stefan Monnier <monnier@iro.umontreal.ca> writes:

    >> If You code the 'minimalist version of the patch'. I could try it on this version.

    Stefan> See patch below.  You don't need to rebuild Emacs either: just recompile that one file
    Stefan> and you're done.

I have re-tested the original conditions of this bug report.
Without the 'minimalist version of the patch'. And the problem does NOT
happen anymore.

Perhaps Xristos could provide his test conditions and also try the
'minimalist version of the patch'.


[...]



Best Regards

bug#41423: Installing the fix for bug#41423 on emacs-27

[Andrés Ramírez]

Hi. Stefan.

I have been testing the 'minimalist version of the patch'.

And I have found this issue:
--8<---------------cut here---------------start------------->8---
http://debbugs.gnu.org/47389
--8<---------------cut here---------------end--------------->8---

For fixing that issue. I have added a tiny modification. to the
'minimalist version of the patch'
which is:
--8<---------------cut here---------------start------------->8---
-	 (if glob-name
+	 (if (and (boundp 'glob-name) glob-name)
--8<---------------cut here---------------end--------------->8---

That change let me workaround bug#47389.

The change is OK?

Best Regards

bug#41423: Installing the fix for bug#41423 on emacs-27

[Stefan Monnier]

> I have been testing the 'minimalist version of the patch'.
>
> And I have found this issue:
> --8<---------------cut here---------------start------------->8---
> http://debbugs.gnu.org/47389
> --8<---------------cut here---------------end--------------->8---
>
> For fixing that issue. I have added a tiny modification. to the
> 'minimalist version of the patch'
> which is:
> --8<---------------cut here---------------start------------->8---
> -	 (if glob-name
> +	 (if (and (boundp 'glob-name) glob-name)
> --8<---------------cut here---------------end--------------->8---
>
> That change let me workaround bug#47389.
> The change is OK?

No, the missing part is the `-*- lexical-binding:t -*-` on the first
line of the file in which you place that code.

The above workaround you found might mostly work OK, tho; I suspect it
will just mis-complete when you use a command name like `*foo`.


        Stefan

bug#41423: Installing the fix for bug#41423 on emacs-27

[andrés ramírez]

Hi Stefan.

>>>>> "Stefan" == Stefan Monnier <monnier@iro.umontreal.ca> writes:

    Stefan> No, the missing part is the `-*- lexical-binding:t -*-` on the first line of the file in
    Stefan> which you place that code.

Then I am going to do this change. And keep testing the 'minimalist version of the patch'.

    Stefan> The above workaround you found might mostly work OK, tho; I suspect it will just
    Stefan> mis-complete when you use a command name like `*foo`.

Thanks for the information.

Best Regards

bug#41423: bug#47389: 27.1.91; completion issue on eshell

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> If You code the 'minimalist version of the patch'. I could try it on this
>> version.
>
> See patch below.  You don't need to rebuild Emacs either: just recompile
> that one file and you're done.

[...]

> diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
> index faf749d836..744be525ec 100644
> --- a/lisp/eshell/em-cmpl.el
> +++ b/lisp/eshell/em-cmpl.el
> @@ -399,16 +399,21 @@
>
>  (defun eshell-complete-commands-list ()
>    "Generate list of applicable, visible commands."
> -  (let ((filename (pcomplete-arg)) glob-name)
> +  ;; Building the commands list can take quite a while, especially over Tramp
> +  ;; (bug#41423), so do it lazily.

Re-skimming this bug report quickly, it seems like this patch was
applied at the time, but Andrés had some additional issues, and a
proposed addition patch?

Andrés, did this patch fix the issue, and if not, do you have an
additional patch here?

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

bug#41423: bug#47389: 27.1.91; completion issue on eshell

[andrés ramírez]

Hi. Lars.

>>>>> "Lars" == Lars Ingebrigtsen <larsi@gnus.org> writes:

    Lars> Re-skimming this bug report quickly, it seems like this patch was applied at the time, but
    Lars> Andrés had some additional issues, and a proposed addition patch?

I have removed the patch from my dot emacs today. And everything seems
to be working fine. So no additional patch is needed.

    Lars> Andrés, did this patch fix the issue, and if not, do you have an additional patch here?

Yes. It did.
Thanks Guys
Best Regards

bug#41423: bug#47389: 27.1.91; completion issue on eshell

[Lars Ingebrigtsen]

andrés ramírez <rrandresf@hotmail.com> writes:

> I have removed the patch from my dot emacs today. And everything seems
> to be working fine. So no additional patch is needed.

Thanks; I'm closing this bug report, then.

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