Ruby CSV reading performance problems

Question

I (ruby beginner) have a performance issue with the following ruby code. The idea is to aggregate a CSV into a smaller, better-to-handle file and calculating the average of some values. Basically the same code in groovy runs about 30 times faster. And I can't believe that Ruby is that slow! ;-)

Some background: The file consists of lines like this (it is an output file of a JMeter performance test):

timeStamp|elapsed|label|responseCode|responseMessage|threadName|dataType|success|failureMessage|bytes|Latency
2013-05-17_16:30:11.261|4779|Session_Cookie|200|OK|Thread-Gruppe 1-1|text|true||21647|4739

All values of e.g. a certain minute are selected by looking at the first 16 characters of the timestamp:

2013-05-17_16:30:11.261 => 2013-05-17_16:30

I wanted to collect values in buckets/slices which are represented by the shortened timestamp and the label (third column). The value of the second column ("elapsed") is summed up.

require 'csv'
require_relative 'slice'

# Parameters: <precision> <input file> [<output file>]
precision = ARGV[0].to_i
input_file = ARGV[1]
output_file = ARGV[2]

time_slices = Hash.new

CSV.foreach(input_file, {:col_sep => '|', :headers => :first_line}) do |row|
  current_time_slice = row['timeStamp'][0, precision]

  if time_slices[current_time_slice] == nil
    time_slices[current_time_slice] = Hash.new
  end

  if time_slices[current_time_slice][row['label']]
    time_slices[current_time_slice][row['label']].put_line(row)
  else
    new_slice = Slice.new(current_time_slice, row['label'])
    new_slice.put_line(row)
    time_slices[current_time_slice][row['label']] = new_slice
  end
end

out = File.new(output_file, 'a')
out.puts 'time|label|elapsed_average'
time_slices.values.each do |time_slice|
  time_slice.values.each do |slice|
    out.puts slice.aggregated_row
  end
end

The slice class looks like this:

class Slice
  attr_accessor :slice_timestamp, :slice_label, :lines, :sum, :count

  def initialize(slice_timestamp, slice_label)
    @slice_timestamp = slice_timestamp
    @slice_label = slice_label
    @count = 0
    @sum = 0
  end

  def put_line(line)
    @sum = @sum + line[1].to_i
    @count = @count + 1
  end

  def average
    @sum / @count
  end

  def aggregated_row
    @slice_timestamp + '|' + @slice_label + '|' + average.to_s
  end
end

I think that I chose a quite straightforward and non-optimized approach, but still - the same approach is much faster in Groovy. What can be the reason for that?

Answer 1

if time_slices[current_time_slice] == nil
  time_slices[current_time_slice] = Hash.new
end

It's more idomatic to write

time_slices[current_time_slice] ||= {}

In general don't use Hash.new if you can just write {}. You can rid of that statement alltogether though if you declare the time_slices variable with

time_slices = Hash.new { |h, k| h[k] = {} }

instead of time_slices = Hash.new.

if time_slices[current_time_slice][row['label']]
  time_slices[current_time_slice][row['label']].put_line(row)
else
  new_slice = Slice.new(current_time_slice, row['label'])
  new_slice.put_line(row)
  time_slices[current_time_slice][row['label']] = new_slice
end

Also

time_slices[current_time_slice][row['label']] ||= Slice.new ...

is more idiomatic here. But there is more to say here:

1. Don't forget that Ruby is an interpreted language in principle (although the byte compiler of Ruby 1.9 relativizes that), so don't repeat complex references like time_slices[current_time_slice][row['label']]. But that's not only an performance but even more so an readability issue.
2. Why does a Slice instance has to know the current_time_slice and label? That's completely redundant, since that information is already in the Hash. Get rid of that.

Considering this I would write the loop like this:

time_slices = Hash.new { |h, k| h[k] = Hash.new { |h, k| h[k] = Slice.new } }

CSV.foreach(input_file, col_sep: '|', headers: :first_line) do |row|
  current_time_slice = row['timeStamp'][0, precision]
  time_slices[current_time_slice][row['label']].put_line(row)
end

Of course you then have to modify your output loop a little

time_slices.each do |slice_timestamp, time_slice|
  time_slice.each do |label, slice|
    out.puts [slice_timestamp, label, slice.average].join('|')
  end
end

So you don't need the Slice#aggregated_row method anymore. But that's a good idea anyway: e.g. why does Slice has to know about your pipe separator? That's not a good separation of concerns.

Note that I avoid the + operator here. That would create a new String instance with every + call. Alternatively you can also try <<, but be aware that this modifies the leftmost string (but oftentimes that doesn't matter).

In contrast there is no penalty in using + in groovy normally, because modern JVMs are using StringBuilder under the hood for it when they see fit. But in Ruby you still should be aware of its implications.

attr_accessor :slice_timestamp, :slice_label, :lines, :sum, :count

Why are you declaring these accessors? They are never used.