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.

Sign up for GitHub

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

Jump to bottom

Improve overall execution time when performing the choco upgrade all command #3231

Closed
2 tasks done
TheCakeIsNaOH opened this issue Jun 26, 2023 · 3 comments · Fixed by #3248
Closed
2 tasks done

Improve overall execution time when performing the choco upgrade all command #3231

TheCakeIsNaOH opened this issue Jun 26, 2023 · 3 comments · Fixed by #3248
Assignees
@TheCakeIsNaOH
Labels
5 - Released Improvement
Milestone
2.2.0

Comments

@TheCakeIsNaOH
Copy link
Member

Checklist

  • I have verified this is the correct repository for opening this issue.
  • I have verified no other issues exist related to my request.

Is Your Feature Request Related To A Problem? Please describe.

When a large number of packages (e.g. 300+) are installed on Chocolatey CLI v2.x, the choco upgrade all command takes quite a bit longer to run than it did on v1.4.0

One large contributor to this is that Chocolatey now gets a list of all installed packages before every upgrade check, instead of only refreshing the list when a package upgrade happens. This check can take a while when a large number of packages are installed.

Describe The Solution. Why is it needed?

Chocolatey should only get the list of all installed packages when required, instead of before every package outdated check.

Additional Context

No response

Related Issues
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Jun 27, 2023
@TheCakeIsNaOH
(chocolatey#3231) Cache local package info during upgrade no-ops
6ef9811
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Jul 1, 2023
@TheCakeIsNaOH
(chocolatey#3231) Cache local package info during upgrade no-ops
8507e6b
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Jul 2, 2023
@TheCakeIsNaOH
(chocolatey#3231) Don't refresh local package info during upgrade no-ops
0ee9377 
This optimizes the flow of the upgrade command. First, it uses the
list of locally installed package from the if all is specified check,
instead of throwing that away. Then, it will re-use the list for each
package upgrade attempt, only refreshing the list when a package
upgrade/install is attempted. This speeds up upgrade all runs,
especially when a large (200+) number of packages are installed.
@TheCakeIsNaOH TheCakeIsNaOH mentioned this issue Jul 2, 2023
Merged
10 tasks
@gep13 gep13 added this to the 2.2.0 milestone Jul 7, 2023
@crkinard
Copy link

But the issue was 'resolved' in v2.1.0. 🙄

@gep13
Copy link
Member

gep13 commented Jul 10, 2023

@crkinard said...
But the issue was 'resolved' in v2.1.0. 🙄

I think you might be referring to this issue:

#3183

Which was indeed fixed in v2.1.0. That issue was the result of a imposed 1 second delay on each HTTP request that was made by Chocolatey CLI, which had an impact on pretty much all the commands that Chocolatey CLi has.

This issue that @TheCakeIsNaOH has raised is specifically for the choco upgrade all command. where multiple listing of packages is done during the execution of the command, which results in the choco upgrade all command taking a noticeably longer time to complete compared to v1.4.0.

corbob pushed a commit to TheCakeIsNaOH/choco that referenced this issue Jul 11, 2023
@TheCakeIsNaOH @corbob
(chocolatey#3231) Don't refresh local package info during upgrade no-ops
7795617 
This optimizes the flow of the upgrade command. First, it uses the
list of locally installed package from the if all is specified check,
instead of throwing that away. Then, it will re-use the list for each
package upgrade attempt, only refreshing the list when a package
upgrade/install is attempted. This speeds up upgrade all runs,
especially when a large (200+) number of packages are installed.
corbob added a commit to TheCakeIsNaOH/choco that referenced this issue Jul 11, 2023
@corbob
(chocolatey#3231) Add tests to ensure package listing
853e26a 
Add integration tests that ensure we don't run the local package listing
more than once per upgraded package.
corbob pushed a commit to TheCakeIsNaOH/choco that referenced this issue Jul 11, 2023
@TheCakeIsNaOH @corbob
(chocolatey#3231) Don't refresh local package info during upgrade no-ops
f01aa8d 
This optimizes the flow of the upgrade command. First, it uses the
list of locally installed package from the if all is specified check,
instead of throwing that away. Then, it will re-use the list for each
package upgrade attempt, only refreshing the list when a package
upgrade/install is attempted. This speeds up upgrade all runs,
especially when a large (200+) number of packages are installed.
corbob added a commit to TheCakeIsNaOH/choco that referenced this issue Jul 11, 2023
@corbob
(chocolatey#3231) Add tests to ensure package listing
cd92db1 
Add integration tests that ensure we don't run the local package listing
more than once per upgraded package.
corbob added a commit that referenced this issue Jul 11, 2023
@corbob
Merge pull request #3248 from TheCakeIsNaOH/upgrade-list-cache
03c5aaa 
(#3231) Don't refresh local package info during upgrade no-ops
@gep13 gep13 added 4 - Done and removed 3 - Review labels Jul 11, 2023
@gep13 gep13 changed the title Improve performance when upgrading all packages Improve overall execution time when performing the choco upgrade all command Jul 26, 2023
gep13 added a commit that referenced this issue Jul 26, 2023
@gep13
Merge branch 'release/2.2.0'
76b9ca9 
* release/2.2.0: (21 commits)
  (doc) Update to indicate new package version used
  (maint) Add helper to split on max line lengths
  (#3281) Add validation for cache folder permissions
  (#3264) Update to latest Chocolatey.NuGet.Client
  (#3264) Ignore lock folders in cache directories
  (#3186) Remove easter egg
  (doc) Improve error message for defaultPushSource
  (tests) Clear HTTP Cache before getting packages
  (#3258) Expand logging for nuget resources errors
  (maint) Set file encoding to include BOM
  (#3237) Reduce number of queries for dependencies
  (#3231) Add tests to ensure package listing
  (maint) Remove unnecessary using statements
  (#3231) Don't refresh local package info during upgrade no-ops
  (build) Update to latest recipe package
  (doc) Minor corrections to wording
  (#3242) Add a script to run Authenticated tests
  (#3242) Attempt default credentials for sources
  (maint) Fix incorrect naming style uses
  (doc) Apply scripting best practices to output
  ...
@choco-bot
Copy link

🎉 This issue has been resolved in version 2.2.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

@pauby pauby mentioned this issue Jul 28, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@TheCakeIsNaOH TheCakeIsNaOH

Labels
5 - Released Improvement
Projects
None yet
Milestone
2.2.0
Development

Successfully merging a pull request may close this issue.

(#3231) Don't refresh local package info during upgrade no-ops TheCakeIsNaOH/choco
4 participants
@gep13 @crkinard @choco-bot @TheCakeIsNaOH