Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Ruby's range step method causes very slow execution?

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.

like image 478
Aaron Vegh Avatar asked Mar 13 '11 01:03

Aaron Vegh


2 Answers

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.

Edit: Solution

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
like image 96
Nemo157 Avatar answered Oct 27 '22 00:10

Nemo157


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.

like image 37
mu is too short Avatar answered Oct 26 '22 23:10

mu is too short