From a4ea2b0398eefd9acd67be7c99e7f5102bbb590d Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Thu, 10 Jan 2019 15:02:20 -0600 Subject: [PATCH] [miq_provision_virt_workflow.rb] Only use hash results This significantly improves the speed of the method by: - Avoiding instantiating intermediate `ActiveRecord` objects that are just garbage collected - Reusing hash results to create the `MiqHashStruct` instantiate Benchmarks ---------- This tests against a AWS EMS with 100k+ templates (public images): **Before** | ms | queries | query (ms) | rows | | ---: | ---: | ---: | ---: | | 14344 | 33 | 1759.6 | 243133 | | 14631 | 33 | 1729.0 | 243133 | | 13405 | 33 | 1752.3 | 243133 | **After** | ms | queries | query (ms) | rows | | ---: | ---: | ---: | ---: | | 4102 | 33 | 1751.3 | 243133 | | 4124 | 33 | 1787.1 | 243133 | | 4133 | 33 | 1746.9 | 243133 | --- app/models/miq_provision_virt_workflow.rb | 56 ++++++++++++++--------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/app/models/miq_provision_virt_workflow.rb b/app/models/miq_provision_virt_workflow.rb index 1e4d1568877..6f520956b20 100644 --- a/app/models/miq_provision_virt_workflow.rb +++ b/app/models/miq_provision_virt_workflow.rb @@ -346,7 +346,8 @@ def allowed_templates(options = {}) # Only select the colums we need vms = vms.select(:id, :name, :guid, :uid_ems, :ems_id, :cloud_tenant_id) - allowed_templates_list = source_vm_rbac_filter(vms, condition, VM_OR_TEMPLATE_EXTRA_COLS).to_a + sql = source_vm_rbac_filter(vms, condition, VM_OR_TEMPLATE_EXTRA_COLS).to_sql + allowed_templates_list = ActiveRecord::Base.connection.select_all(sql).to_a @allowed_templates_filter = filter_id @allowed_templates_tag_filters = @values[:vm_tags] rails_logger('allowed_templates', 1) @@ -1048,33 +1049,46 @@ def setup_parameters_for_visibility_service(options) end def create_hash_struct_from_vm_or_template(vm_or_template, options) - data_hash = {:id => vm_or_template.id, - :name => vm_or_template.name, - :guid => vm_or_template.guid, - :uid_ems => vm_or_template.uid_ems, - :platform => vm_or_template.platform, - :logical_cpus => vm_or_template.cpu_total_cores, - :mem_cpu => vm_or_template.mem_cpu, - :allocated_disk_storage => vm_or_template.allocated_disk_storage, - :v_total_snapshots => vm_or_template.v_total_snapshots, - :evm_object_class => :Vm} - data_hash[:cloud_tenant] = vm_or_template.cloud_tenant if vm_or_template.cloud_tenant_id - if vm_or_template.operating_system - data_hash[:operating_system] = MiqHashStruct.new(:product_name => vm_or_template.operating_system_product_name) - end - if options[:include_datacenter] == true - data_hash[:datacenter_name] = vm_or_template.owning_blue_folder.try(:parent_datacenter).try(:name) + if vm_or_template.kind_of?(Hash) + data_hash = vm_or_template + data_hash[:logical_cpus] = data_hash.delete(:cpu_total_cores) + if vm_or_template[:operating_system_product_name] + data_hash[:operating_system] = MiqHashStruct.new(:product_name => vm_or_template[:operating_system_product_name]) + end + # TODO: solve owning_blue_folder for the "Hash method" (if needed...) + else + data_hash = {:id => vm_or_template.id, + :name => vm_or_template.name, + :guid => vm_or_template.guid, + :uid_ems => vm_or_template.uid_ems, + :platform => vm_or_template.platform, + :logical_cpus => vm_or_template.cpu_total_cores, + :mem_cpu => vm_or_template.mem_cpu, + :allocated_disk_storage => vm_or_template.allocated_disk_storage, + :v_total_snapshots => vm_or_template.v_total_snapshots} + data_hash[:cloud_tenant] = vm_or_template.cloud_tenant if vm_or_template[:cloud_tenant_id] + if vm_or_template.operating_system + data_hash[:operating_system] = MiqHashStruct.new(:product_name => vm_or_template.operating_system_product_name) + end + if vm_or_template[:ems_id] + data_hash[:ext_management_system] = MiqHashStruct.new(:name => vm_or_template.ext_management_system.name) + end + if options[:include_datacenter] == true + data_hash[:datacenter_name] = vm_or_template.owning_blue_folder.try(:parent_datacenter).try(:name) + end end + data_hash[:evm_object_class] = :Vm + # Handle EMS data, either with a cache, or with the relation (assumes it is # preloaded usually) if @_ems_allowed_templates_cache # don't have a key, so don't attempt to add to the data_hash - if vm_or_template.ems_id - ems_id = vm_or_template.ems_id + if vm_or_template[:ems_id] + ems_id = vm_or_template[:ems_id] # only fetch the ems if not fetched previously - unless @_ems_allowed_templates_cache.key?(vm_or_template.ems_id) + unless @_ems_allowed_templates_cache.key?(vm_or_template[:ems_id]) @_ems_allowed_templates_cache[ems_id] = ExtManagementSystem.find(ems_id).try(:name) end @@ -1082,8 +1096,6 @@ def create_hash_struct_from_vm_or_template(vm_or_template, options) data_hash[:ext_management_system] = MiqHashStruct.new(:name => @_ems_allowed_templates_cache[ems_id]) end end - elsif vm_or_template.ems_id - data_hash[:ext_management_system] = MiqHashStruct.new(:name => vm_or_template.ext_management_system.name) end MiqHashStruct.new(data_hash)