Querying and rendering an HTML table of ISP performance statistics

So, the first problem I'd try to address is that you're running SQL queries in your views. This is generally considered a bad practice (and there are helpful gems/frameworks that will throw errors if you try to do this).

The second major issue is the # of sql queries being run, as we're executing multiple queries for each measurement (1+N problem). This is the source of your performance issue in both the original example and your updated code.

Ideally you want all the relevant data to be loaded in a small number of SQL queries ahead of time.

It looks like you want 1 row per measurement per operator (is that right?). This is not easy to do with your current database schema. I've added a sql query that builds this but it's complex. This could be a great use case for a SQL view with an activerecord object on top of it.

I'm also assuming you only have at most 1 row per measurement + operator combo in the browsings/ftp_dls/ftp_uls tables. If this is correct, I'd add a unique index to enforce this on each table. If there could be multiple rows per operator and measurement then you'll need to adjust the SQL or this will return duplicate rows.

So I'd start by building SQL queries that generates the data you want. Something like...

@operators = ["Verizon", "USCell", "T-Mobile"]

# You don't need to hard-code the operators but I thought it made
# the SQL easier to understand
operator_join_sql = <<-SQL
  INNER JOIN (('Verizon'),('USCell'),('T-Mobile')) 
  AS operators(name) 
  ON true
SQL
browsings_join_sql = <<-SQL
  LEFT OUTER JOIN browsings ON browsings.operator = operators.name 
  AND browsings.measurment_id = measurements.id
SQL
ftp_uls_join_sql = <<-SQL
  LEFT OUTER JOIN ftp_uls ON ftp_uls.operator = operators.name 
  AND ftp_uls.measurment_id = measurements.id
SQL
ftp_dls_join_sql = <<-SQL
  LEFT OUTER JOIN ftp_dls ON ftp_dls.operator = operators.name 
  AND ftp_dls.measurment_id = measurements.id
SQL
select_sql = <<-SQL
  SELECT measurements.*
  , operators.name AS operator
  , browsings.avg AS ftp_urls_avg
  , browsings.min AS ftp_urls_min
  , browsings.max AS ftp_urls_max
  , ftp_urls.avg AS ftp_urls_avg
  , ftp_urls.min AS ftp_urls_min
  , ftp_urls.max AS ftp_urls_max
  , ftp_drls.avg AS ftp_drls_avg
  , ftp_drls.min AS ftp_drls_min
  , ftp_drls.max AS ftp_drls_max
SQL

@measurement_by_operator = @measurements.select(select_sql)
  .joins(operator_join_sql)
  .joins(browsings_join_sql)
  .joins(ftp_uls_join_sql)
  .joins(ftp_dls_join_sql)

Then, in your view, you would do:

<% @measurement_by_operator.each do |measurement| %>
  <tr>
    <td><%= measurement.location %></td>
    <td><%= measurement.event %></td>
    <td><%= measurement.date %></td>
    <td><%= measurement.operator %></td>
    <td><%= measurement.browsings_avg %></td>
    <td><%= measurement.browsings_min %></td>
    <td><%= measurement.browsings_max %></td>
    <td><%= measurement.ftp_uls_avg %></td>
    <td><%= measurement.ftp_uls_min %></td>
    <td><%= measurement.ftp_uls_max %></td>
    <td><%= measurement.ftp_dls_avg %></td>
    <td><%= measurement.ftp_dls_min %></td>
    <td><%= measurement.ftp_dls_max %></td>
  </ul>
<% end %>

---- EDIT ----

Some additional thoughts on your data schema...

Your three tables (browsings, ftp_uls, fpt_dls) have identical schema. They could be a single table (e.g. "measurement_readings" with a a "type" column, for example).

While the first approach I'm suggesting is probably best, it's fairly advanced. With the readings in a single table, there's an alternative that might be more approachable and still reasonable. Something like:

In your model (or better, a helper or presenter!)

class Measurement
  def for(operator, type)
    measurement_readings.detect{|mr| mr.operator == operator && mr.type == type } || NullMeasurementReading.new
  end
end
# A `NullMeasurementReading` is just a nullobject pattern. Could be a PORO or subclass of MeasurementReading or whatever.
# Alternatively use `&.` in the view.
# We're using `detect` here because we expect the array of readings to already be loaded 
# and explicitly *do not* want to trigger SQL queries

your controller

@measurements = @measurements.preload(:measurement_readings)

in the view

<% @measurements.each do |measurement| %>
  <% operators.each do |operator| %>
    <tr>
      <td><%= measurement.location %></td>
      <td><%= measurement.event %></td>
      <td><%= measurement.date %></td>
      <td><%= operator %></td>
      <td><%= measurement.for(operator, 'browsings').avg %>
      <td><%= measurement.for(operator, 'browsings').min %>
      <td><%= measurement.for(operator, 'browsings').max %>
      <%# etc.. %>
   </tr>
  <% end %>
<% end %>

there is many way to refactor this

first change operator into integer/enum instead string, or change operator to become an object/table instead with a name and order column

so that we can sort the browsings/ftpdl/ftp ul using operator column more easier

then to simplify the loop and query we can transform it into and array first then render the table row/data from that array

the new measurement_controller.rb index method would be like this

def index
  @measurements = Measurement.all #we could also add eager loading here 
                 #Measurement.all.includes(:browsings, :ftp_dls, :ftp_uls)
  @rows = []
  @measurements.each do |measurement|
    row = []
    Operator.all.each_with_index do |operator, index|
      row << [measurement.location, measurement.event, measurement.date, operator.name]
    end
    measurement.browsings.each_with_index do |browsing, index|
      row[index].concat([browsing.avg, browsing.min, browsing.max])
    end 
    measurement.ftp_dls.each_with_index do |ftp_dl, index| 
      row[index].concat([ftp_dl.avg, ftp_dl.min, ftp_dl.max])
    end
    measurement.ftp_uls.each_with_index do |ftp_ul, index|
      row[index].concat([ftp_ul.avg, ftp_ul.min, ftp_ul.max])
    end
    @rows << row
  end 
  @rows = @rows.flatten(1)
end

the new measurements/index.html.erb <tbody> table render would be like this

<tbody>
  <% @rows.each do |row|%>
    <tr>
      <% row.each do |data| %>
        <td><%= data %></td>
      <% end %>
    </tr>
  <% end%>
</tbody>

I think it will perform faster than the code before and the code is more readable and also we avoid loop inside loop idk but people said loop inside loop is bad

I just test it and in my use case it reduce the load time from 266693.0 ms to 13103.0 ms and to 7432.4 ms with eager loading