From 2d273f067cb8178f0d764691172270cfdaee22e4 Mon Sep 17 00:00:00 2001 From: Malaber Date: Wed, 15 Apr 2026 09:36:56 +0200 Subject: [PATCH] Optimize tournament render hot path --- app/controllers/tournaments_controller.rb | 29 ++++++- app/models/group_score.rb | 28 +++++-- app/models/match.rb | 17 ++++- app/models/tournament.rb | 2 +- app/services/group_stage_service.rb | 75 ++++++++++++------- app/services/request_profiling.rb | 43 +++++++++++ spec/models/group_score_spec.rb | 32 ++++---- .../tournament_rendering_performance_spec.rb | 37 +++++++++ spec/services/group_stage_service_spec.rb | 8 ++ spec/support/performance_helpers.rb | 55 ++++++++++++++ 10 files changed, 272 insertions(+), 54 deletions(-) create mode 100644 app/services/request_profiling.rb create mode 100644 spec/requests/tournament_rendering_performance_spec.rb create mode 100644 spec/support/performance_helpers.rb diff --git a/app/controllers/tournaments_controller.rb b/app/controllers/tournaments_controller.rb index 85d1eb3..2abde14 100644 --- a/app/controllers/tournaments_controller.rb +++ b/app/controllers/tournaments_controller.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true class TournamentsController < ApplicationController - before_action :set_tournament, only: %i[show update destroy set_timer_end timer_end] + before_action :set_tournament_for_show, only: %i[show] + before_action :set_tournament, only: %i[update destroy set_timer_end timer_end] before_action :authenticate_user!, only: %i[create update destroy set_timer_end] before_action -> { require_owner! @tournament.owner }, only: %i[update destroy set_timer_end] before_action :validate_create_params, only: %i[create] @@ -30,7 +31,11 @@ class TournamentsController < ApplicationController if show_params.fetch(:simple, 'false') == 'true' render json: @tournament, serializer: SimpleTournamentSerializer else - render json: @tournament, include: '**' + rendered_json = @request_profiling.measure('serialize_tournament') do + ActiveModelSerializers::SerializableResource.new(@tournament, include: '**').as_json + end + @request_profiling.apply_to(response, label: 'tournament.show') + render json: rendered_json end end @@ -137,12 +142,30 @@ class TournamentsController < ApplicationController @tournament = Tournament.find(params[:id]) end + def set_tournament_for_show + profiling = RequestProfiling.new(enabled: show_params.fetch(:profile, 'false') == 'true') + @tournament = profiling.measure('load_tournament') do + Tournament.includes( + :user, + :teams, + stages: [ + { matches: { match_scores: :team } }, + { groups: [ + { matches: { match_scores: :team } }, + { group_scores: :team } + ] } + ] + ).find(params[:id]) + end + @request_profiling = profiling + end + def index_params params.permit(:type) end def show_params - params.permit(:simple) + params.permit(:simple, :profile) end def tournament_params diff --git a/app/models/group_score.rb b/app/models/group_score.rb index 325efa5..0d71e46 100644 --- a/app/models/group_score.rb +++ b/app/models/group_score.rb @@ -114,10 +114,28 @@ class GroupScore < ApplicationRecord end def comparison_match_against(other, decider:) - group.matches.reload - .select do |match| - match.decider? == decider && match.teams.include?(team) && match.teams.include?(other.team) - end - .max_by(&:position) + comparison_matches_index[[decider, *[team.id, other.team.id].sort]] + end + + def comparison_matches_index + cache = group.instance_variable_get(:@comparison_matches_index) + return cache unless cache.nil? + + matches = if group.association(:matches).loaded? && group.matches.any? + group.matches + else + group.matches.includes(match_scores: :team).reload + end + + cache = matches.each_with_object({}) do |match, index| + team_ids = match.teams.map(&:id).sort + next unless team_ids.size == 2 + + key = [match.decider?, *team_ids] + existing_match = index[key] + index[key] = match if existing_match.nil? || match.position.to_i > existing_match.position.to_i + end + + group.instance_variable_set(:@comparison_matches_index, cache) end end diff --git a/app/models/match.rb b/app/models/match.rb index 9500c65..3056f60 100644 --- a/app/models/match.rb +++ b/app/models/match.rb @@ -42,13 +42,15 @@ class Match < ApplicationRecord def scored_points_of(team) return 0 if decider? - teams.include?(team) ? match_scores.find_by(team: team).points : 0 + match_score = match_score_for(team) + match_score ? match_score.points : 0 end def received_points_of(team) return 0 if decider? - teams.include?(team) ? match_scores.find { |ms| ms.team != team }.points : 0 + opponent_match_score = opponent_match_score_for(team) + opponent_match_score ? opponent_match_score.points : 0 end def group_points_of(team) @@ -66,11 +68,20 @@ class Match < ApplicationRecord end def hidden_points_of(team) - teams.include?(team) ? match_scores.find_by(team: team).hidden_points : 0 + match_score = match_score_for(team) + match_score ? match_score.hidden_points : 0 end private + def match_score_for(team) + match_scores.find { |match_score| match_score.team_id == team.id } + end + + def opponent_match_score_for(team) + match_scores.find { |match_score| match_score.team_id != team.id } + end + def stage_xor_group errors.add(:stage_xor_group, 'Stage and Group missing or both present') unless stage.present? ^ group.present? end diff --git a/app/models/tournament.rb b/app/models/tournament.rb index 1f02015..52c7987 100644 --- a/app/models/tournament.rb +++ b/app/models/tournament.rb @@ -29,7 +29,7 @@ class Tournament < ApplicationRecord def group_stage # get the stage with level -1 (group stage) - stages.find_by(level: -1) + stages.find { |stage| stage.level == -1 } end private diff --git a/app/services/group_stage_service.rb b/app/services/group_stage_service.rb index 735fd4d..5297eb2 100644 --- a/app/services/group_stage_service.rb +++ b/app/services/group_stage_service.rb @@ -43,7 +43,7 @@ class GroupStageService # # This problem is only fixed for a group size of 4, because we did not come up with a generalized version of this # switcharoo magic and we needed it for groups of 4. - return unless team_size == 4 + return matches unless team_size == 4 matches[5].position, matches[1].position = matches[1].position, matches[5].position matches @@ -97,37 +97,15 @@ class GroupStageService end def ranking_decision_for(group_score) - sorted_group_scores = group_score.group.group_scores.reload.sort - index = sorted_group_scores.index(group_score) - return nil if index.nil? || sorted_group_scores.size <= 1 - - comparison_index = comparison_partner_index(sorted_group_scores, index) - return nil if comparison_index.nil? - - compared_group_score = sorted_group_scores[comparison_index] - comparison_reason = group_score.comparison_reason_against(compared_group_score) - - { - resolved_by: comparison_reason[:resolved_by].to_s, - compared_with_team_id: compared_group_score.team.id, - compared_with_team_name: compared_group_score.team.name, - direct_comparison_result: comparison_reason[:direct_comparison_result].to_s, - direct_comparison_match_id: comparison_reason[:direct_comparison_match_id], - hidden_points_result: comparison_reason[:hidden_points_result].to_s, - decider_match_id: comparison_reason[:decider_match_id], - tied: comparison_reason[:tied], - needs_decider_match: comparison_reason[:tied] && - unresolved_tie_crosses_advancing_cutoff?(sorted_group_scores, group_score, advancing_slots_for_group(group_score.group)) - } + ranking_decisions_for_group(group_score.group)[group_score_cache_key(group_score)] end def blocking_ties_for(group_stage) return [] if group_stage.nil? group_stage.groups.flat_map do |group| - sorted_scores = group.group_scores.reload.sort - sorted_scores.filter_map do |group_score| - decision = ranking_decision_for(group_score) + ranking_decisions_for_group(group).filter_map do |group_score_id, decision| + group_score = group_scores_for(group).find { |score| group_score_cache_key(score) == group_score_id } next unless decision&.fetch(:needs_decider_match, false) comparison_team_id = decision[:compared_with_team_id] @@ -286,5 +264,50 @@ class GroupStageService base_slots + (group_index < extra_slot_groups ? 1 : 0) end end + + def ranking_decisions_for_group(group) + cache = group.instance_variable_get(:@ranking_decisions_cache) + return cache unless cache.nil? + + sorted_group_scores = group_scores_for(group).sort + advancing_slots = advancing_slots_for_group(group) + + cache = sorted_group_scores.each_with_index.each_with_object({}) do |(group_score, index), decisions| + next if sorted_group_scores.size <= 1 + + comparison_index = comparison_partner_index(sorted_group_scores, index) + next if comparison_index.nil? + + compared_group_score = sorted_group_scores[comparison_index] + comparison_reason = group_score.comparison_reason_against(compared_group_score) + + decisions[group_score_cache_key(group_score)] = { + resolved_by: comparison_reason[:resolved_by].to_s, + compared_with_team_id: compared_group_score.team.id, + compared_with_team_name: compared_group_score.team.name, + direct_comparison_result: comparison_reason[:direct_comparison_result].to_s, + direct_comparison_match_id: comparison_reason[:direct_comparison_match_id], + hidden_points_result: comparison_reason[:hidden_points_result].to_s, + decider_match_id: comparison_reason[:decider_match_id], + tied: comparison_reason[:tied], + needs_decider_match: comparison_reason[:tied] && + unresolved_tie_crosses_advancing_cutoff?(sorted_group_scores, group_score, advancing_slots) + } + end + + group.instance_variable_set(:@ranking_decisions_cache, cache) + end + + def group_scores_for(group) + if group.association(:group_scores).loaded? && group.group_scores.any? + group.group_scores + else + group.group_scores.includes(:team).reload + end + end + + def group_score_cache_key(group_score) + group_score.id || group_score.object_id + end end end diff --git a/app/services/request_profiling.rb b/app/services/request_profiling.rb new file mode 100644 index 0000000..6367d57 --- /dev/null +++ b/app/services/request_profiling.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +class RequestProfiling + IGNORED_SQL_NAMES = %w[SCHEMA CACHE].freeze + IGNORED_SQL = /\A(?:BEGIN|COMMIT|ROLLBACK|SAVEPOINT|RELEASE)/i + + def initialize(enabled: false) + @enabled = enabled + @measurements = [] + end + + def measure(name) + return yield unless @enabled + + query_count = 0 + subscriber = lambda do |_event_name, _start, _finish, _id, payload| + sql = payload[:sql].to_s + next if IGNORED_SQL_NAMES.include?(payload[:name]) + next if sql.match?(IGNORED_SQL) + + query_count += 1 + end + + started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) + result = nil + ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') do + result = yield + end + duration = (Process.clock_gettime(Process::CLOCK_MONOTONIC) - started_at) * 1000.0 + @measurements << { name: name, duration: duration.round(1), query_count: query_count } + result + end + + def apply_to(response, label:) + return unless @enabled + + response.set_header('Server-Timing', @measurements.map { |m| "#{m[:name]};dur=#{m[:duration]}" }.join(', ')) + response.set_header( + 'X-Turniere-Profile', + "#{label}: #{@measurements.map { |m| "#{m[:name]}=#{m[:duration]}ms/#{m[:query_count]}sql" }.join(', ')}" + ) + end +end diff --git a/spec/models/group_score_spec.rb b/spec/models/group_score_spec.rb index 2e4e184..a2c61aa 100644 --- a/spec/models/group_score_spec.rb +++ b/spec/models/group_score_spec.rb @@ -78,9 +78,9 @@ RSpec.describe GroupScore, type: :model do it 'reports the direct comparison result when head-to-head resolves the tie' do group_score_b.update!(group_points: 6) - create(:group_match, group: group, state: :finished).tap do |match| - create(:match_score, match: match, team: team_a, points: 3) - create(:match_score, match: match, team: team_b, points: 1) + match = create(:group_match, group: group, state: :finished).tap do |created_match| + create(:match_score, match: created_match, team: team_a, points: 3) + create(:match_score, match: created_match, team: team_b, points: 1) end expect(group_score_a.comparison_reason_against(group_score_b)).to eq( @@ -88,16 +88,16 @@ RSpec.describe GroupScore, type: :model do tied: false, direct_comparison_result: :won, hidden_points_result: :not_played, - direct_comparison_match_id: group.matches.last.id, + direct_comparison_match_id: match.id, decider_match_id: nil ) end it 'reports an unresolved tie when head-to-head was a draw' do group_score_b.update!(group_points: 6) - create(:group_match, group: group, state: :finished).tap do |match| - create(:match_score, match: match, team: team_a, points: 2) - create(:match_score, match: match, team: team_b, points: 2) + match = create(:group_match, group: group, state: :finished).tap do |created_match| + create(:match_score, match: created_match, team: team_a, points: 2) + create(:match_score, match: created_match, team: team_b, points: 2) end expect(group_score_a.comparison_reason_against(group_score_b)).to eq( @@ -105,20 +105,20 @@ RSpec.describe GroupScore, type: :model do tied: true, direct_comparison_result: :draw, hidden_points_result: :not_played, - direct_comparison_match_id: group.matches.last.id, + direct_comparison_match_id: match.id, decider_match_id: nil ) end it 'falls back to hidden points when direct comparison is still tied' do group_score_b.update!(group_points: 6) - create(:group_match, group: group, state: :finished).tap do |match| - create(:match_score, match: match, team: team_a, points: 2) - create(:match_score, match: match, team: team_b, points: 2) + direct_match = create(:group_match, group: group, state: :finished).tap do |created_match| + create(:match_score, match: created_match, team: team_a, points: 2) + create(:match_score, match: created_match, team: team_b, points: 2) end - create(:group_match, group: group, state: :finished, decider: true).tap do |match| - create(:match_score, match: match, team: team_a, hidden_points: 3) - create(:match_score, match: match, team: team_b, hidden_points: 1) + decider_match = create(:group_match, group: group, state: :finished, decider: true).tap do |created_match| + create(:match_score, match: created_match, team: team_a, hidden_points: 3) + create(:match_score, match: created_match, team: team_b, hidden_points: 1) end expect(group_score_a.comparison_reason_against(group_score_b)).to eq( @@ -126,8 +126,8 @@ RSpec.describe GroupScore, type: :model do tied: false, direct_comparison_result: :draw, hidden_points_result: :won, - direct_comparison_match_id: group.matches.reject(&:decider?).last.id, - decider_match_id: group.matches.select(&:decider?).last.id + direct_comparison_match_id: direct_match.id, + decider_match_id: decider_match.id ) end end diff --git a/spec/requests/tournament_rendering_performance_spec.rb b/spec/requests/tournament_rendering_performance_spec.rb new file mode 100644 index 0000000..a38c27e --- /dev/null +++ b/spec/requests/tournament_rendering_performance_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'rails_helper' +require_relative '../support/performance_helpers' + +RSpec.describe 'Tournament rendering performance', type: :request do + [ + { groups_count: 8, teams_per_group: 4, max_duration_ms: 1000 }, + { groups_count: 64, teams_per_group: 6, max_duration_ms: 5000 }, + { groups_count: 4, teams_per_group: 32, max_duration_ms: 5000 } + ].each do |scenario| + it "renders #{scenario[:groups_count]} groups with #{scenario[:teams_per_group]} teams within budget" do + tournament = create_group_stage_tournament( + groups_count: scenario[:groups_count], + teams_per_group: scenario[:teams_per_group] + ) + + get "/tournaments/#{tournament.id}" + expect(response).to have_http_status(:ok) + + performance = capture_runtime_and_queries do + get "/tournaments/#{tournament.id}", params: { profile: 'true' } + end + + expect(response).to have_http_status(:ok) + expect(response.headers['Server-Timing']).to include('load_tournament') + expect(response.headers['Server-Timing']).to include('serialize_tournament') + expect(response.headers['X-Turniere-Profile']).to include('tournament.show') + expect(response.headers['X-Turniere-Profile']).to include('sql') + + body = response.parsed_body + expect(body.fetch('stages').first.fetch('groups').size).to eq(scenario[:groups_count]) + expect(body.fetch('teams').size).to eq(scenario[:groups_count] * scenario[:teams_per_group]) + expect(performance[:duration_ms]).to be <= scenario[:max_duration_ms] + end + end +end diff --git a/spec/services/group_stage_service_spec.rb b/spec/services/group_stage_service_spec.rb index 1cf60fb..0d7a00f 100644 --- a/spec/services/group_stage_service_spec.rb +++ b/spec/services/group_stage_service_spec.rb @@ -89,6 +89,14 @@ RSpec.describe GroupStageService do expect(match.teams.first).to_not eq(match.teams.second) end end + + it 'also generates matches for group sizes other than four' do + teams = create_list(:team, 6) + + matches = GroupStageService.generate_all_matches_between(teams) + + expect(matches.size).to eq(15) + end end describe '#update_group_scores' do diff --git a/spec/support/performance_helpers.rb b/spec/support/performance_helpers.rb new file mode 100644 index 0000000..44b22a3 --- /dev/null +++ b/spec/support/performance_helpers.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module PerformanceHelpers + IGNORED_SQL_NAMES = %w[SCHEMA CACHE].freeze + IGNORED_SQL = /\A(?:BEGIN|COMMIT|ROLLBACK|SAVEPOINT|RELEASE)/i + + def capture_runtime_and_queries + query_count = 0 + subscriber = lambda do |_name, _start, _finish, _id, payload| + sql = payload[:sql].to_s + next if IGNORED_SQL_NAMES.include?(payload[:name]) + next if sql.match?(IGNORED_SQL) + + query_count += 1 + end + + started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) + result = nil + + ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') do + result = yield + end + + duration_ms = (Process.clock_gettime(Process::CLOCK_MONOTONIC) - started_at) * 1000.0 + { result: result, query_count: query_count, duration_ms: duration_ms } + end + + def create_group_stage_tournament(groups_count:, teams_per_group:) + owner = create(:user) + tournament = owner.tournaments.create!( + name: "Perf #{groups_count}x#{teams_per_group}", + description: 'Tournament render performance fixture', + public: true, + playoff_teams_amount: groups_count + ) + + groups = Array.new(groups_count) do |group_index| + Array.new(teams_per_group) do |team_index| + tournament.teams.create!(name: "G#{group_index + 1}-T#{team_index + 1}") + end + end + + group_stage = GroupStageService.generate_group_stage(groups) + tournament.stages = [group_stage] + tournament.instant_finalists_amount, tournament.intermediate_round_participants_amount = + TournamentService.calculate_default_amount_of_teams_advancing(tournament.playoff_teams_amount, groups_count) + tournament.save! + + tournament.reload + end +end + +RSpec.configure do |config| + config.include PerformanceHelpers +end