Get-ChocoInstalledPackage is very slow, causing long Test, Set, and Upgrade times for multiple packages

[jrdnr]

Because cChocoPackageInstallerSet, just provides a foreach overlay for cChocoPackageInstaller it is very slow 1-2 seconds per package due to the time it takes to run choco list -lo

Is there a fundamental reason that cChocoPackageInstaller could not be refactored to allow an array of package names, and then change cChocoPackageInstallerSet to simply be an alias?

This would also address issue #80.

[ferventcoder]

No reason - was just discussing this with a colleague recently.

[ferventcoder]

I think refactoring would be a great idea

[ferventcoder]

As long as it is done in a compatible way that is.

[jrdnr]

Is this something you want to add as a 2.x milestone possibly replacing #80, or is this just a nice idea?

If no one else is going to be working on it, I'd be glad to try to take a first whack, I've never contributed to a project or worked with AppVeyor before so I'll have a little bit of a learning curve to get over to get used to the tools before I can make any real contribution on the project itself.

[ferventcoder]

2.x is current, 3.x is next major.

[ferventcoder]

@jrdnr For contributions, I'd suggest reading Chocolatey's CONTRIBUTING at https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md#contributing-1. It's very comprehensive and helpful for first time folks to regular contributors alike.

[ferventcoder]

A couple of things to keep in mind:

• CLA signature does not apply to cChoco.
• Setting up your environment is different as well

[jrdnr]

Ok I'll review the contribution articles

[jrdnr]

I finally got a chance to sit down and finish getting my head around how everything ties together, and see that a few of my original assumptions were a little off base.

It looks like there are probably a few good reasons (ie exposing a limited set of parameters to cChocoPackageInstallerSet for 1) that there should be both cChocoPackageInstaller vs ..InstallerSet. Also due to the way DSC processes the module the only way to only pull the choco list -lo once per run appears to be to glob all packages into a single instance in the mof file, which does not appear to be an appropriate work around.

The only ways I have been able to come up with to improve the speed of the dsc run would be to directly work on choco list -lo (something that is most likely over my head) or for the dsc resource to cache the output of choco list -lo to a file the first time it runs, and just read it back in from the file when it needs the list.

@ferventcoder would it make sense to update the title on this issue, or close it and open the issue somewhere else?

[ferventcoder]

Update the title.

[jrdnr]

Is this an issue we can address in the DSC resource by caching to disk or something along those lines, or should the fix for this really be made in the core of choco list itself?

[ferventcoder]

@jrdnr choco list -lo is subsecond return for me (150+ packages). But sometimes there are factors on systems that cause it to take up to 2 seconds or more (and not really explainable reasons).

[ferventcoder]

I think we should work on making it better here.

[ferventcoder]

More explanation - Puppet and Chef run this query one time before all packages during a run and cache the results for all. I'm actually surprised (well not really) that Microsoft decided to ignore this pattern.

[jrdnr]

Doing some testing the simplest work around I've found is to add an Export-Clixml to the Get-ChocoInstalledPackage function, then when setting $installedPackages I just check if the cache file exists, and if its less then a minute old I import from the file instead of running the full Get-ChocoInstalledPackage again. On my system this takes 30 reps of setting the $installedPackages variable from 67.44 seconds, down to 2.27 seconds. This is not elegant, and I'm not really sure that storing a list of installed programs in C:\Choco\ is the best way to go, however it appears to do the job.

[ferventcoder]

Storing to a file could have security implications, would you be able to hold a lock on the file or store it somewhere secure during the run?

[jrdnr]

Unfortunately coming back around to the initial problem with DSC being stateless, I don't think file locking is an option.

However because DSC runs as System we can easily write to a secure location. My current version tests for and if needed creates a cChoco directory in $env:ProgramData and then exports the choco list -lo object as Clixml.

In the current implementation I'm using last write time to ensure the cached package list was updated less than 60 seconds ago, if its stale or does not exist I re-run choco list -lo and Export-Clixml again to ensure the package list stays up to date.

From limited testing it does not appear as though DSC runs a Test- after running a Set- and the default LCM config will run another test for 30 min, The only time I see the 60 second limit potentially being a problem would be if someone were to run a Start-DscConfiguration and immediately follow with a Test-DscConfiguration. If necessary this could be remedied by having InstallPackage, UninstallPackage, and Upgrade-Package each test for and remove the cache file or just setting time to stale to a shorter time period.

[jrdnr]

@ferventcoder, sorry I'm new to the open source collaboration (and I know you have a life and a day job). I'm just not sure about next steps.

Should I go ahead and submit a pull request now, or do we continue discussing the proposed solution until any potential issues have been addressed and then I submit the PR?

[ferventcoder]

Go for the PR, we have the basics down and are agreeing on them. Specifics are easily changed.

[jrdnr]

@ferventcoder This issue can be closed now correct?