From 1e0d6afdbe862397fa1f6c03fb186a403af0820d Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Tue, 24 Apr 2018 16:25:01 -0500 Subject: [PATCH] Refactor get_ems_folders to create less strings Creating some strings in this method is inevitable, since we are generating string paths based on this compiled info, but there were a lot of excessive strings being created because of it: - Both of the strings in this method were being created every time this method was called. For the "Datacenter".freeze one, it was just used as a comparison as well - Using << where possible will avoid generating new substrings, so we are only creating one new string at most. - Deferred the `.dup` of full_path until a new string was known that it would be generated. This did require creating a new variable to hold on to the existing variable, but that wasn't a big deal and didn't increase the number of allocations. Split up some ternary operators as well just to make running profilers on this code easier to determine what as causing allocations (granularity in tracing object allocations in ruby is only down to a line by line basis). --- app/models/miq_request_workflow.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app/models/miq_request_workflow.rb b/app/models/miq_request_workflow.rb index f6207b56db2..ba233e2c7ad 100644 --- a/app/models/miq_request_workflow.rb +++ b/app/models/miq_request_workflow.rb @@ -856,19 +856,25 @@ def ems_has_clusters? end def get_ems_folders(folder, dh = {}, full_path = "") + path = full_path if folder.evm_object_class == :EmsFolder if folder.hidden return dh if folder.name != 'vm' else - full_path += full_path.blank? ? folder.name.to_s : " / #{folder.name}" - dh[folder.id] = full_path unless folder.type == "Datacenter" + path = full_path.dup + if path.blank? + path << folder.name.to_s + else + path << " / ".freeze << folder.name + end + dh[folder.id] = path unless folder.type == "Datacenter".freeze end end # Process child folders @_get_ems_folders_prefix ||= _log.prefix node = load_ems_node(folder, @_get_ems_folders_prefix) - node.children.each { |child| get_ems_folders(child.attributes[:object], dh, full_path) } unless node.nil? + node.children.each { |child| get_ems_folders(child.attributes[:object], dh, path) } unless node.nil? dh end