Optimizing Rails API Controller for Improved Performance

Question

module Api
  module V1
    class AngularDashboardApisController < BaseController
      include RolesHelper
      before_action :verify_auth_token
      before_action -> { verify_user_permission(2) }

  def dashboard_data_a
    @user = User.find_by(id: params[:user_id])

    unless @user
      return json_response('Bad Request', false, [], :bad_request)
    end

    get_con_tech

    if !params[:from_date].present? && !params[:to_date].present?
      params[:from_date] = DateTime.now.beginning_of_month.to_date
      params[:to_date] = Date.today
    else
      params[:from_date] = params[:from_date].to_date
      params[:to_date] = params[:to_date].to_date
    end

    all_years = Lead.pluck(:created_at).map { |x| x.year }.uniq
    
    check_role(@user, params)
    filter_by_scope
    set_old_and_new_country_tech_hash_A_and_B
    graph_data
    lead_user_summary

    json_success_send_dashboard_graph_response('Dashboard-A', true,@countries_A, @countries_B, @countries_C,@country_total_A, @country_total_B,@country_total_C, @country_tech_array_hash_A, @country_tech_array_hash_B, @country_tech_array_hash_C,Hash[@technology_hash_A.sort_by{|k, v| v}].keys, Hash[@technology_hash_B.sort_by{|k, v| v}].keys, Hash[@technology_hash_C.sort_by{|k, v| v}].keys,@graphdata_lst, @Summary,all_years, :ok)
  end
  
  def graph_data
    graph_lead = sort_by_date(@country_tech_graph)
    @graphdata_map = Hash.new(0) # Initialize a default value of 0 for missing keys
    @graphdata_lst = []
  
    if params[:mode] == 'month' || params[:mode] == 'day' || params[:mode] == 'week'
      graph_lead.each do |lead|
        date_key = case params[:mode]
                   when 'month'
                     month_map[lead.date.month.to_s]
                   when 'day', 'week'
                     lead.date.to_date
                   end
  
        @graphdata_map[date_key] += 1
      end
    end
  
    @graphdata_map.each do |key, value|
      @graphdata_lst.push({ name: "#{key}", value: "#{value}", tooltip: "#{key}" })
    end
  end      

  def filter_by_scope
    @country_tech_hash_A, @country_tech_hash_A_graph, @country_tech_hash_id_A = filter_and_group_leads(@technology_hash_A, params)
    @country_tech_hash_B, @country_tech_hash_B_graph, @country_tech_hash_id_B = filter_and_group_leads(@technology_hash_B, params)
    @country_tech_hash_C, @country_tech_hash_C_graph, @country_tech_hash_id_C = filter_and_group_leads(@technology_hash_C, params)

    @country_tech_graph = @country_tech_hash_A + @country_tech_hash_B + @country_tech_hash_C

    @country_tech_hash_A = @country_tech_hash_A_graph
    @country_tech_hash_B = @country_tech_hash_B_graph
    @country_tech_hash_C = @country_tech_hash_C_graph
  end

  def set_old_and_new_country_tech_hash_A_and_B
    @new_country_tech_hash_id_A = {}
    @country_tech_hash_id_A.each do |key,value|
      if key[0] == nil
        key[0] = 'unknown'
      end
      k=key[0]+"", ""+key[1]
      @new_country_tech_hash_id_A[key[2]]=k
    end
    @new_country_tech_hash_id_A = @new_country_tech_hash_id_A.each_with_object({}) { |(k,v),g| (g[v] ||= []) << k }

    @country_tech_array_hash_A = {}
    @country_tech_array_A = []
    @countries_A = Set.new
    @countries_B = Set.new
    @countries_C = Set.new
    index = 0
    @klass = Klass.find_by(name: "Lead")
    @fields = @user.fields_for_table_with_order1(klass: @klass)
    @country_tech_hash_A.each do |key,value|
      array = []
      @countries_A.add(key[0])
      ids = @new_country_tech_hash_id_A[[key[0],key[1]]]
      @leads = Lead.where('id in (?)', ids).select(@fields.pluck(:name))
      if @country_tech_array_hash_A.key? (key[1])
        @country_tech_array_hash_A[key[1]] = @country_tech_array_hash_A[key[1]].push(array.push(key[0],value,@leads))
      else
        @country_tech_array_A = []
        @country_tech_array_hash_A[key[1]] = @country_tech_array_A.push(array.push(key[0],value,@leads))
      end
      @country_hash_A[key[0]] = index
      index = index + 1
      if @country_total_A.key? (key[0])
        @country_total_A[key[0]] = @country_total_A[key[0]] + value
      else
        @country_total_A[key[0]] =  value
      end
    end

    @new_country_tech_hash_id_B = {}
    @country_tech_hash_id_B.each do |key,value|
      if key[0] == nil
        key[0] = 'unknown'
      end
      k=key[0]+"", ""+key[1]
      @new_country_tech_hash_id_B[key[2]]=k
    end
    @new_country_tech_hash_id_B = @new_country_tech_hash_id_B.each_with_object({}) { |(k,v),g| (g[v] ||= []) << k }
    @country_tech_array_hash_B = {}
    @country_tech_array_B = []
    @country_tech_hash_B.each do |key,value|
      array = []
      @countries_B.add(key[0])
      ids = @new_country_tech_hash_id_B[[key[0],key[1]]]
      @leads = Lead.where('id in (?)', ids).select(@fields.pluck(:name))
      if @country_tech_array_hash_B.key? (key[1])
        @country_tech_array_hash_B[key[1]] = @country_tech_array_hash_B[key[1]].push(array.push(key[0],value,@leads))
      else
        @country_tech_array_B = []
        @country_tech_array_hash_B[key[1]] = @country_tech_array_B.push(array.push(key[0],value,@leads))
      end
      @country_hash_B[key[0]] = index
      index = index + 1
      if @country_total_B.key? (key[0])
        @country_total_B[key[0]] = @country_total_B[key[0]] + value
      else
        @country_total_B[key[0]] =  value
      end
    end

    @new_country_tech_hash_id_C = {}
    @country_tech_hash_id_C.each do |key,value|
      if key[0] == nil
        key[0] = 'unknown'
      end
      k=key[0]+"", ""+key[1]
      @new_country_tech_hash_id_C[key[2]]=k
    end
    @new_country_tech_hash_id_C = @new_country_tech_hash_id_C.each_with_object({}) { |(k,v),g| (g[v] ||= []) << k }
    @country_tech_array_hash_C = {}
    @country_tech_array_C = []
    @country_tech_hash_C.each do |key,value|
      array = []
      @countries_C.add(key[0])
      ids = @new_country_tech_hash_id_C[[key[0],key[1]]]
      @leads = Lead.where('id in (?)', ids).select(@fields.pluck(:name))
      if @country_tech_array_hash_C.key? (key[1])
        @country_tech_array_hash_C[key[1]] = @country_tech_array_hash_C[key[1]].push(array.push(key[0],value,@leads))
      else
        @country_tech_array_C = []
        @country_tech_array_hash_C[key[1]] = @country_tech_array_C.push(array.push(key[0],value,@leads))
      end
      @country_hash_C[key[0]] = index
      index = index + 1
      if @country_total_C.key? (key[0])
        @country_total_C[key[0]] = @country_total_C[key[0]] + value
      else
        @country_total_C[key[0]] =  value
      end
    end
  end

  def get_con_tech
    @country_hash_A = {}
    @country_hash_B = {}
    @country_hash_C = {}
    @technology_hash_A = {}
    @technology_hash_B = {}
    @technology_hash_C = {}

    @technology_A = CountryTechnology.where(group:'A',country_technology:'technology')
    @technology_B = CountryTechnology.where(group:'B',country_technology:'technology')
    @technology_C = CountryTechnology.where(group:'C',country_technology:'technology')

    @technology_A.each do |technology|
      @technology_hash_A[technology.name] = technology.position
    end

    @technology_B.each do |technology|
      @technology_hash_B[technology.name] = technology.position
    end

    @technology_C.each do |technology|
      @technology_hash_C[technology.name] = technology.position
    end
      @country_total_A = Hash[@country_hash_A.sort_by{|k, v| v}]
      @country_total_B = Hash[@country_hash_B.sort_by{|k, v| v}]
      @country_total_C = Hash[@country_hash_C.sort_by{|k, v| v}]
      @country_total_A.each { |k, v| @country_total_A[k] = 0 }
      @country_total_B.each { |k, v| @country_total_B[k] = 0 }
      @country_total_C.each { |k, v| @country_total_C[k] = 0 }
  end

  def lead_user_summary
    @LeadgroupByUser = Lead.filter_by_from_to_date_tech(params[:from_date], params[:to_date], @technology_hash_A.keys + @technology_hash_B.keys + @technology_hash_C.keys)
  
    filters = {
      source: params[:source],
      is_closed: params[:is_closed],
      live_chat_handle_name: params[:live_chat_handle_name],
      if_paid: params[:if_paid],
      if_organic: params[:if_organic],
      supportdesk: params[:supportdesk],
      terminate_reasons: params[:terminate_reasons],
      device: params[:device],
      user_id: params[:assign_to],
      technology: params[:technology]
    }
  
    filters.each do |filter_name, filter_value|
      @LeadgroupByUser = @LeadgroupByUser.send("filter_by_#{filter_name}", filter_value) if filter_value.present? && filter_value != "All"
    end
  
    @Summaryhash = @LeadgroupByUser.group(:user_id).count
    @LeadgroupByUser1 = @LeadgroupByUser.filter_by_is_closed("Yes")
    @Summaryhash1 = @LeadgroupByUser1.group(:user_id).count
    @Summary = []
    @klass = Klass.find_by(name: "Lead")
    @fields = @user.fields_for_table_with_order1(klass: @klass)
  
    @Summaryhash.each do |key, value|
      total_closed_count = @Summaryhash1[key].present? ? @Summaryhash1[key] : 0
      @assignedLeads1 = @LeadgroupByUser.where(user_id: key).select(:id, @fields.pluck(:name))
      @Summary.push({'key': key, 'total_assigned_count': value, 'total_closed_count': total_closed_count, 'assigned_leads': @assignedLeads1, 'labels': @fields.pluck(:name)})
    end
  end

  private

  def sort_by_date(arr)
    arr.sort_by { |h| h["date"].to_date.to_s.split('-') }
  end

  def filter_and_group_leads(technology_hash, params)
    filtered_leads = Lead.filter_by_from_to_date_tech(params[:from_date], params[:to_date], technology_hash.keys)
    filtered_leads = filtered_leads.filter_by_source(params[:source]) if params[:source].present? && params[:source] != "All"
    filtered_leads = filtered_leads.filter_by_is_closed(params[:is_closed]) if params[:is_closed].present? && params[:is_closed] != "All"
    filtered_leads = filtered_leads.filter_by_livechathandle(params[:live_chat_handle_name]) if params[:live_chat_handle_name].present? && params[:live_chat_handle_name] != "All"
    filtered_leads = filtered_leads.filter_by_if_paid(params[:if_paid]) if params[:if_paid].present? && params[:if_paid] != "All"
    filtered_leads = filtered_leads.filter_by_if_organic(params[:if_organic]) if params[:if_organic].present? && params[:if_organic] != "All"
    filtered_leads = filtered_leads.filter_by_supportdesk(params[:supportdesk]) if params[:supportdesk].present? && params[:supportdesk] != "All"
    filtered_leads = filtered_leads.filter_by_terminate_reasons(params[:terminate_reasons]) if params[:terminate_reasons].present? && params[:terminate_reasons] != "All"
    filtered_leads = filtered_leads.filter_by_device(params[:device]) if params[:device].present? && params[:device] != "All"
    filtered_leads = filtered_leads.filter_by_user_id(params[:assign_to]) if params[:assign_to].present? && params[:assign_to] != "All"
    filtered_leads = filtered_leads.filter_by_technology(params[:technology]) if params[:technology].present? && params[:technology] != "All"
  
    graph_data = filtered_leads.group(:country, :technology).count
    country_tech_hash_id = filtered_leads.group(:country, :technology, :id).count

    return filtered_leads, graph_data, country_tech_hash_id
  end
