Patch #39835
closed
Optimize repository menu visibility check
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
Updated by Go MAEDA about 1 year ago
Updated by Holger Just about 1 year 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.
Updated by Go MAEDA about 1 year 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
Updated by Go MAEDA about 1 year ago
- Target version changed from Candidate for next major release to 6.0.0