Created
February 23, 2026 21:26
-
-
Save jbonhag/9ca9babf3dca74769854d2c6f6e937a8 to your computer and use it in GitHub Desktop.
ProjectsController#index performance optimizations
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| diff --git i/app/controllers/api/v2/projects_controller.rb w/app/controllers/api/v2/projects_controller.rb | |
| index a115f3ff925..c3873506539 100644 | |
| --- i/app/controllers/api/v2/projects_controller.rb | |
| +++ w/app/controllers/api/v2/projects_controller.rb | |
| @@ -39,9 +39,11 @@ module Api | |
| @projects = @projects.search(q) | |
| end | |
| - # Eager load effective tags because the serializer makes use of their existence | |
| - # It's also possible that they are sideloaded using the include query param | |
| - @projects = @projects.preload(:effective_tag_bindings => []) | |
| + # Eager load associations accessed by the serializer to prevent N+1 queries: | |
| + # - effective_tag_bindings: Used by has_effective_tag_bindings? and sideloaded | |
| + # - organization: Accessed by resolved_default_execution_mode | |
| + # - default_agent_pool: Accessed by resolved_default_agent_pool | |
| + @projects = @projects.preload(:effective_tag_bindings, :organization, :default_agent_pool) | |
| if tag_binding_filters_present? | |
| tag_binding_filters = extract_tag_binding_filters | |
| @@ -52,11 +54,17 @@ module Api | |
| status_counts["matching"] = @projects.length | |
| @projects = paginate(@projects) | |
| + | |
| + # Use pluck(:id) instead of map(&:id) to avoid loading full ActiveRecord objects | |
| + project_ids = @projects.pluck(:id) | |
| + | |
| + # Only fetch projects with non-zero counts; serializer defaults missing entries to 0 | |
| proj_with_workspace_count = | |
| - Project.left_joins(:workspaces) | |
| - .select('projects.id', 'COUNT(workspaces.id) FILTER (WHERE workspaces.discarded_at IS NULL) AS workspace_count') | |
| - .where(id: @projects.map(&:id)) | |
| + Project.joins(:workspaces) | |
| + .select('projects.id', 'COUNT(workspaces.id) AS workspace_count') | |
| + .where(id: project_ids, workspaces: { discarded_at: nil }) | |
| .group('projects.id') | |
| + .having('COUNT(workspaces.id) > 0') | |
| .map{ |p| [T.unsafe(p).id, T.unsafe(p).workspace_count]}.to_h | |
| if @organization.is_unified? | |
| @@ -65,19 +73,25 @@ module Api | |
| readable_teams = policy_scope(@organization.teams).pluck(:id) | |
| end | |
| - proj_with_team_count = Hash[@projects.map { |project| [project.id, 0] }] | |
| - TeamProject.where(project_id: @projects).distinct.pluck(:project_id, :team_id).each do |project_id, team_id| | |
| - if proj_with_team_count[project_id].nil? | |
| - proj_with_team_count[project_id] = 0 | |
| - end | |
| - proj_with_team_count[project_id] += 1 if readable_teams.include?(team_id) | |
| + # Only fetch projects with teams; serializer defaults missing entries to 0 | |
| + if readable_teams.any? | |
| + proj_with_team_count = TeamProject | |
| + .where(project_id: project_ids, team_id: readable_teams) | |
| + .group(:project_id) | |
| + .distinct | |
| + .count(:team_id) | |
| + .reject { |_id, count| count == 0 } | |
| + else | |
| + proj_with_team_count = {} | |
| end | |
| + # Only fetch projects with non-zero stack counts; serializer defaults missing entries to 0 | |
| proj_with_stack_count = | |
| - Project.left_joins(:stacks) | |
| - .select('projects.id', 'COUNT(stacks.id) FILTER (WHERE stacks.discarded_at IS NULL) AS stack_count') | |
| - .where(id: @projects.map(&:id)) | |
| + Project.joins(:stacks) | |
| + .select('projects.id', 'COUNT(stacks.id) AS stack_count') | |
| + .where(id: project_ids, stacks: { discarded_at: nil }) | |
| .group('projects.id') | |
| + .having('COUNT(stacks.id) > 0') | |
| .map{ |p| [T.unsafe(p).id, T.unsafe(p).stack_count]}.to_h | |
| render_object( | |
| diff --git i/app/serializers/api/v2/project_serializer.rb w/app/serializers/api/v2/project_serializer.rb | |
| index 3824acdb1b5..ef968146b37 100644 | |
| --- i/app/serializers/api/v2/project_serializer.rb | |
| +++ w/app/serializers/api/v2/project_serializer.rb | |
| @@ -15,11 +15,11 @@ module Api | |
| attribute :description | |
| attribute :created_at | |
| attribute :permissions, if: :include_permissions? | |
| - attribute :workspace_count, if: -> { @instance_options[:workspace_count_map].present? } | |
| - attribute :team_count, if: -> { @instance_options[:team_count_map].present? } | |
| + attribute :workspace_count, if: -> { @instance_options.key?(:workspace_count_map) } | |
| + attribute :team_count, if: -> { @instance_options.key?(:team_count_map) } | |
| attribute :hcp_id, if: :is_unified? | |
| attribute :is_unified, if: :is_unified? | |
| - attribute :stack_count, if: -> { @instance_options[:stack_count_map].present? } | |
| + attribute :stack_count, if: -> { @instance_options.key?(:stack_count_map) } | |
| attribute :auto_destroy_activity_duration | |
| attribute :default_execution_mode | |
| attribute :setting_overwrites | |
| @@ -75,23 +75,17 @@ module Api | |
| def workspace_count | |
| workspace_count_map = @instance_options[:workspace_count_map] | |
| - if workspace_count_map.present? | |
| - workspace_count_map[object.id] || 0 | |
| - end | |
| + workspace_count_map[object.id] || 0 | |
| end | |
| def team_count | |
| team_count_map = @instance_options[:team_count_map] | |
| - if team_count_map.present? | |
| - team_count_map[object.id] || 0 | |
| - end | |
| + team_count_map[object.id] || 0 | |
| end | |
| def stack_count | |
| stack_count_map = @instance_options[:stack_count_map] | |
| - if stack_count_map.present? | |
| - stack_count_map[object.id] || 0 | |
| - end | |
| + stack_count_map[object.id] || 0 | |
| end | |
| def id | |
| diff --git i/spec/controllers/api/v2/projects_controller_spec.rb w/spec/controllers/api/v2/projects_controller_spec.rb | |
| index 315da2d1db8..6a60a0c2c0b 100644 | |
| --- i/spec/controllers/api/v2/projects_controller_spec.rb | |
| +++ w/spec/controllers/api/v2/projects_controller_spec.rb | |
| @@ -20,6 +20,45 @@ RSpec.describe Api::V2::ProjectsController do | |
| expect { get :index, params: params }.to match_openapi_spec | |
| end | |
| + it "does not trigger N+1 queries for associations" do | |
| + # Create additional projects with various associations to test preloading | |
| + agent_pool = create(:agent_pool, organization: api_organization) | |
| + projects = create_list(:project, 3, organization: api_organization, default_agent_pool: agent_pool) | |
| + | |
| + # Create teams and team_projects to test team count optimization | |
| + team1 = create(:team, organization: api_organization) | |
| + team2 = create(:team, organization: api_organization) | |
| + create(:team_project, team: team1, project: projects[0]) | |
| + create(:team_project, team: team2, project: projects[1]) | |
| + | |
| + # Track queries after the initial request | |
| + get :index, params: params | |
| + | |
| + # Verify response is successful | |
| + expect(response).to have_http_status(:ok) | |
| + | |
| + # Now measure queries for a second request with same number of projects | |
| + queries = [] | |
| + query_counter = lambda do |_name, _start, _finish, _id, payload| | |
| + queries << payload[:sql] unless payload[:name] == "SCHEMA" | |
| + end | |
| + | |
| + ActiveSupport::Notifications.subscribed(query_counter, "sql.active_record") do | |
| + get :index, params: params | |
| + end | |
| + | |
| + # Verify no N+1 queries on these associations: | |
| + # - organization (accessed by resolved_default_execution_mode) | |
| + # - default_agent_pool (accessed by resolved_default_agent_pool) | |
| + # These should be preloaded and not trigger individual queries per project | |
| + organization_queries = queries.select { |q| q =~ /SELECT.*FROM.*organizations.*WHERE.*organizations.*id.*=/ } | |
| + agent_pool_queries = queries.select { |q| q =~ /SELECT.*FROM.*agent_pools.*WHERE.*agent_pools.*id.*IN/ } | |
| + | |
| + # Should have at most 1 query for organizations and 1 for agent_pools (batch loading) | |
| + expect(organization_queries.count).to be <= 1 | |
| + expect(agent_pool_queries.count).to be <= 1 | |
| + end | |
| + | |
| context "without workspaces, teams, or stacks" do | |
| let(:params) { | |
| { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment