I have two active records StudentDemographics
and StudentWeeklyReport
both having has_many
relation like this:
class StudentDemographics < ActiveRecord::Base
has_many :student_weekly_reports, :foreign_key => :student_id
end
I have to check marks for each student in the last fifth week seminar with the latest one. If the result is true
, student should be active, else inactive. I have the following code. Here, I am repeating the loop with each date. @distinct
is an array of dates.
def active_learner
safe = []
@active = []
@inactive = []
@distinct = StudentDemographics.select(:date).map(&:date).uniq.sort
@students = StudentDemographics.includes(:student_weekly_reports).select("student_id,date")
@distinct.each_with_index do |date,i|
if i < 4
@count = StudentDemographics.where("date <= ?", date).count
@active[i] = @count
@inactive[i] = 0
else
sum = safe.length
active = inactive = 0
(@students - safe).each do |student|
next if student.date > date
@stu = student.student_weekly_reports.last(5)
if @stu.length > 4
if @stu[4].golden_eggs > @stu[0].golden_eggs
safe << student
active += 1
else
inactive += 1
end
else
safe << student
active += 1
end
end
@active[i] = active + sum
@inactive[i] = inactive
end
end
end
The performance is not good. It's taking more than 3 secs of time. My MySQL database has 13600 in the StudentWeeklyReports
table and 2000 in the StudentDemographics
table. Can anyone suggest how I can optimize this?
2 Answers 2
I'd suggest a couple of things:
the
for i in [email protected]
takes your array's length and uses it to construct a range which it turns into an array again. Not necessarily time consuming but unnecessary and hard to read. Ruby gives youeach_with_index
which can do it in a nicer, more concise way.favoring working with IDs should speed things up because no Active Record objects need to be created and filled and it's quite enough for your calculations. One important tool for that is
pluck
.I also removed several local variables as you only use them for tallies and that's, again, not really necessary. But no real speedup here, either.
TL;DR: here's the re-factored version with calculations in ruby.
safe_ids = safe.map(&:student_id)
@distinct.each_with_index do |date, i|
@active[i] = safe_ids.length
@inactive[i] = 0
@student_ids = StudentDemographics.where("id not in (?)", safe_ids).where("date <= ?", date).pluck("student_id")
@student_ids.each do |student_id|
@stu = StudentWeeklyReport.where(:student_id => student_id).last(5).pluck(:golden_eggs)
if @stu.length <= 4 || @stu[4] > @stu[0]
safe << student_id
@active[i] += 1
else
@inactive[i] += 1
end
end
end
You could also try and have the whole calculation done by the database, by using group_by
and having
in a fairly complicated but probably very fast query.
But I'll leave it to the astute reader to figure that one out (mainly since I couldn't manage to get it right in time).
Issing a separate SQL query per record almost always leads to a performance problem. You should be able to formulate this code using just one query, and let the database server do all the work. Stop thinking in terms of Ruby code, and develop a good SQL query that returns three columns (date, active_count, inactive_count).
-
\$\begingroup\$ Alas, such a query might be impossible in MySQL due to the lack of a
RANK()
function, which would be necessary to work on a pair of seminars four weeks apart. It should be possible in, say, PostgreSQL, which does have such a function. \$\endgroup\$200_success– 200_success2013年10月23日 09:44:18 +00:00Commented Oct 23, 2013 at 9:44
Explore related questions
See similar questions with these tags.
StudentDemographics
has adate
field, butStudentWeeklyReport
doesn't? I would have expected the opposite to be true. Perhaps you could provide more context about the whole problem you are trying to solve, and what the motivation is. \$\endgroup\$