Skip to content

Commit

Permalink
Refactor get_ems_folders to create less strings
Browse files Browse the repository at this point in the history
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 constants in this method were being created every time
  this method was called.  For the DATACENTER_FOLDER_TYPE 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).
  • Loading branch information
NickLaMuro committed Apr 27, 2018
1 parent c2e01c9 commit c12c75b
Showing 1 changed file with 11 additions and 3 deletions.
14 changes: 11 additions & 3 deletions app/models/miq_request_workflow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -855,20 +855,28 @@ def ems_has_clusters?
false
end

FOLDER_PATH_SEPARATOR = " / ".freeze
DATACENTER_FOLDER_TYPE = "Datacenter".freeze
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 << FOLDER_PATH_SEPARATOR << folder.name
end
dh[folder.id] = path unless folder.type == DATACENTER_FOLDER_TYPE
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
Expand Down

0 comments on commit c12c75b

Please sign in to comment.