Optimize tournament render hot path
This commit is contained in:
parent
798eb56657
commit
2d273f067c
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
Loading…
Reference in New Issue