Skip to content

Conversation

@crazy-max
Copy link
Member

follow-up #4147 (comment)

- What I did

We can slightly improve plugins listing by spawning a goroutine for each iteration.

- How I did it

- How to verify it

$ hyperfine --warmup 2 --runs 5 '/tmp/docker-23.0.2 __complete ""'
Benchmark 1: /tmp/docker-23.0.2 __complete ""
  Time (mean ± σ):      87.7 ms ±   2.4 ms    [User: 66.2 ms, System: 54.2 ms]
  Range (min … max):    83.8 ms …  90.3 ms    5 runs

$ make -f docker.Makefile binary
...

$ hyperfine --warmup 2 --runs 5 './build/docker __complete ""'
Benchmark 1: ./build/docker __complete ""
  Time (mean ± σ):      34.6 ms ±   1.2 ms    [User: 104.4 ms, System: 74.7 ms]
  Range (min … max):    32.8 ms …  36.0 ms    5 runs

Also run this repro.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

We can slightly improve plugins listing by spawning a
goroutine for each iteration.

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max crazy-max force-pushed the improve-plugins-list branch from 13070fa to 89583b9 Compare April 1, 2023 14:02
@codecov-commenter
Copy link

codecov-commenter commented Apr 1, 2023

Codecov Report

Merging #4151 (89583b9) into master (33961a7) will increase coverage by 0.00%.
The diff coverage is 71.05%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4151   +/-   ##
=======================================
  Coverage   59.03%   59.03%           
=======================================
  Files         287      287           
  Lines       24767    24778   +11     
=======================================
+ Hits        14620    14627    +7     
- Misses       9264     9267    +3     
- Partials      883      884    +1     

return p, nil
}
if cmd.HasAlias(p.Name) {
p.Err = NewPluginError("plugin %q duplicates an alias of builtin command %q", p.Name, cmd.Name())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR, but it looks like the only reason NewPluginError() is exported, is for it to be used in a test in docker info;

Err: pluginmanager.NewPluginError("something wrong"),

🤔

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@thaJeztah
Copy link
Member

@cpuguy83 @laurazard ptal 🤗

@thaJeztah thaJeztah merged commit 966e191 into docker:master Apr 3, 2023
@crazy-max crazy-max deleted the improve-plugins-list branch April 3, 2023 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants