2
\$\begingroup\$

So I have three methods and I'm not sure how to optimize them. I will be working on this and updating this post while I do but basically running these three methods is central to some parts of my application and they are causing a great deal of slowdown. I just have too many queries being ran because of them. If I could get some optimization help, I would appreciate it!

def self.appropriate_users(current_user)
    unless current_user.organization.nil?
      if current_user.has_role? :director
        @users = User.where(organization: current_user.organization)
      elsif current_user.has_role? :supervisor
        @users = current_user.subordinates
      elsif current_user.has_role? :leader
        @users = []
        groups = current_user.leading_groups.each do |g|
          g.users.each do |u|
            @users << u
          end
        end

        @users.uniq
      else
        @users = User.none
      end
    else
      @users = User.all if current_user.has_role? :admin
    end
    @users ||= []

    @users
  end

  def expired_trainings(current_user)
    retval = []
    Organization.appropriate_users(current_user).each do |user|
      Expiration.where("user_id = #{user.id} and expire_on <= '#{Date.today}'").each do |expir|
        retval << expir
      end
    end
    ttypes = []
    if current_user.max_role.downcase == 'leader'
      current_user.leading_groups.each do |g|
        g.training_types.each do |tt|
          ttypes << tt
        end
      end
      ttypes.flatten.uniq!
      retval.delete_if { |expir| !ttypes.include?(expir.training_type) }
    end
    retval.flatten
  end

  def current_trainings(current_user)
    retval = []
    Organization.appropriate_users(current_user).each do |user|
      Expiration.where("user_id = #{user.id} and expire_on >= '#{Date.today}' and expire_on <= '#{Date.today + 30.days}'").each do |expir|
        retval << expir
      end
    end
    ttypes = []
    if current_user.max_role.downcase == 'leader'
      current_user.leading_groups.each do |g|
        g.training_types.each do |tt|
          ttypes << tt
        end
      end
      ttypes.flatten.uniq!
      retval.delete_if { |expir| !ttypes.include?(expir.training_type) }
    end
    retval.flatten
  end

  def future_trainings(current_user)
    retval = []
    Organization.appropriate_users(current_user).each do |user|
      Expiration.where("user_id = #{user.id} and expire_on >= '#{Date.today + 31.days}'").each do |expir|
        retval << expir
      end
    end
    ttypes = []
    if current_user.max_role.downcase == 'leader'
      current_user.leading_groups.each do |g|
        g.training_types.each do |tt|
          ttypes << tt
        end
      end
      ttypes.flatten.uniq!
      retval.delete_if { |expir| !ttypes.include?(expir.training_type) }
    end
    retval.flatten
  end
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

You could benchmark and see if a INNER JOIN would be faster on the Expiration model w/ the User model. I'm assuming they are not one to one, since you are grabbing multiple expirations per user.

How big is the Users table? If it's relatively small, and not growing exponentially you might not need to use a join.

If it has millions of records, you are essentially doing a million SELECTs:

Benchmark.realtime  { User.all.each { |x| Expiration.where(:user_id => x.id) }

Which can be bad for performance and cause your MySQL server to be overloaded.

There is also something to look out for is the N + 1 problem, which is a very similar problem to what might be happening to you:

https://www.codemy.net/posts/optimizing-your-rails-app-part-1-n-1-queries

Edit:

N+1 queries occur when you write a piece of code that executes a SQL query many times (N times depending on the code), where one query would have been appropriate. It occurs commonly in loops, maps, and places where lazy loading is not enabled. It's very common in views that display tabular info that uses other tables, and is not noticeable in a development environment where you're not testing a ton of data.

\$\endgroup\$
0

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.