Add uniq on datacenters in #host_to_folder

[NickLaMuro]

The more stripped down version of this method, prior to the change, is as follows:

def host_to_folder(src)
  sources = src[:host].nil? ? allowed_hosts_obj : [src[:host]]
  datacenters = sources.collect { |h|
    find_datacenter_for_ci(h)
  end.compact
  datacenters.each_with_object({}) do |dc, folders|
    folders.merge!(get_ems_folders(dc))
  end
end

When setting the datacenters local variable in that method, the find_datacenter_for_ci method basically looks up the datacenter for the matching Host object in the xml tree that has most likely already been generated. In most/all cases, though, their will be more than one host per datacenter, and that wasn't taken into account for this method since the next block blindly takes the datacenters it found previously and merges the folder structure it gets from get_ems_folders for each datacenter.

By adding a .uniq prior to the assignment of datacenters, this saves a significant number of unnecessary CPU cycles and object allocations to fetch the same data and apply it to the folders variable, without changing the end result.

Note: Ideally we would want a better algorithm for finding the datacenters for a collection of hosts, one that allowed for ejecting early from find_datacenter_for_ci when we knew an existing datacenter for a given host had already been found. But the time taken to get the datacenters from the hosts is far less in CPU time than the time taken to call #get_ems_folders per datacenter object and merge the hash with the existing results.

Benchmarks

The benchmark script used here basically runs the following:

ManageIQ::Providers::Vmware::InfraManager::ProvisionWorkflow.new.init_from_dialog

And is targeting a fairly large EMS, with about 600 hosts.

	ms	queries	rows
before	15023	1961	48395
after	6068	1961	48395

Before                                                     After
------                                                     -----

Total allocated: 2069091751 bytes (23226536 objects)   |   Total allocated: 376117422 bytes (4221185 objects)
                                                       |   
allocated objects by gem                               |   allocated objects by gem
-----------------------------------                    |   -----------------------------------
   9665227  pending                                    |      2512007  activerecord-5.0.7
   5561668  manageiq/app  <<<<<<<<<<                   |       418737  ancestry-2.2.2
   3513258  manageiq/lib  <<<<<<<<<<                   |       278793  activemodel-5.0.7
   2512007  activerecord-5.0.7  <<<<<<<<<<             |       207491  manageiq/app
    861270  activesupport-5.0.7  <<<<<<<<<<            |       190717  pending
    418737  ancestry-2.2.2                             |       178419  ruby-2.3.3/lib
    278793  activemodel-5.0.7  <<<<<<<<<<              |       165577  arel-7.1.4
    178419  ruby-2.3.3/lib  <<<<<<<<<<                 |       165168  activesupport-5.0.7
    165577  arel-7.1.4                                 |        52875  manageiq-providers-vmware-e2e2b8029538
     52875  manageiq-providers-vmware-0be2f13a0dc9     |        32696  manageiq/lib
     14424  fast_gettext-1.2.0                         |        14424  fast_gettext-1.2.0
       ...                                             |          ...

Note: The benchmarks for this change do NOT include the changes from other PRs in the links below. Benchmarks of all changes can be found here.

Links

• Partially addresses https://bugzilla.redhat.com/show_bug.cgi?id=1569626
• Full collection of proposed changes: git compare
• Document keeping track of overall improvements for this effort: gist
• Related PRs:
  • Prevents N+1 SQL queries in miq_request_workflow.rb #17354
    • Extracted PRs: Add MiqPreloader.polymorphic_preload_for_child_classes #17457, Use MiqPreloader.polymorphic_preload_for_child_classes in RelationshipMixin#fulltree_arranged #17460, Use MiqPreloader/Relationship improvements in MiqRequestWorkflow #17461
  • Memoize _log.prefix calls #17355
  • Use nested hashes instead of string keys #17357
  • Refactor get_ems_folders to create less strings #17358
  • Use OpenStruct over MiqHashStruct #17359
  • Ancestry Patches #17360
  • Load Lans effeciently in MiqProvisionVirtWorkflow #17381
  • Avoid duplicate host load in allowed_hosts_obj #17402
  • [WIP] Lighter unique Host#domain lookup for in MiqProvisionVirtWorkflow #17403

[miq-bot]

Checked commit NickLaMuro@1bc8c3f 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. ⭐

[syncrou]

@NickLaMuro - LGTM

[NickLaMuro]

@miq-bot add_label fine/yes, gaprindashvili/yes

[NickLaMuro]

@miq-bot add_label performance, provisioning

[simaishi]

Fine backport details:

$ git log -1
commit 347a98c00148a05acfeabf480285d49cddb68f26
Author: Keenan Brock <[email protected]>
Date:   Fri May 18 12:12:14 2018 -0400

    Merge pull request #17422 from NickLaMuro/avoid_duplicate_get_ems_folders_for_datacenters
    
    Add uniq on datacenters in #host_to_folder
    (cherry picked from commit 9b33e0b3919266f70096b3ccacfab0657c1839ab)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1593798

[simaishi]

Gaprindashvili backport details:

$ git log -1
commit 38b974b124cd1bfdb106c5eb88b384a8f66ae21d
Author: Keenan Brock <[email protected]>
Date:   Fri May 18 12:12:14 2018 -0400

    Merge pull request #17422 from NickLaMuro/avoid_duplicate_get_ems_folders_for_datacenters
    
    Add uniq on datacenters in #host_to_folder
    (cherry picked from commit 9b33e0b3919266f70096b3ccacfab0657c1839ab)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1593797