Defect #32985
closedRemove unnecessary use of instance variables in CSV and Atom response handlers
0%
Description
I set "10000" as "Issues export limit" (instead of default "500") for some reasons,
but I noticed that exporting around 10,000 issues CSV causes out of memory issue.
I thought that the cause is inside some plugins, but it seems to be happen without any plugins,
and after changing the following Issues Controller part, the memory usage seems to become stable (no memory leak).
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 69a947b03..6596df30c 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -63,8 +63,8 @@ class IssuesController < ApplicationController
render_feed(@issues, :title => "#{@project || Setting.app_title}: #{l(:label_issue_plural)}")
}
format.csv {
- @issues = @query.issues(:limit => Setting.issues_export_limit.to_i)
- send_data(query_to_csv(@issues, @query, params[:csv]), :type => 'text/csv; header=present', :filename => 'issues.csv')
+ tmp_issues = @query.issues(:limit => Setting.issues_export_limit.to_i)
+ send_data(query_to_csv(tmp_issues, @query, params[:csv]), :type => 'text/csv; header=present', :filename => 'issues.csv')
}
format.pdf {
@issues = @query.issues(:limit => Setting.issues_export_limit.to_i)
Ruby process memory usage
Pattern | Initial | 1st | 2nd | 3rd |
Before modification | 126.4 MB | 1.20 GB | 1.92 GB | 973.4 MB |
After modification | 131.4 MB | 1.04GB | 776.3 MB | 803.5 MB |
I am not sure whether above modification is correct, so someone's review is quite helpful...
Here is my local reproducible environment:- Redmine: 4.0.5
- OS: macOS Mojave
- Ruby: 2.6.5
- DB: PostgreSQL 12
- Server: Puma
Thanks,
Files
Updated by Go MAEDA 9 months ago
- File 32985.patch 32985.patch added
- Subject changed from Garbage collection issue when exporting huge CSV to Remove unnecessary instance variable usage in CSV and Atom format handlers within respond_to blocks in controllers
- Category changed from Issues list to Code cleanup/refactoring
- Target version set to 6.0.0
The code you posted changes the variable from an instance variable to a local variable. I am not sure if the change has any impact on memory usage, but there is no need to use instance variables here.
I have changed the code to remove the use of unnecessary instance variables in other controllers as well.
Updated by Go MAEDA 9 months ago
- Subject changed from Remove unnecessary instance variable usage in CSV and Atom format handlers within respond_to blocks in controllers to Remove unnecessary use of instance variables in CSV and Atom response handlers
- Status changed from New to Closed
- Assignee set to Go MAEDA
- Resolution set to Fixed
Committed the patch. Thank you for your contribution.