Vote #77634
完了Issue#editable_custom_field_values very slow for issues with many custom fields
0%
説明
I have several issues with around 50 custom fields. When doing updates to these issues, I identified poor performance on the @editable_custom_field_values@ function being called to check if current user has access to the custom fields.
Here is the code for this method (in @app/model/issue.rb@):
# Returns the custom_field_values that can be edited by the given user
def editable_custom_field_values(user=nil)
visible_custom_field_values(user).reject do |value|
read_only_attribute_names(user).include?(value.custom_field_id.to_s)
end
end
Here, it seems that @read_only_attribute_names(user)@ is recomputed for each @visible_custom_field_values(user)@ without caching, which will slow down the action if the issue has many custom fields associated (only if user is provided).
I have applied a simple patch (I store the result of @read_only_attribute_names(user)@ before the loop):
def editable_custom_field_values(user=nil)
read_only_attr_names_array = read_only_attribute_names(user)
visible_custom_field_values(user).reject do |value|
read_only_attr_names_array.include?(value.custom_field_id.to_s)
end
end
Now, some results computed for an admin user, for approx 100 issues with various custom fields:
Before:
2.2.2 :001 > puts Benchmark.measure { u = User.find(88); Issue.last(100).to_a.map{|i| i.editable_custom_field_values(u)} } 2.2.2 :002 > 102.260000 0.710000 102.970000 (103.480136)
After:
2.2.2 :001 > puts Benchmark.measure { u = User.find(88); Issue.last(100).to_a.map{|i| i.editable_custom_field_values(u)} } 2.2.2 :002 > 5.070000 0.090000 5.160000 ( 5.198187)
Environment: Redmine version 3.3.0.stable Ruby version 2.2.2-p95 (2015-04-13) [x86_64-linux] Rails version 4.2.6 Environment development Database adapter Mysql2
journals
Good, I will try this.
--------------------------------------------------------------------------------
Victor Campos wrote:
> Good, I will try this.
It works here, thx =)
--------------------------------------------------------------------------------
Looks good to me. Caching a value is reasonable. Setting target version to 3.3.1.
Here is a diff made from Stephane Evr's post.
<pre><code class="diff">
Index: app/models/issue.rb
===================================================================
--- app/models/issue.rb (revision 15663)
+++ app/models/issue.rb (working copy)
@@ -574,8 +574,9 @@
# Returns the custom_field_values that can be edited by the given user
def editable_custom_field_values(user=nil)
+ read_only_attr_names_array = read_only_attribute_names(user)
visible_custom_field_values(user).reject do |value|
- read_only_attribute_names(user).include?(value.custom_field_id.to_s)
+ read_only_attr_names_array.include?(value.custom_field_id.to_s)
end
end
</code></pre>
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
Change committed in r15742, thanks.
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------