From 0560b7a2ab87382cbbf60e62c76085eb686da284 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Tue, 20 Sep 2016 18:32:51 +0200 Subject: [PATCH 1/6] Add index on labels title --- .../20160920160832_add_index_to_labels_title.rb | 11 +++++++++++ db/schema.rb | 3 ++- 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20160920160832_add_index_to_labels_title.rb diff --git a/db/migrate/20160920160832_add_index_to_labels_title.rb b/db/migrate/20160920160832_add_index_to_labels_title.rb new file mode 100644 index 00000000000..b5de552b98c --- /dev/null +++ b/db/migrate/20160920160832_add_index_to_labels_title.rb @@ -0,0 +1,11 @@ +class AddIndexToLabelsTitle < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def change + add_concurrent_index :labels, :title + end +end diff --git a/db/schema.rb b/db/schema.rb index 59b3e237707..425fc33b7b3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160915042921) do +ActiveRecord::Schema.define(version: 20160920160832) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -521,6 +521,7 @@ ActiveRecord::Schema.define(version: 20160915042921) do add_index "labels", ["priority"], name: "index_labels_on_priority", using: :btree add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree + add_index "labels", ["title"], name: "index_labels_on_title", using: :btree create_table "lfs_objects", force: :cascade do |t| t.string "oid", null: false From ac3470280dec5c9af43d9fa0da2ca47295c28d9a Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Tue, 20 Sep 2016 20:59:17 +0200 Subject: [PATCH 2/6] Eager-load assignee and labels associations for GlobalMilestore issuables --- app/models/global_milestone.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb index da7c265a371..81b7343d7ad 100644 --- a/app/models/global_milestone.rb +++ b/app/models/global_milestone.rb @@ -53,11 +53,11 @@ class GlobalMilestone end def issues - @issues ||= Issue.of_milestones(milestones.map(&:id)).includes(:project) + @issues ||= Issue.of_milestones(milestones.map(&:id)).includes(:project, :assignee, :labels) end def merge_requests - @merge_requests ||= MergeRequest.of_milestones(milestones.map(&:id)).includes(:target_project) + @merge_requests ||= MergeRequest.of_milestones(milestones.map(&:id)).includes(:target_project, :assignee, :labels) end def participants From b8bfe50a5080909c7ba2b517d99d3c5f79d2a32f Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Tue, 20 Sep 2016 23:05:49 +0200 Subject: [PATCH 3/6] Reduce number of queries when calling GlobalMilestone#{labels,participants} --- CHANGELOG | 1 + app/models/global_milestone.rb | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 5fbc8830d7b..90092c3008e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -180,6 +180,7 @@ v 8.12.0 (unreleased) - Clean environment variables when running git hooks - Fix Import/Export issues importing protected branches and some specific models - Fix non-master branch readme display in tree view + - Speed-up group milestones show page - Add UX improvements for merge request version diffs v 8.11.7 diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb index 81b7343d7ad..8f38a2b6254 100644 --- a/app/models/global_milestone.rb +++ b/app/models/global_milestone.rb @@ -61,11 +61,11 @@ class GlobalMilestone end def participants - @participants ||= milestones.map(&:participants).flatten.compact.uniq + @participants ||= milestones_relation.includes(:participants).map(&:participants).flatten.compact.uniq end def labels - @labels ||= GlobalLabel.build_collection(milestones.map(&:labels).flatten) + @labels ||= GlobalLabel.build_collection(milestones_relation.includes(:labels).map(&:labels).flatten) .sort_by!(&:title) end @@ -89,4 +89,14 @@ class GlobalMilestone end end end + + private + + def milestones_relation + @milestones_relation ||= if milestones.is_a?(ActiveRecord::Relation) + milestones + else + Milestone.where(id: milestones.map(&:id)) + end + end end From b5132118cd43a3e005f0ed159d4a1be34b9a6ce6 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Fri, 23 Sep 2016 13:34:39 +0200 Subject: [PATCH 4/6] Ensure milestones passed to GlobalMilestone is an ActiveRecord::Relation --- app/models/global_milestone.rb | 17 ++++------------- spec/models/global_milestone_spec.rb | 5 +++-- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb index 8f38a2b6254..3e10bed19c2 100644 --- a/app/models/global_milestone.rb +++ b/app/models/global_milestone.rb @@ -8,7 +8,8 @@ class GlobalMilestone milestones = milestones.group_by(&:title) milestones.map do |title, milestones| - new(title, milestones) + milestones_relation = Milestone.where(id: milestones.map(&:id)) + new(title, milestones_relation) end end @@ -61,11 +62,11 @@ class GlobalMilestone end def participants - @participants ||= milestones_relation.includes(:participants).map(&:participants).flatten.compact.uniq + @participants ||= milestones.includes(:participants).map(&:participants).flatten.compact.uniq end def labels - @labels ||= GlobalLabel.build_collection(milestones_relation.includes(:labels).map(&:labels).flatten) + @labels ||= GlobalLabel.build_collection(milestones.includes(:labels).map(&:labels).flatten) .sort_by!(&:title) end @@ -89,14 +90,4 @@ class GlobalMilestone end end end - - private - - def milestones_relation - @milestones_relation ||= if milestones.is_a?(ActiveRecord::Relation) - milestones - else - Milestone.where(id: milestones.map(&:id)) - end - end end diff --git a/spec/models/global_milestone_spec.rb b/spec/models/global_milestone_spec.rb index 92e0f7f27ce..dd033480527 100644 --- a/spec/models/global_milestone_spec.rb +++ b/spec/models/global_milestone_spec.rb @@ -50,8 +50,9 @@ describe GlobalMilestone, models: true do milestone1_project2, milestone1_project3, ] + milestones_relation = Milestone.where(id: milestones.map(&:id)) - @global_milestone = GlobalMilestone.new(milestone1_project1.title, milestones) + @global_milestone = GlobalMilestone.new(milestone1_project1.title, milestones_relation) end it 'has exactly one group milestone' do @@ -67,7 +68,7 @@ describe GlobalMilestone, models: true do let(:milestone) { create(:milestone, title: "git / test", project: project1) } it 'strips out slashes and spaces' do - global_milestone = GlobalMilestone.new(milestone.title, [milestone]) + global_milestone = GlobalMilestone.new(milestone.title, Milestone.where(id: milestone.id)) expect(global_milestone.safe_title).to eq('git-test') end From 2161c72da01f919aef008c3eb997981374ca1977 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Fri, 23 Sep 2016 13:35:11 +0200 Subject: [PATCH 5/6] Use select(:foo) instead of map(&:foo) in GlobalMilestone --- app/models/global_milestone.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb index 3e10bed19c2..bda2b5c5d5d 100644 --- a/app/models/global_milestone.rb +++ b/app/models/global_milestone.rb @@ -32,7 +32,7 @@ class GlobalMilestone end def projects - @projects ||= Project.for_milestones(milestones.map(&:id)) + @projects ||= Project.for_milestones(milestones.select(:id)) end def state @@ -54,11 +54,11 @@ class GlobalMilestone end def issues - @issues ||= Issue.of_milestones(milestones.map(&:id)).includes(:project, :assignee, :labels) + @issues ||= Issue.of_milestones(milestones.select(:id)).includes(:project, :assignee, :labels) end def merge_requests - @merge_requests ||= MergeRequest.of_milestones(milestones.map(&:id)).includes(:target_project, :assignee, :labels) + @merge_requests ||= MergeRequest.of_milestones(milestones.select(:id)).includes(:target_project, :assignee, :labels) end def participants From 3753b847130264d6d862c613ef1f8e94fc47cd3e Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Fri, 23 Sep 2016 13:38:27 +0200 Subject: [PATCH 6/6] Update CHANGELOG --- CHANGELOG | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 90092c3008e..e3000968b50 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,8 @@ Please view this file on the master branch, on stable branches it's out of date. +v 8.13.0 (unreleased) + - Speed-up group milestones show page -v 8.12.0 (unreleased) +v 8.12.0 - Update the rouge gem to 2.0.6, which adds highlighting support for JSX, Prometheus, and others. !6251 - Only check :can_resolve permission if the note is resolvable - Bump fog-aws to v0.11.0 to support ap-south-1 region @@ -180,7 +182,6 @@ v 8.12.0 (unreleased) - Clean environment variables when running git hooks - Fix Import/Export issues importing protected branches and some specific models - Fix non-master branch readme display in tree view - - Speed-up group milestones show page - Add UX improvements for merge request version diffs v 8.11.7