3
\$\begingroup\$

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?

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Oct 22, 2013 at 10:29
\$\endgroup\$
2
  • \$\begingroup\$ StudentDemographics has a date field, but StudentWeeklyReport 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\$ Commented Oct 23, 2013 at 6:07
  • \$\begingroup\$ @200_success StudentWeeklyReport also has date field...I have edited my post...In first 4 weeks every one is active...from 5th on wards I have to calculate active students based on the last five weeks data along with that week...if the user marks for 5th week is greater than first week we can consider him as active... \$\endgroup\$ Commented Oct 23, 2013 at 6:21

2 Answers 2

1
\$\begingroup\$

I'd suggest a couple of things:

  1. 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 you each_with_index which can do it in a nicer, more concise way.

  2. 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.

  3. 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).

answered Oct 22, 2013 at 14:41
\$\endgroup\$
0
\$\begingroup\$

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).

answered Oct 22, 2013 at 23:56
\$\endgroup\$
1
  • \$\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\$ Commented Oct 23, 2013 at 9:44

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.