Project

General

Profile

Actions

Patch #39835

closed

Optimize repository menu visibility check

Added by Go MAEDA 10 months ago. Updated 9 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Performance
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

This patch introduces an optimization in the way Redmine determines the visibility of the "Repository" menu item.

While the current code loads the entire collection of repositories associated with the project, the new code only checks if there are any records in the repositories table associated with the project's ID. This can be faster if the project has a lot of repositories.

SQL of the current code:

 Repository Load (0.0ms)  SELECT "repositories".* FROM "repositories" WHERE "repositories"."project_id" = ?  [["project_id", 1]]

SQL of the new code:

  Repository Exists? (0.0ms)  SELECT 1 AS one FROM "repositories" WHERE "repositories"."project_id" = ? LIMIT ?  [["project_id", 1], ["LIMIT", 1]]


Files

optimize-repository-menu.patch (666 Bytes) optimize-repository-menu.patch Go MAEDA, 2023-12-12 15:19
Actions #2

Updated by Holger Just 10 months ago

You probably could also use the equivalent ActiveRecord functionality, i.e. ActiveRecord::FinderMethods#exists?, which always performs an SQL query. Thus, the following condition should also work:

:if => Proc.new {|p| p.repositories.exists?}

As we perform a SQL query here, the previous check for new records should also be solved nicely, as those new records are naturally not present in the database and thus do not affect the result of the SQL query.

The generated SQL query is exactly the same as the one produced by your patch - just with the slight semantic change that with exists? we ALWAYS run a new SQL query, while any? relies on cached or preloaded data is available. But I'd say this is generally desirable.

Actions #3

Updated by Go MAEDA 10 months ago

  • Category changed from Code cleanup/refactoring to Performance

Thank you for your advice.

I think it would be better to rewrite p.repositories.any? as well for consistency.

diff --git a/lib/redmine/preparation.rb b/lib/redmine/preparation.rb
index c51ecfdb9..9b33ae7da 100644
--- a/lib/redmine/preparation.rb
+++ b/lib/redmine/preparation.rb
@@ -361,13 +361,13 @@ module Redmine
                   :if => Proc.new {|p| p.wiki && !p.wiki.new_record?}
         menu.push :boards, {:controller => 'boards', :action => 'index', :id => nil},
                   :param => :project_id,
-                  :if => Proc.new {|p| p.boards.any?}, :caption => :label_board_plural
+                  :if => Proc.new {|p| p.boards.exists?}, :caption => :label_board_plural
         menu.push :files, {:controller => 'files', :action => 'index'},
                   :caption => :label_file_plural, :param => :project_id
         menu.push :repository,
                   {:controller => 'repositories', :action => 'show',
                    :repository_id => nil, :path => nil, :rev => nil},
-                  :if => Proc.new {|p| p.repositories.any? {|r| !r.new_record?}}
+                  :if => Proc.new {|p| p.repositories.exists?}
         menu.push :settings, {:controller => 'projects', :action => 'settings'},
                   :last => true
       end
Actions #4

Updated by Go MAEDA 10 months ago

  • Target version changed from Candidate for next major release to 6.0.0
Actions #5

Updated by Go MAEDA 9 months ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch posted in #note-2 in r22580. Thank you.

Actions #6

Updated by Go MAEDA 9 months ago

  • Tracker changed from Defect to Patch
  • Resolution deleted (Fixed)
Actions

Also available in: Atom PDF