Commit a79fdb0c authored by Alexandru Croitor's avatar Alexandru Croitor

Performance improvements on milestone burndown chart

Don't build up events array when checking if burndown chart
is empty or accurate. Preload issue project to avoid N+1 calls.
parent 41537027
---
title: Performance improvements on milestone burndown chart
merge_request: 22380
author:
type: performance
......@@ -4,7 +4,6 @@ class Burndown
include Gitlab::Utils::StrongMemoize
attr_reader :issues, :start_date, :end_date, :due_date, :accurate
alias_method :accurate?, :accurate
def initialize(issues, start_date, due_date)
@start_date = start_date
......@@ -16,7 +15,6 @@ class Burndown
end
@issues = filter_issues_created_before(@end_date, issues)
@accurate = true
end
# Returns an array of milestone issue event data in the following format:
......@@ -28,7 +26,7 @@ class Burndown
end
def empty?
burndown_events.any? && legacy_data?
issues.any? && legacy_data?
end
def valid?
......@@ -38,13 +36,27 @@ class Burndown
# If all closed issues have no closed events, mark burndown chart as containing legacy data
def legacy_data?
strong_memoize(:legacy_data) do
closed_events = issues.select(&:closed?)
closed_events.any? && !Event.closed.where(target: closed_events, action: Event::CLOSED).exists?
closed_events = closed_issues
closed_events.any? && closed_issues_events_count == 0
end
end
def accurate?
closed_issues.count == closed_issues_events_count
end
private
def closed_issues
issues.select(&:closed?)
end
def closed_issues_events_count
strong_memoize(:closed_issues_events_count) do
Event.closed.where(target: closed_issues).count
end
end
def burndown_events
issues
.map { |issue| burndown_events_for(issue) }
......@@ -101,9 +113,6 @@ class Burndown
return [] unless issue.closed?
return [] if milestone_events_per_issue[issue.id]&.any?(&:closed_action?)
# Mark burndown chart as inaccurate
@accurate = false
build_burndown_event(start_date.beginning_of_day, issue.weight, 'closed')
end
......@@ -114,6 +123,6 @@ class Burndown
def filter_issues_created_before(date, issues)
return [] unless valid?
issues.where('issues.created_at <= ?', date.end_of_day)
issues.where('issues.created_at <= ?', date.end_of_day).includes(:project)
end
end
......@@ -209,6 +209,33 @@ describe Burndown do
end
end
describe 'load burndown events' do
let(:project) { create(:project) }
let(:milestone) { create(:milestone, project: project, start_date: start_date, due_date: due_date) }
subject { described_class.new(milestone.issues_visible_to_user(user), milestone.start_date, milestone.due_date).as_json }
before do
project.add_developer(user)
end
it 'avoids N+1 database queries' do
Timecop.freeze(milestone.due_date) do
create(:issue, milestone: milestone, weight: 2, project: project, author: user)
control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do
subject
end.count
create_list(:issue, 3, milestone: milestone, weight: 2, project: project, author: user)
expect do
subject
end.not_to exceed_all_query_limit(control_count)
end
end
end
def build_sample(milestone, issue_params)
project.add_master(user)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment