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.