CODEWITHSUNDEEP
ruby-on-railsactiverecordeager-loading
steven_noble
steven_noble

Reputation: 4203

Is this eager loading doing what I think it's meant to be doing?

I've just changed an action in competitors_controller.rb from...

def audit
  @review = Review.find(params[:review_id])
  @competitor = @review.competitors.find(params[:id])
  respond_with(@review, @competitor)
end

...to...

def audit
  @review = Review.find(params[:review_id])
  @competitor = Competitor.find(params[:id], :include => {:surveys => {:question => [:condition, :precondition]}})
  respond_with(@review, @competitor)
end

...as the page was timing out on loading.

The underlying associations are:

class Competitor < ActiveRecord::Base
  has_many :surveys
end

class Survey < ActiveRecord::Base
  belongs_to :competitor
  belongs_to :question
  delegate :dependencies, :precondition, :condition, :to => :question  
end

class Question < ActiveRecord::Base
  has_many :dependancies, :class_name => "Question", :foreign_key => "precondition_id"
  belongs_to :precondition, :class_name => "Question"
  has_many :surveys, :dependent => :delete_all
end

Basically, the audit.html.haml page loads:

 @competitor.surveys.{sorting, etc}.each do |s|
    s.foo, s.bar
    s.{sorting, etc}.dependant_surveys.each do |s2|
       s2.foo, s2.bar
       s2.{sorting, etc}.dependant_surveys.each do |s3|
           s3.foo, s3.bar, etc etc

If I nest it too far, the page doesn't load before it times out.

What I need to know is whether the eager loading I have inserted into competitor_controllers.rb theoretically speeds up each of the following two methods, which are called so often when building audit.html.haml?

class Survey < ActiveRecord::Base
  def dependant_surveys
    self.review.surveys.select{|survey| self.dependencies.include?(survey.question)}
  end
end

class Question < ActiveRecord::Base
  def dependencies
    Question.all.select{|question| question.precondition == self}
  end
end

(I say "theoretically" as I know the question could also be answered with benchmarking. But before I get that far I want to check I have the theory right.)

Upvotes: 1

Views: 77

Answers (3)

Mike Campbell
Mike Campbell

Reputation: 7978

As far as I know you can only include associations for eager loading. I don't think what you have will work at all. You haven't shown us where Question.condition is defined, or Survey.review. Also, the associations that you are eager loading won't be used by your methods dependent_surveys and dependencies as they're performing their own queries.

AFAICT your eager loading won't make a jot of difference, or it will probably slow it down. I think you'll have to refactor your dependent_surveys as an association and eager load that. I haven't got the slightest clue what that method is trying to do, on a high level, so I'm not even going to attempt to refactor it.

Upvotes: 0

Mike Szyndel
Mike Szyndel

Reputation: 10593

You do a lot of processing in ruby, and there's no need for that. You should move all operations like 

Question.all.select{|question| question.precondition == self}
@competitor.surveys.{sorting, etc}

to database.

If I understand properly, first line is meant to select all records that have a precondition set to given question. Remember that Question.all returns an array so you perform select in array, and you could do it in db with simple scope scope :has_precondition, -> {|q| where(precondition_id: q.id} or so.

Given you always sort models in same way, you may consider creating a default scope with order clause. Doing it in ruby is very counter-efficient.

Upvotes: 1

RobHeaton
RobHeaton

Reputation: 1390

The eager loading looks like it should work, but much more importantly these are things that you could and should be doing using SQL. Loading and iterating through all the ActiveRecord models in your DB has the potential to be incredibly time consuming (as you've found), whereas working out how to do it in SQL will allow you to load all the models directly from a single query. It can take a while to get a handle on this, but it's well worth it!

Upvotes: 1

Related Questions