Project

General

Profile

Actions

Defect #32985

closed

Remove unnecessary use of instance variables in CSV and Atom response handlers

Added by Ko Nagase over 4 years ago. Updated 9 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Code cleanup/refactoring
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

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

32985.patch (3.98 KB) 32985.patch Go MAEDA, 2024-01-20 08:27
Actions #1

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.

Actions #2

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.

Actions

Also available in: Atom PDF