-
Notifications
You must be signed in to change notification settings - Fork 340
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
Update vmware_vm_info.py #2194
Update vmware_vm_info.py #2194
Conversation
unrolling the return statement for "def get_vm_attributes(self, vm)" results in a ~25 fold speed increase processing a 2,200 VMs inventory. From 2.7 seconds average per VM getting attributes for original code to 0.11 seconds for the new unrolled code example using original and new code: 2024-10-02 16:41:31,188 - DEBUG - {'BusinessArea': 'b1', 'Project': '199400081', 'CriticalityLevel': '1', 'DataSensitivity': 'L1', 'LicensedProduct': 'N/A', 'OS': 'Windows2019', 'SystemEnvironment': 'prd'} 2024-10-02 16:41:31,189 - DEBUG - Time to get VM attributes1: 2.718272 seconds 2024-10-02 16:41:31,303 - DEBUG - {'BusinessArea': 'b1', 'Project': '199400081', 'CriticalityLevel': '1', 'DataSensitivity': 'L1', 'LicensedProduct': 'N/A', 'OS': 'Windows2019', 'SystemEnvironment': 'prd'} 2024-10-02 16:41:31,304 - DEBUG - Time to get VM attributes2: 0.114557 seconds
Build succeeded. ✔️ ansible-tox-linters SUCCESS in 7m 54s |
Thanks @eddiehavila! I wanted to have a closer look at your PR, but ran into some issues with my test environment that I have to fix first. I'll try to have a closer look at you PR next week! BTW could you please add a changelog fragment? Maybe something like
or something similar. |
added changelog fragment
added the changelog fragment. thanks @mariolenz for your guidance |
Build failed. ✔️ ansible-tox-linters SUCCESS in 8m 03s |
recheck |
Build succeeded. ✔️ ansible-tox-linters SUCCESS in 7m 52s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't reproduce your ~25 fold speed increase, and I wonder why. I've tested with an environment that's not quite as large as yours (you're talking about 2,200 VMs) but coming close. Maybe you just use attributes more heavily than we do?
Still, I've seen an improvement for my very simple test case from a little bit more than 7 to somewhat less than 5 minutes.
Thanks @eddiehavila!
Build succeeded (gate pipeline). ✔️ ansible-tox-linters SUCCESS in 7m 45s |
ea1db49
into
ansible-collections:main
SUMMARY Improve the performance of retrieving custom attributes. ISSUE TYPE Feature Pull Request COMPONENT NAME vmware_vm_info ADDITIONAL INFORMATION Backport of #2194 cc @eddiehavila
SUMMARY
unrolling the return statement for "def get_vm_attributes(self, vm)" results in a ~25 fold speed increase processing a 2,200 VMs inventory. From 2.7 seconds average per VM getting attributes for original code to 0.11 seconds for the new unrolled code
ISSUE TYPE
COMPONENT NAME
vmware_vm_info.py
ADDITIONAL INFORMATION
Comparison using original and new code