Redmine

• before the patch, IssuesController#create average response time was 5.8s
• after applying the patch, average response time is 1.6s, yay :)
• nearly the same for IssuesController#update

For now no problem on other actions.

Jean-Baptiste Barth wrote:

On my production server:

• before the patch, IssuesController#create average response time was 5.8s
• after applying the patch, average response time is 1.6s, yay :)
• nearly the same for IssuesController#update

For now no problem on other actions.

• if you were sending asynchronous notifications you would'nt notice the bloat, would you?
• 1.6s is still slow :p

Etienne Massip wrote:

FWIW:

• if you were sending asynchronous notifications you would'nt notice the bloat, would you?

I already use async_smtp settings. My SMTP relay is near me in the datacenter, pretty fast, it doesn't change much anyway.

• 1.6s is still slow :p

Yep I know :/ There are many factors (mainly related to our redmine usage which is not the most "mainstream" one). I don't have the fastest stack too but there are good reasons behind (passenger, ruby 1.9, standard postgres on debian, etc.). Still digging into quick wins like this one ;-)

Jean-Baptiste Barth wrote:

Etienne Massip wrote:

FWIW:

• if you were sending asynchronous notifications you would'nt notice the bloat, would you?

I already use async_smtp settings. My SMTP relay is near me in the datacenter, pretty fast, it doesn't change much anyway.

True, my remark was stupid since the recipient list is created sync.

Rails 4 adds a pooled async mailer where the mail composition is async too. If we .dup every model object involved in the mail creation and pass them through the async mailer task pool it should work but I don't know how much pricy switching to this behavior can be.

I'm currently in the process of rewriting the whole notification process in a plugin so that I can add a lot more flexibility, exceptions, etc. Here's the plugin repo, I can show you some screenshots of the admin interface later if you want.

The only real problem to do everything asynchronously is the dirty attributes used in Mailer. Passing <Model>#id and a flat <Model>#changes hash everywhere is good enough I think. For the sake of simplicity I'll rely on the sucker_punch gem (a wrapper around Celluloid) for asynchronicity.

It generates lots of activerecord queries for non admin users. They hit the AR cache but still, I'd prefer doing something like this:

Index: app/models/user.rb
===================================================================
--- app/models/user.rb    (revision 11497)
+++ app/models/user.rb    (working copy)
@@ -132,6 +132,7 @@
   def reload(*args)
     @name = nil
     @projects_by_role = nil
+    @membership_by_project_id = nil
     base_reload(*args)
   end

@@ -419,6 +420,12 @@
     !logged?
   end

+  def membership_by_project_id
+    @membership_by_project_id ||= Hash.new {|h, project_id|
+      h[project_id] = memberships.where(:project_id => project_id).first
+    }
+  end
+
   # Return user's roles for project
   def roles_for_project(project)
     roles = []
@@ -426,7 +433,7 @@
     return roles if project.nil? || project.archived?
     if logged?
       # Find project membership
-      membership = memberships.detect {|m| m.project_id == project.id}
+      membership = membership_by_project_id[project.id]
       if membership
         roles = membership.roles
       else

You're basically substituting a hash cache to the AR cache, I'm curious to know the performance gain for Jean-Baptiste but if the AR cache is good enough it should not be much?

With AR cache on:

Benchmark.ms { 200.times {user.memberships.where(:project_id => 1).first }}
=> 452.026 ms

Benchmark.ms { 200.times {user.membership_by_project_id[1] }}
=> 2.0 ms

Any questions?

No.

Nice catch, it's even better on my test platform with your version Jean-Philippe (+30-40% faster).