end
end
end

I have this Rails API controller that retrieves data from a database, and I'm experiencing performance issues. The API takes different times to fetch the same data every time. I'm looking for advice on optimizing the code for better performance.

I've attempted to optimize the dashboard_data_a method by refactoring the code for better performance. Specifically, I refactored the database queries and added caching for some frequently accessed data.

And also I created common function for filtering which is filter_and_group_leads() but this won't reduced my time complexity in this so I expected that these optimizations would lead to a significant reduction in response times, particularly for requests that retrieve data for the Dashboard-A. However, while there was a slight improvement, the response times still vary considerably, and I believe there's room for further enhancement.

Answer 1

Your coding style is too verbose.

Have you heard of Sandi Metz? She is elaborating on coding styles. She says e.g. a method has to have 5 rows max. So breaking down methods to smaller understandable chunks is somewhat the game.

Your set_old_and_new_country_tech_hash_A_and_B method has 98 rows which is much too crazy to look and identify any problem you are encountering especially performance issues. I would work on this again and edit your question. My controllers are half the size in total length of this including white lines. So making smaller methods will also help you to identify performance issues.

Helping you on performance on your code base is somewhat impossible in current state. In fact that is the longest controller action I ever saw in my life and I am working 15 years on Ruby On Rails!

Answer 2

The issue lies with the sheer amount of DB calls your code generates.

But more generally, you would begin the process by using a benchmarking tool, something that measures the actual time it takes to run the code. I tend to use 2 tools: Benchmark and RackMiniProfiler.

Then once you have a baseline for the current state of the code, you would then attempt to make incremental changes and ensure that those changes do in fact take less time in the benchmark.

And now to attempt to make some specific changes and recommendations for you.

1. Have you checked that each of the fields you are looking up are backed by database indexes?
2. I noticed you are not using any eager loading of the data, unless I am missing something. Have you attempted to fix any n+1 issues with eager loading?