Commit c0a6836b authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'view-issue-performance'

parents 4b28f2d9 97e02f4b
......@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 8.1.0 (unreleased)
- Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu)
- Speed up load times of issue detail pages by roughly 1.5x
- Make diff file view easier to use on mobile screens (Stan Hu)
- Add support for creating directories from Files page (Stan Hu)
- Allow removing of project without confirmation when JavaScript is disabled (Stan Hu)
......
......@@ -216,6 +216,9 @@ group :development do
gem 'quiet_assets', '~> 1.0.2'
gem 'rack-mini-profiler', '~> 0.9.0', require: false
gem 'rerun', '~> 0.10.0'
gem 'bullet', require: false
gem 'active_record_query_trace', require: false
gem 'rack-lineprof', platform: :mri
# Better errors handler
gem 'better_errors', '~> 1.0.1'
......
......@@ -17,6 +17,7 @@ GEM
activesupport (= 4.1.12)
builder (~> 3.1)
erubis (~> 2.7.0)
active_record_query_trace (1.5)
activemodel (4.1.12)
activesupport (= 4.1.12)
builder (~> 3.1)
......@@ -87,6 +88,9 @@ GEM
terminal-table (~> 1.4)
browser (1.0.0)
builder (3.2.2)
bullet (4.14.9)
activesupport (>= 3.0.0)
uniform_notifier (~> 1.9.0)
byebug (6.0.2)
cal-heatmap-rails (0.0.1)
capybara (2.4.4)
......@@ -134,6 +138,7 @@ GEM
daemons (1.2.3)
database_cleaner (1.4.1)
debug_inspector (0.0.2)
debugger-ruby_core_source (1.3.8)
default_value_for (3.0.1)
activerecord (>= 3.2.0, < 5.0)
descendants_tracker (0.0.4)
......@@ -484,6 +489,10 @@ GEM
rack-attack (4.3.0)
rack
rack-cors (0.4.0)
rack-lineprof (0.0.3)
rack (~> 1.5)
rblineprof (~> 0.3.6)
term-ansicolor (~> 1.3)
rack-mini-profiler (0.9.7)
rack (>= 1.1.3)
rack-mount (0.8.3)
......@@ -522,6 +531,8 @@ GEM
rb-fsevent (0.9.5)
rb-inotify (0.9.5)
ffi (>= 0.5.0)
rblineprof (0.3.6)
debugger-ruby_core_source (~> 1.3)
rbvmomi (1.8.2)
builder
nokogiri (>= 1.4.1)
......@@ -736,6 +747,7 @@ GEM
unicorn-worker-killer (0.4.3)
get_process_mem (~> 0)
unicorn (~> 4)
uniform_notifier (1.9.0)
uuid (2.3.8)
macaddr (~> 1.0)
version_sorter (2.0.0)
......@@ -765,6 +777,7 @@ PLATFORMS
DEPENDENCIES
RedCloth (~> 4.2.9)
ace-rails-ap (~> 2.0.1)
active_record_query_trace
activerecord-deprecated_finders (~> 1.0.3)
activerecord-session_store (~> 0.1.0)
acts-as-taggable-on (~> 3.4)
......@@ -781,6 +794,7 @@ DEPENDENCIES
bootstrap-sass (~> 3.0)
brakeman (= 3.0.1)
browser (~> 1.0.0)
bullet
byebug
cal-heatmap-rails (~> 0.0.1)
capybara (~> 2.4.0)
......@@ -861,6 +875,7 @@ DEPENDENCIES
quiet_assets (~> 1.0.2)
rack-attack (~> 4.3.0)
rack-cors (~> 0.4.0)
rack-lineprof
rack-mini-profiler (~> 0.9.0)
rack-oauth2 (~> 1.0.5)
rails (= 4.1.12)
......
......@@ -57,7 +57,7 @@ class Projects::IssuesController < Projects::ApplicationController
def show
@participants = @issue.participants(current_user)
@note = @project.notes.new(noteable: @issue)
@notes = @issue.notes.inc_author.fresh
@notes = @issue.notes.with_associations.fresh
@noteable = @issue
respond_with(@issue)
......
......@@ -68,13 +68,17 @@ module ApplicationHelper
end
end
def avatar_icon(user_email = '', size = nil)
user = User.find_by(email: user_email)
def avatar_icon(user_or_email = nil, size = nil)
if user_or_email.is_a?(User)
user = user_or_email
else
user = User.find_by(email: user_or_email)
end
if user
user.avatar_url(size) || default_avatar
else
gravatar_icon(user_email, size)
gravatar_icon(user_or_email, size)
end
end
......
......@@ -29,7 +29,7 @@ module ProjectsHelper
author_html = ""
# Build avatar image tag
author_html << image_tag(avatar_icon(author.try(:email), opts[:size]), width: opts[:size], class: "avatar avatar-inline #{"s#{opts[:size]}" if opts[:size]}", alt:'') if opts[:avatar]
author_html << image_tag(avatar_icon(author, opts[:size]), width: opts[:size], class: "avatar avatar-inline #{"s#{opts[:size]}" if opts[:size]}", alt:'') if opts[:avatar]
# Build name span tag
author_html << content_tag(:span, sanitize(author.name), class: opts[:author_class]) if opts[:name]
......
......@@ -47,7 +47,8 @@ module Issuable
prefix: true
attr_mentionable :title, :description
participant :author, :assignee, :notes, :mentioned_users
participant :author, :assignee, :notes_with_associations, :mentioned_users
end
module ClassMethods
......@@ -176,6 +177,10 @@ module Issuable
self.class.to_s.underscore
end
def notes_with_associations
notes.includes(:author, :project)
end
private
def filter_superceded_votes(votes, notes)
......
......@@ -64,7 +64,7 @@ class Group < Namespace
end
def owners
@owners ||= group_members.owners.map(&:user)
@owners ||= group_members.owners.includes(:user).map(&:user)
end
def add_users(user_ids, access_level, current_user = nil)
......
......@@ -60,6 +60,11 @@ class Note < ActiveRecord::Base
scope :inc_author_project, ->{ includes(:project, :author) }
scope :inc_author, ->{ includes(:author) }
scope :with_associations, -> do
includes(:author, :noteable, :updated_by,
project: [:project_members, { group: [:group_members] }])
end
serialize :st_diff
before_create :set_diff, if: ->(n) { n.line_code.present? }
......
......@@ -139,15 +139,28 @@ class ProjectTeam
Gitlab::Access.options.key max_member_access(user_id)
end
# This method assumes project and group members are eager loaded for optimal
# performance.
def max_member_access(user_id)
access = []
access << project.project_members.find_by(user_id: user_id).try(:access_field)
project.project_members.each do |member|
if member.user_id == user_id
access << member.access_field if member.access_field
break
end
end
if group
access << group.group_members.find_by(user_id: user_id).try(:access_field)
group.group_members.each do |member|
if member.user_id == user_id
access << member.access_field if member.access_field
break
end
end
end
access.compact.max
access.max
end
private
......
......@@ -8,7 +8,7 @@
= @user.name
%ul.well-list
%li
= image_tag avatar_icon(@user.email, 60), class: "avatar s60"
= image_tag avatar_icon(@user, 60), class: "avatar s60"
%li
%span.light Profile page:
%strong
......
......@@ -7,4 +7,4 @@
= link_to_gfm issue.title, [project.namespace.becomes(Namespace), project, issue], title: issue.title
.pull-right.assignee-icon
- if issue.assignee
= image_tag avatar_icon(issue.assignee.email, 16), class: "avatar s16"
= image_tag avatar_icon(issue.assignee, 16), class: "avatar s16"
......@@ -7,4 +7,4 @@
= link_to_gfm merge_request.title, [project.namespace.becomes(Namespace), project, merge_request], title: merge_request.title
.pull-right.assignee-icon
- if merge_request.assignee
= image_tag avatar_icon(merge_request.assignee.email, 16), class: "avatar s16"
= image_tag avatar_icon(merge_request.assignee, 16), class: "avatar s16"
......@@ -79,7 +79,7 @@
- @dashboard_milestone.participants.each do |user|
%li
= link_to user, title: user.name, class: "darken" do
= image_tag avatar_icon(user.email, 32), class: "avatar s32"
= image_tag avatar_icon(user, 32), class: "avatar s32"
%strong= truncate(user.name, lenght: 40)
%br
%small.cgray= user.username
......@@ -5,7 +5,7 @@
%li{class: "#{dom_class(member)} js-toggle-container", id: dom_id(member)}
%span{class: ("list-item-name" if show_controls)}
- if member.user
= image_tag avatar_icon(user.email, 16), class: "avatar s16", alt: ''
= image_tag avatar_icon(user, 16), class: "avatar s16", alt: ''
%strong
= link_to user.name, user_path(user)
%span.cgray= user.username
......
......@@ -7,4 +7,4 @@
= link_to_gfm issue.title, [project.namespace.becomes(Namespace), project, issue], title: issue.title
.pull-right.assignee-icon
- if issue.assignee
= image_tag avatar_icon(issue.assignee.email, 16), class: "avatar s16", alt: ''
= image_tag avatar_icon(issue.assignee, 16), class: "avatar s16", alt: ''
......@@ -7,4 +7,4 @@
= link_to_gfm merge_request.title, [project.namespace.becomes(Namespace), project, merge_request], title: merge_request.title
.pull-right.assignee-icon
- if merge_request.assignee
= image_tag avatar_icon(merge_request.assignee.email, 16), class: "avatar s16", alt: ''
= image_tag avatar_icon(merge_request.assignee, 16), class: "avatar s16", alt: ''
......@@ -87,7 +87,7 @@
- @group_milestone.participants.each do |user|
%li
= link_to user, title: user.name, class: "darken" do
= image_tag avatar_icon(user.email, 32), class: "avatar s32"
= image_tag avatar_icon(user, 32), class: "avatar s32"
%strong= truncate(user.name, lenght: 40)
%br
%small.cgray= user.username
......@@ -18,7 +18,7 @@
= render partial: 'layouts/collapse_button'
- if current_user
= link_to current_user, class: 'sidebar-user' do
= image_tag avatar_icon(current_user.email, 60), alt: 'User activity', class: 'avatar avatar s36'
= image_tag avatar_icon(current_user, 60), alt: 'User activity', class: 'avatar avatar s36'
.username
= current_user.username
.content-wrapper
......
......@@ -15,7 +15,7 @@
= render partial: 'layouts/collapse_button'
- if current_user
= link_to current_user, class: 'sidebar-user' do
= image_tag avatar_icon(current_user.email, 60), alt: 'User activity', class: 'avatar avatar s36'
= image_tag avatar_icon(current_user, 60), alt: 'User activity', class: 'avatar avatar s36'
.username
= current_user.username
.content-wrapper
......
......@@ -68,7 +68,7 @@
.col-md-5
.light-well
= image_tag avatar_icon(@user.email, 160), alt: '', class: 'avatar s160'
= image_tag avatar_icon(@user, 160), alt: '', class: 'avatar s160'
.clearfix
.profile-avatar-form-option
......
......@@ -2,10 +2,10 @@
.md-header.clearfix
%ul.center-top-menu
%li.active
= link_to '#md-write-holder', class: 'js-md-write-button', tabindex: '-1' do
%a.js-md-write-button(href="#md-write-holder" tabindex="-1")
Write
%li
= link_to '#md-preview-holder', class: 'js-md-preview-button', tabindex: '-1' do
%a.js-md-preview-button(href="md-preview-holder" tabindex="-1")
Preview
- if defined?(referenced_users) && referenced_users
......
.zennable
%input#zen-toggle-comment.zen-toggle-comment{ tabindex: '-1', type: 'checkbox' }
%input#zen-toggle-comment.zen-toggle-comment(tabindex="-1" type="checkbox")
.zen-backdrop
- classes << ' js-gfm-input markdown-area'
= f.text_area attr, class: classes, placeholder: ''
= link_to nil, class: 'zen-enter-link', tabindex: '-1' do
%a.zen-enter-link(tabindex="-1" href="#")
%i.fa.fa-expand
Edit in fullscreen
= link_to nil, class: 'zen-leave-link' do
%a.zen-leave-link(href="#")
%i.fa.fa-compress
%li{ id: dom_id(issue, 'sortable'), class: 'issue-row', 'data-iid' => issue.iid, 'data-url' => issue_path(issue) }
.pull-right.assignee-icon
- if issue.assignee
= image_tag avatar_icon(issue.assignee.email, 16), class: "avatar s16", alt: ''
= image_tag avatar_icon(issue.assignee, 16), class: "avatar s16", alt: ''
%span
= link_to [@project.namespace.becomes(Namespace), @project, issue] do
%span.cgray ##{issue.iid}
......
......@@ -5,4 +5,4 @@
= link_to_gfm merge_request.title, [@project.namespace.becomes(Namespace), @project, merge_request], title: merge_request.title
.pull-right.assignee-icon
- if merge_request.assignee
= image_tag avatar_icon(merge_request.assignee.email, 16), class: "avatar s16", alt: ''
= image_tag avatar_icon(merge_request.assignee, 16), class: "avatar s16", alt: ''
......@@ -104,7 +104,7 @@
- @users.each do |user|
%li
= link_to user, title: user.name, class: "darken" do
= image_tag avatar_icon(user.email, 32), class: "avatar s32"
= image_tag avatar_icon(user, 32), class: "avatar s32"
%strong= truncate(user.name, lenght: 40)
%br
%small.cgray= user.username
%li.timeline-entry{ id: dom_id(note), class: [dom_class(note), "note-row-#{note.id}", ('system-note' if note.system)], data: { discussion: note.discussion_id } }
.timeline-entry-inner
.timeline-icon
= link_to user_path(note.author) do
= image_tag avatar_icon(note.author_email), class: 'avatar s40', alt: ''
%a{href: user_path(note.author)}
%img.avatar.s40{src: avatar_icon(note.author), alt: ''}
.timeline-content
.note-header
- if note_editable?(note)
......@@ -25,7 +25,7 @@
= '@' + note.author.username
%span.note-last-update
= link_to "##{dom_id(note)}", name: dom_id(note), title: "Link here" do
%a{name: dom_id(note), href: "##{dom_id(note)}", title: 'Link here'}
= time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note_created_ago')
- if note.updated_at != note.created_at
%span
......
......@@ -4,7 +4,7 @@
%li{class: "#{dom_class(member)} js-toggle-container project_member_row access-#{member.human_access.downcase}", id: dom_id(member)}
%span.list-item-name
- if member.user
= image_tag avatar_icon(user.email, 16), class: "avatar s16", alt: ''
= image_tag avatar_icon(user, 16), class: "avatar s16", alt: ''
%strong
= link_to user.name, user_path(user)
%span.cgray= user.username
......
......@@ -9,8 +9,8 @@
.row
%section.col-md-7
.header-with-avatar
= link_to avatar_icon(@user.email, 400), target: '_blank' do
= image_tag avatar_icon(@user.email, 90), class: "avatar avatar-tile s90", alt: ''
= link_to avatar_icon(@user, 400), target: '_blank' do
= image_tag avatar_icon(@user, 90), class: "avatar avatar-tile s90", alt: ''
%h3
= @user.name
- if @user == current_user
......
if ENV['ENABLE_QUERY_TRACE']
require 'active_record_query_trace'
ActiveRecordQueryTrace.enabled = 'true'
end
if ENV['ENABLE_BULLET']
require 'bullet'
Bullet.enable = true
Bullet.console = true
end
# The default colors of rack-lineprof can be very hard to look at in terminals
# with darker backgrounds. This patch tweaks the colors a bit so the output is
# actually readable.
if Rails.env.development? and RUBY_ENGINE == 'ruby' and ENV['ENABLE_LINEPROF']
Gitlab::Application.config.middleware.use(Rack::Lineprof)
module Rack
class Lineprof
class Sample < Rack::Lineprof::Sample.superclass
def format(*)
formatted = if level == CONTEXT
sprintf " | % 3i %s", line, code
else
sprintf "% 8.1fms %5i | % 3i %s", ms, calls, line, code
end
case level
when CRITICAL
color.red formatted
when WARNING
color.yellow formatted
when NOMINAL
color.white formatted
else # CONTEXT
formatted
end
end
end
end
end
end
# Profiling
To make it easier to track down performance problems GitLab comes with a set of
profiling tools, some of these are available by default while others need to be
explicitly enabled.
## rack-mini-profiler
This Gem is enabled by default in development only. It allows you to see the
timings of the various components that made up a web request (e.g. the SQL
queries executed and their execution timings).
## Bullet
Bullet is a Gem that can be used to track down N+1 query problems. Because
Bullet adds quite a bit of logging noise it's disabled by default. To enable
Bullet, set the environment variable `ENABLE_BULLET` to a non-empty value before
starting GitLab. For example:
ENABLE_BULLET=true bundle exec rails s
Bullet will log query problems to both the Rails log as well as the Chrome
console.
## ActiveRecord Query Trace
This Gem adds backtraces for every ActiveRecord query in the Rails console. This
can be useful to track down where a query was executed. Because this Gem adds
quite a bit of noise (5-10 extra lines per ActiveRecord query) it's disabled by
default. To use this Gem you'll need to set `ENABLE_QUERY_TRACE` to a non empty
file before starting GitLab. For example:
ENABLE_QUERY_TRACE=true bundle exec rails s
## rack-lineprof
This is a Gem that can trace the execution time of code on a per line basis.
Because this Gem can add quite a bit of overhead it's disabled by default. To
enable it, set the environment variable `ENABLE_LINEPROF` to a non-empty value.
For example:
ENABLE_LINEPROF=true bundle exec rails s
Once enabled you'll need to add a query string parameter to a request to
actually profile code execution. The name of the parameter is `lineprof` and
should be set to a regular expression (minus the starting/ending slash) used to
select what files to profile. To profile all files containing "foo" somewhere in
the path you'd use the following parameter:
?lineprof=foo
Or when filtering for files containing "foo" and "bar" in their path:
?lineprof=foo|bar
Once set the profiling output will be displayed in your terminal.
require 'spec_helper'
describe ProjectTeam, benchmark: true do
describe '#max_member_access' do
let(:group) { create(:group) }
let(:project) { create(:empty_project, group: group) }
let(:user) { create(:user) }
before do
project.team << [user, :master]
5.times do
project.team << [create(:user), :reporter]
project.group.add_user(create(:user), :reporter)
end
end
benchmark_subject { project.team.max_member_access(user.id) }
it { is_expected.to iterate_per_second(35000) }
end
end
......@@ -99,6 +99,15 @@ describe ApplicationHelper do
helper.avatar_icon('foo@example.com', 20)
end
describe 'using a User' do
it 'should return an URL for the avatar' do
user = create(:user, avatar: File.open(avatar_file_path))
expect(helper.avatar_icon(user).to_s).
to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
end
end
end
describe 'gravatar_icon' do
......
......@@ -70,4 +70,18 @@ describe ProjectsHelper do
expect(helper.send(:readme_cache_key)).to eq("#{project.path_with_namespace}-nil-readme")
end
end
describe 'link_to_member' do
let(:group) { create(:group) }
let(:project) { create(:empty_project, group: group) }
let(:user) { create(:user) }
describe 'using the default options' do
it 'returns an HTML link to the user' do
link = helper.link_to_member(project, user)
expect(link).to match(%r{/u/#{user.username}})
end
end
end
end
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