diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2015-10-14 14:54:04 +0200 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2015-10-15 12:05:01 +0200 |
commit | 3025b711419fd1813baea5e2a2925c6ccf8d455a (patch) | |
tree | e915fa74473477b5942e8a48b1fac274b00b2a9f | |
parent | b5f8161daeefeaa66e810e9dddec43959333d8a7 (diff) | |
download | gitlab-ce-3025b711419fd1813baea5e2a2925c6ccf8d455a.tar.gz |
Improve ProjectTeam#max_member_access performance
By comparing objects in Ruby we can greatly improve the performance of
this method. In the worst case (should no data be eager loaded) this
will run the same amount of queries as before, in the best case (when
data _is_ eager loadeD) it requires no queries at all.
The added benchmark used to produce around 273 iterations per second.
With this commit this has been increased to almost 40 000 iterations per
second: a speedup of roughly 145 times.
Combined with eager loading Note associations this results in about 30
queries less when viewing a single issue, this in turn cuts down the
loading time by 30-40%.
-rw-r--r-- | app/models/project_team.rb | 19 | ||||
-rw-r--r-- | spec/benchmarks/models/project_team_spec.rb | 23 |
2 files changed, 39 insertions, 3 deletions
diff --git a/app/models/project_team.rb b/app/models/project_team.rb index f602a965364..9f380a382cb 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -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 diff --git a/spec/benchmarks/models/project_team_spec.rb b/spec/benchmarks/models/project_team_spec.rb new file mode 100644 index 00000000000..8b039ef7317 --- /dev/null +++ b/spec/benchmarks/models/project_team_spec.rb @@ -0,0 +1,23 @@ +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 |