Project

General

Profile

Actions

Patch #13301

closed

Performance: avoid querying all memberships in User#roles_for_project

Added by Jean-Baptiste Barth over 11 years ago. Updated over 11 years ago.

Status:
Closed
Priority:
Normal
Category:
Permissions and roles
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

Already discussed a bit in #11827

Problem

On my instance at work the average staff user has 150 private projects. User#roles_for_project retrieves all memberships then selects only the one needed for a specific project. This is not problematic when done for current user (for whom memberships need to be retrieved for the project jump box) but it can lead to substantial performance loss when applied to many other users, for instance when generating the list of recipients for email notifications.

Proposal

Replace app/models/user.rb line 428 :

membership = memberships.detect {|m| m.project_id == project.id}

with :
membership = memberships.where(:project_id => project.id).first

Results

On a very slow machine I noticed a 70% gain when generating this list for a specific issue (13s=>4s). On a decent server you will probably see a smaller gap. Some notes :
  • some 50 users on project or so, private project
  • takes ~50ms to retrieve all memberships per user, + object instantiation
  • take less than 5ms to retrieve only needed memberships per user
  • a global count shows 50 memberships for this project, 7000+ memberships in the worst case retrieved without the patch

Of course the whole test suite is green before and after the modification (trunk, redmine 1.9.3 on sqlite without scm tests, but no reason to doubt it'd be different with an other setup).

What's next?

First, let me know what you think. I think this patch won't change anything for little redmine installations but can be great for redmine instances with hundreds of projects and dozens of users per project.

Second, performance is hard to track in a complex app such as Redmine. This method is central for determining object visibility, and a simple test showed it's called multiple times for different projects on the same User instance in a few functional/integration test cases, so it's probably the case in real world usage. I'll patch manually my own instance at work and tell you in a few days the benefits and if it doesn't degrade performance dramatically elsewhere.

Third, object visibility code and permissions may deserve a bigger rewrite later, this change is just a quick win and a way to begin the discussion here if you want.


Related issues

Related to Redmine - Patch #11827: Avoid retrieving memberships for projects jump boxClosedJean-Baptiste Barth

Actions
Actions

Also available in: Atom PDF