chocolatey / cChoco

Community resource to manage Chocolatey
Apache License 2.0
153 stars 99 forks source link

Speed boost for cChocoFeature by reading Chocolatey.config XML #124

Open mrhockeymonkey opened 5 years ago

mrhockeymonkey commented 5 years ago

Internally we have moved away from cChocoFeature and instead use a custom resource. Our custom resource reads from Chocolatey.config xml to determine if any given feature is enabled rather than running powershell choco feature

This gives us a pretty reasonable boost in speed for the routine consistency checks. Would you consider us merging our changes upstream to this repo? I can appreciate the idea of reading the config file might not be desired and possible considered anti-pattern

As an idea of speed increase and thus the reason we went this way:

PS D:\Code> measure-command {([xml](Get-content C:\ProgramData\chocolatey\config\chocolatey.config)).Select
SingleNode("/chocolatey/config/add[@key='cacheLocation']").value}
Days              : 0
Hours             : 0
Minutes           : 0
Seconds           : 0
Milliseconds      : 3
Ticks             : 38312
TotalDays         : 4.43425925925926E-08
TotalHours        : 1.06422222222222E-06
TotalMinutes      : 6.38533333333333E-05
TotalSeconds      : 0.0038312
TotalMilliseconds : 3.8312

PS D:\Code> Measure-Command { choco config get --name "cachelocation"}
Days              : 0
Hours             : 0
Minutes           : 0
Seconds           : 1
Milliseconds      : 528
Ticks             : 15283585
TotalDays         : 1.76893344907407E-05
TotalHours        : 0.000424544027777778
TotalMinutes      : 0.0254726416666667
TotalSeconds      : 1.5283585
TotalMilliseconds : 1528.3585

worth noting also 1 seconds isn't exactly bad but we have massively varying number on this which was the other reason we made a custom resource. For instance on my workstation the latter command takes 19 seconds. Something else entirely to look into but nonetheless the xml method is blindingly quick.

Thoughts and feedback welcome

mrhockeymonkey commented 5 years ago

Worth noting also we only use this method for Test-TargetResource, Never for Set-TargetResource.

pauby commented 5 years ago

@mrhockeymonkey Thanks for raising this issue.

I'd prefer to stay away from reading Chocolatey config files directly for the reason you mentioned. Having said that Puppet has chocolateyfeature and other configuration managers similar 'resources' so it makes sense to follow what they are doing. What they are doing I need to look into (but if anybody knows feel free to update here).

mrhockeymonkey commented 5 years ago

Yep seems fair. Well I'll stick with what I have so far at my own peril. If you do find out and/or want to include this as a separate resource or perhaps a switch to control this behavior then I'm happy to make a PR. If not I'm happy for this to be closed as answered.

mrhockeymonkey commented 5 years ago

I seem to have accidentally closed this...

I had a look at puppet's 'chocolateyfeature' that you mentioned. It looks to me like it does interact with the config file directly rather than through the CLI. See here which points to here

It looks to only read features this way and uses the LCI to make changes (as we have done also). See here

pauby commented 5 years ago

If it's being done that way on other config managers then I'd say it's fine to do it here also.