Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bash completion slows down bash login sessions #3889

Open
2 of 3 tasks
hoxu opened this issue Nov 29, 2022 · 7 comments
Open
2 of 3 tasks

Bash completion slows down bash login sessions #3889

hoxu opened this issue Nov 29, 2022 · 7 comments

Comments

@hoxu
Copy link

hoxu commented Nov 29, 2022

  • I have tried with the latest version of Docker Desktop
  • I have tried disabling enabled experimental features
  • I have uploaded Diagnostics
  • Diagnostics ID:

Expected behavior

bash_completion.d/docker should be fast to execute. It probably should not make any docker calls until actually needed.

Actual behavior

Executing the bash completion script causes a noticeable pause when opening new bash sessions.

The slowest part is a call to docker:

+++ [[ -r /usr/local/etc/bash_completion.d/docker ]]
+++ . /usr/local/etc/bash_completion.d/docker
+++++ shopt -p extglob
++++ __docker_previous_extglob_setting='shopt -s extglob'
++++ shopt -s extglob
+++++ docker info --format '{{range .ClientInfo.Plugins}}{{if eq .Name "compose"}}{{.Path}}{{end}}{{end}}'

This seems to be:

COMPOSE_PLUGIN_PATH=$(docker info --format '{{range .ClientInfo.Plugins}}{{if eq .Name "compose"}}{{.Path}}{{end}}{{end}}')
$ time docker info --format '{{range .ClientInfo.Plugins}}{{if eq .Name "compose"}}{{.Path}}{{end}}{{end}}'
/usr/local/lib/docker/cli-plugins/docker-compose

real    0m2.538s
user    0m0.142s
sys     0m0.085s

Information

  • macOS Version:
  • Intel chip or Apple chip:
  • Docker Desktop Version: 4.14.1 (91661)

Output of /Applications/Docker.app/Contents/MacOS/com.docker.diagnose check

N/A

Steps to reproduce the behavior

Open a new bash login session.

To visually see what is taking up time:

bash -lx
@thaJeztah
Copy link
Member

Thanks for reporting; this looks to be related to changes that were made in the completion script in https://github.com/docker/cli (#3752).

Let me transfer this ticket to the CLI issue tracker (I know some improvements were made after that, but not sure if they can be included in the 20.10 version)

@thaJeztah thaJeztah transferred this issue from docker/for-mac Nov 29, 2022
@thaJeztah
Copy link
Member

I know some improvements were made after that, but not sure if they can be included in the 20.10 version

Did a quick comparison;

# on docker 20.10;
time $(docker info --format '{{range .ClientInfo.Plugins}}{{if eq .Name "compose"}}{{.Path}}{{end}}{{end}}')
real	0m1.034s
user	0m0.130s
sys	0m0.094s

# on docker 23.0.0-dev
time $(docker info --format '{{range .ClientInfo.Plugins}}{{if eq .Name "compose"}}{{.Path}}{{end}}{{end}}')
real	0m0.196s
user	0m0.136s
sys	0m0.048s

@hoxu
Copy link
Author

hoxu commented Dec 2, 2022

That's a big improvement, but 0.20 s is still pretty long. If every completion file took that long time, the result would be unbearable.

Could the call to docker be deferred until the information it prints is actually required in the bash session?

@vegerot
Copy link

vegerot commented May 25, 2023

💀

$ docker --version
Docker version 20.10.21, build baeda1f

$ hyperfine --warmup=1 /usr/local/etc/bash_completion.d/docker
Benchmark 1: /usr/local/etc/bash_completion.d/docker
  Time (mean ± σ):     342.8 ms ±  18.5 ms    [User: 161.4 ms, System: 124.2 ms]
  Range (min … max):   308.6 ms … 371.8 ms    10 runs

@vegerot
Copy link

vegerot commented May 25, 2023

☠️

$ docker --version
Docker version 23.0.5, build bc4487a

📁~/
$ hyperfine --warmup=1 /usr/local/etc/bash_completion.d/docker
Benchmark 1: /usr/local/etc/bash_completion.d/docker
  Time (mean ± σ):     223.4 ms ±  13.0 ms    [User: 247.5 ms, System: 200.1 ms]
  Range (min … max):   206.1 ms … 238.9 ms    13 runs

@guss77
Copy link
Contributor

guss77 commented Aug 21, 2023

The setting of $DOCKER_PLUGINS_PATH during bash completion initialization looks to be a cache for building the plugin commands during docker command completion.

Why is it done during initialization of every single session instead of only as needed?

If docker info --format '{{range .ClientInfo.Plugins}}{{.Path}}:{{end}}' is that fast that it doesn't slow down bash initialization, then it doesn't need to be cached: it can be run as needed when the user calls for docker command completion; OTOH If it is slow, then it definitely cannot be run during bash completion initialization as that unnecessarily slows down every new session.

Regardless, this should be removed.

guss77 added a commit to guss77/cli that referenced this issue Aug 21, 2023
Fixes issue docker#3889 by only loading docker plugins path when needed: if it is fast enough than it shouldn't be a problem to do this on demand; OTOH if it is slow then we shouldn't do this during *every* bash session initialization, regardless if docker completion will be needed or not.
@guss77
Copy link
Contributor

guss77 commented Aug 21, 2023

I offered a PR #4505 to move that plugins path reading to where it should be.

guss77 added a commit to guss77/cli that referenced this issue Aug 21, 2023
Fixes issue docker#3889 by only loading docker plugins path when needed: if it is fast enough than it shouldn't be a problem to do this on demand; OTOH if it is slow then we shouldn't do this during *every* bash session initialization, regardless if docker completion will be needed or not.

Signed-off-by: Oded Arbel <oded@geek.co.il>
guss77 added a commit to guss77/cli that referenced this issue Aug 23, 2023
Fixes issue docker#3889 by only loading docker plugins path when needed: if it is fast enough than it shouldn't be a problem to do this on demand; OTOH if it is slow then we shouldn't do this during *every* bash session initialization, regardless if docker completion will be needed or not.

Signed-off-by: Oded Arbel <oded@geek.co.il>
thaJeztah pushed a commit to thaJeztah/cli that referenced this issue Aug 23, 2023
Fixes issue docker#3889 by only loading docker plugins path when needed: if it is fast enough than it shouldn't be a problem to do this on demand; OTOH if it is slow then we shouldn't do this during *every* bash session initialization, regardless if docker completion will be needed or not.

Signed-off-by: Oded Arbel <oded@geek.co.il>
(cherry picked from commit 1da67be)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah pushed a commit to thaJeztah/cli that referenced this issue Aug 23, 2023
Fixes issue docker#3889 by only loading docker plugins path when needed: if it is fast enough than it shouldn't be a problem to do this on demand; OTOH if it is slow then we shouldn't do this during *every* bash session initialization, regardless if docker completion will be needed or not.

Signed-off-by: Oded Arbel <oded@geek.co.il>
(cherry picked from commit 1da67be)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah pushed a commit to thaJeztah/cli that referenced this issue Aug 23, 2023
Fixes issue docker#3889 by only loading docker plugins path when needed: if it is fast enough than it shouldn't be a problem to do this on demand; OTOH if it is slow then we shouldn't do this during *every* bash session initialization, regardless if docker completion will be needed or not.

Signed-off-by: Oded Arbel <oded@geek.co.il>
(cherry picked from commit 1da67be)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants