I've got this block of code:
date_counter = Time.mktime(2011,01,01,00,00,00,"+05:00")
@weeks = Array.new
(date_counter..Time.now).step(1.week) do |week|
logger.debug "WEEK: " + week.inspect
@weeks << week
end
Technically, the code works, outputting:
Sat Jan 01 00:00:00 -0500 2011
Sat Jan 08 00:00:00 -0500 2011
Sat Jan 15 00:00:00 -0500 2011
etc.
But the execution time is complete rubbish! It takes approximately four seconds to compute each week.
Is there some grotesque inefficiency that I'm missing in this code? It seems straight-forward enough.
I'm running Ruby 1.8.7 with Rails 3.0.3.
Assuming MRI and Rubinius use similar methods to generate the range the basic algorithm used with all the extraneous checks and a few Fixnum optimisations etc. removed is:
class Range
def each(&block)
current = @first
while current < @last
yield current
current = current.succ
end
end
def step(step_size, &block)
counter = 0
each do |o|
yield o if counter % step_size = 0
counter += 1
end
end
end
(See the Rubinius source code)
For a Time
object #succ
returns the time one second later. So even though you are asking it for just each week it has to step through every second between the two times anyway.
Build a range of Fixnum's since they have an optimised Range#step
implementation.
Something like:
date_counter = Time.mktime(2011,01,01,00,00,00,"+05:00")
@weeks = Array.new
(date_counter.to_i..Time.now.to_i).step(1.week).map do |time|
Time.at(time)
end.each do |week|
logger.debug "WEEK: " + week.inspect
@weeks << week
end
Yes, you are missing a gross inefficiency. Try this in irb to see what you're doing:
(Time.mktime(2011,01,01,00,00,00,"+05:00") .. Time.now).each { |x| puts x }
The range operator is going from January 1 to now in increments of one second and that's a huge list. Unfortunately, Ruby isn't clever enough to combine the range generation and the one-week chunking into a single operation so it has to build the entire ~6million entry list.
BTW, "straight forward" and "gross inefficiency" are not mutually exclusive, in fact they're often concurrent conditions.
UPDATE: If you do this:
(0 .. 6000000).step(7*24*3600) { |x| puts x }
Then the output is produced almost instantaneously. So, it appears that the problem is that Range doesn't know how to optimize the chunking when faced with a range of Time objects but it can figure things out quite nicely with Fixnum ranges.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With