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

Replace function calls with properties where possible for SCVMM refresh. #6902

Merged
merged 1 commit into from Feb 23, 2016
Merged

Replace function calls with properties where possible for SCVMM refresh. #6902

merged 1 commit into from Feb 23, 2016

Conversation

djberg96
Copy link
Contributor

This is an interim improvement for the SCVMM inventory collection script. It turns out that some of the information we were collecting for VM's and images using powershell functions in a loop were already available as properties. This means a 3x reduction in the number of calls per VM and a 2x reduction in the number of calls per image.

While not as fast as the upcoming JSON implementation, I saw significant improvements. As a standalone WinRM script, the older implementation typically took 120-150 seconds. This implementation typically came in around 80 seconds.

@djberg96
Copy link
Contributor Author

@bronaghs Everything looked good in the UI, but please review and make sure I didn't miss or break anything.

@chessbyte
Copy link
Member

Nice find!
/cc @blomquisg

@Fryguy
Copy link
Member

Fryguy commented Feb 23, 2016

@djberg96 Part of the reason we had single spaces was because there is a maximum size to the script that could be passed down. Is that still a problem?

@djberg96
Copy link
Contributor Author

@Fryguy True, I added some very minor formatting, but the overall number of bytes has been reduced. I modified the functions to use inline parameters, and shrunk some variable names, in addition to the removal of the function calls.

@Fryguy
Copy link
Member

Fryguy commented Feb 23, 2016

Sounds good to me 👍

@miq-bot
Copy link
Member

miq-bot commented Feb 23, 2016

Checked commit https://github.com/djberg96/manageiq/commit/33923de4540b43139100573fb986b60cc4e99d05 with ruby 2.2.3, rubocop 0.34.2, and haml-lint 0.13.0
0 files checked, 0 offenses detected
Everything looks good. ⭐

Fryguy added a commit that referenced this pull request Feb 23, 2016
Replace function calls with properties where possible for SCVMM refresh.
@Fryguy Fryguy merged commit 46e5ab5 into ManageIQ:master Feb 23, 2016
@Fryguy Fryguy added this to the Sprint 37 Ending Mar 7, 2016 milestone Feb 23, 2016
@bronaghs
Copy link

I didnt have a chance to review this yesterday afternoon before it was merged, this is a really nice improvement. Thanks @djberg96

@djberg96 djberg96 deleted the scvmm_refresh branch March 17, 2017 13:37
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.

None yet

5 participants