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

Use parallel for nested loops in VirtualMachineImageService#list_all method #363

Merged
merged 1 commit into from
Mar 13, 2018

Conversation

djberg96
Copy link
Collaborator

This updates the VirtualMachineImageService#list_all method to use multiple loops via the Parallel library in order to significantly speed up the results. Currently this method takes over 20 minutes without threading. With threading the time drops down to about 1 minute to collect all results.

Addresses #306

@djberg96 djberg96 requested a review from bzwei March 12, 2018 22:02
@miq-bot
Copy link
Member

miq-bot commented Mar 12, 2018

Checked commit djberg96@f74cefb with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

Parallel.each(skus(offer.name, location, publisher.name), :in_threads => configuration.max_threads) do |sku|
Parallel.each(versions(sku.name, offer.name, location, publisher.name), :in_threads => configuration.max_threads) do |version|
mutex.synchronize do
images << Azure::Armrest::VirtualMachineImage.new(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the place that the code needs to make API calls to the provider? Is it in the constructor? Maybe we can use multiple loops to collect the hashes, and finally use one Parallel.each to construct images.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bzwei There's more than one way to skin a cat, sure, but I don't think it's any cleaner or easier to read. I did something like that when I was fiddling with Excon:

https://gist.github.com/djberg96/f44d2cb7b930c2898ce7a8466bd54956

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The versions, skus and offers and publishers methods are where the actual API calls happen.

@bzwei bzwei merged commit 7f719fe into ManageIQ:master Mar 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants