Speed up long running Ruby code

Question

I'm running code which works fast locally, but when it's on Heroku it's slow. Heroku says this is because there is less memory and CPU power on my hosting than locally. I'm trying to rewrite my code so it's more efficient, but I'm not sure how.

This is what they said:

...[my] method (especially, a.each part) is taking time. I think a itself is pretty long, and you're transpose it with subs.keys, and do some process for each of them. Even though s.gsub!(*pair) is not "long running" code, if you run this million times (actually runs 1,161,216 times for the word "startup"), it'll be "slow" request.

Here is my code:

def substitute(word)
 subs = Hash.new
 Section.all.to_a.each do |s|
 alternates_array = Array.new
 s.substitutions.each do |alt|
 alternates_array << alt.alternate
 end
 alternates_array << s.part
 new_hash = Hash.new
 subs[s.part] = alternates_array
 end
 a = subs.values
 a = a.first.product(*a.drop(1))
 alternates = Array.new
 a.each do |a|
 new_string = [subs.keys, a].transpose.each_with_object(word.dup){|pair, s| s.gsub!(*pair)}
 end
 return alternates
end

Section Model

class Section
 include Mongoid::Document
 field :part, type: String
 embeds_many :substitutions
 accepts_nested_attributes_for :substitutions
end

Substitution Model

class Substitution
 include Mongoid::Document
 field :alternate, type: String
 embedded_in :section
end

• user2270029: Section model is added.
• ChrisWue: Could you add some explanation what your code is supposed to do? This will make it easier for people to review it.

Answer 1

Read with batches
Looping over Section.all.to_a is very memory greedy (it reads everything to memory before starting to process it). You will be far better off using batches:

Section.find_each do |section|
 # ...
end

Naming
Using member names like s, a, etc. is not very readable, and makes understanding what your code does very difficult. For that reason, it is very hard for us reviewers to give you an intelligent advice on how your code can be improved - we just don't understand what it does...

Shadowing
a.each do |a| - to add insult to injury, you shadow the a member when you loop over it. This makes your code totally unreadable. I still don't understand what that loop is all about.

Loop bloat
The line a = a.first.product(*a.drop(1)) is very suspicious, since it bloats the amount of data your are looping on by several scales. Are you sure that is what you want to do?

Where does your data go?
The new_string, which holds the result for each iteration is not seemed to be saved anywhere...

Filter your data
At the bottom line, you are substituting words in your input. Not all sections exist in all inputs, but you search for them over and over again. You might save a lot of calculations if you filter irrelevant sections in your code, before looping on them:

Section.find_each do |section|
 next unless word[section.part]
 alternate_array #...
end

Doing stuff over and over again
I'm pretty sure, though I might be wrong, that at the end s.gsub!(*pair) ends up running the same "pairs" more than once on the same string (or its duplicates). It will be better strategy to cache results, or maybe pre-process your data (build a template, and only set the variables, or something).

---

Edit
You could use a variant of gsub! which receives a hash as the replacement:

If the second argument is a Hash, and the matched text is one of its keys, the corresponding value is the replacement string.

So you could write:

replace_regexp = Regexp.union(subs.keys)
replaced_words = a.map do |alts|
 word.gsub(replace_regexp, Hash[[subs.keys, alts].transpose])
end

This does a single gsub per combination (instead of once per key), and further reduces the run complexity of your code.