Patch #26747

Use find_by instead of where.first to remove unnecessary sorting

Added by jwjw yy about 1 year ago. Updated about 1 month ago.

Status:NewStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:Performance
Target version:Candidate for next major release

Description

sicne where.first will issue query with order by primary key which is actually unnecessary.

redmine/app/helpers/application_helper.rb:909:              u = User.visible.where(:id => oid, :type => 'User').first
redmine/app/helpers/application_helper.rb:941:                  if repository && (changeset = Changeset.visible.where("repository_id = ? AND scmid LIKE ?", repository.id, "#{name}%").first)
redmine/app/helpers/application_helper.rb:966:              if p = Project.visible.where("identifier = :s OR LOWER(name) = :s", :s => name.downcase).first
redmine/app/helpers/application_helper.rb:970:              u = User.visible.where(:login => name, :type => 'User').first
redmine/app/helpers/application_helper.rb:975:            u = User.visible.where(:login => name, :type => 'User').first
redmine/app/models/issue_query.rb:474:      root_id, lft, rgt = Issue.where(:id => value.first.to_i).pluck(:root_id, :lft, :rgt).first
redmine/app/models/issue_query.rb:490:      parent_id = Issue.where(:id => value.first.to_i).pluck(:parent_id).first
redmine/app/models/issue_query.rb:497:      root_id, lft, rgt = Issue.where(:id => value.first.to_i).pluck(:root_id, :lft, :rgt).first
redmine/app/models/wiki_content_version.rb:94:      where("wiki_content_id = ? AND version < ?", wiki_content_id, version).first
redmine/app/models/wiki_content_version.rb:102:      where("wiki_content_id = ? AND version > ?", wiki_content_id, version).first
redmine/app/models/principal.rb:128:    Principal.visible(user).where(:id => id).first == self
redmine/app/models/token.rb:115:    token = Token.where(:action => action, :value => key).first
redmine/app/models/user.rb:196:    user = where(:identity_url => url).first
redmine/app/models/user.rb:489:        user = where("LOWER(login) = ?", login.downcase).first
redmine/app/models/user.rb:560:      h[project_id] = memberships.where(:project_id => project_id).first
redmine/app/models/wiki.rb:56:    page = pages.where("LOWER(title) = LOWER(?)", title).first
redmine/app/models/wiki.rb:59:      redirect = redirects.where("LOWER(title) = LOWER(?)", title).first
redmine/app/models/issue_import.rb:148:          elsif issue_id = items.where(:position => parent_issue_id).first.try(:obj_id)
redmine/app/models/issue_import.rb:193:    child_id = items.where(:position => child_position).first.try(:obj_id)
redmine/app/models/attachment.rb:271:      attachment = Attachment.where(:id => attachment_id, :digest => attachment_digest).first
redmine/app/models/time_entry_query.rb:150:      issue = Issue.where(:id => value.first.to_i).first
redmine/app/models/repository.rb:252:      changesets.where("revision = ?", s).first
redmine/app/models/repository.rb:254:      changesets.where("revision LIKE ?", s + '%').first
redmine/app/models/role.rb:296:    role = unscoped.where(:builtin => builtin).first
redmine/app/models/repository/git.rb:92:      changesets.where(:revision => name.to_s).first ||
redmine/app/models/repository/git.rb:93:        changesets.where('scmid LIKE ?', "#{name}%").first
redmine/app/models/repository/mercurial.rb:100:      cs = changesets.where(:scmid => s).first
redmine/app/models/repository/mercurial.rb:102:      cs = changesets.where(:revision => s).first
redmine/app/models/repository/mercurial.rb:105:    changesets.where('scmid LIKE ?', "#{s}%").first
redmine/app/models/enumeration.rb:47:      where(:is_default => true, :type => 'Enumeration').first
redmine/app/models/enumeration.rb:50:      where(:is_default => true).first
redmine/app/controllers/imports_controller.rb:100:    @import = Import.where(:user_id => User.current.id, :filename => params[:id]).first

History

#1 Updated by Toshi MARUYAMA about 1 year ago

  • Category set to Code cleanup/refactoring
  • Status changed from New to Needs feedback

Please post by attachment patch file.

#2 Updated by Toshi MARUYAMA about 1 year ago

jwjw yy wrote:

sicne where.first will issue query with order by primary key which is actually unnecessary.

Really? Do all your description "where"s call "order by"?

#3 Updated by jwjw yy about 1 year ago

Toshi MARUYAMA wrote:

jwjw yy wrote:

sicne where.first will issue query with order by primary key which is actually unnecessary.

Really? Do all your description "where"s call "order by"?

yes

#4 Updated by jwjw yy 2 months ago

Removed advertisement -- Holger Just

#5 Updated by Go MAEDA about 1 month ago

  • Subject changed from use find_by to replace where.first to Use find_by instead of where.first to remove unnecessary sorting
  • Category changed from Code cleanup/refactoring to Performance
  • Status changed from Needs feedback to New
  • Target version set to Candidate for next major release

Toshi MARUYAMA wrote:

jwjw yy wrote:

sicne where.first will issue query with order by primary key which is actually unnecessary.

Really? Do all your description "where"s call "order by"?

It is documented in the Rails API reference.

https://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-first

Find the first record (or first N records if a parameter is supplied). If no order is defined it will order by primary key.

> User.where(:login => 'admin').first
  User Load (0.4ms)  SELECT  "users".* FROM "users" WHERE "users"."type" IN ('User', 'AnonymousUser') AND "users"."login" = ? ORDER BY "users"."id" ASC LIMIT ?  [["login", "admin"], ["LIMIT", 1]]
> User.find_by(:login => 'admin')
  User Load (0.4ms)  SELECT  "users".* FROM "users" WHERE "users"."type" IN ('User', 'AnonymousUser') AND "users"."login" = ? LIMIT ?  [["login", "admin"], ["LIMIT", 1]]

#6 Updated by Go MAEDA about 1 month ago

find_by is significantly faster than where.first.

require 'benchmark/ips'

Benchmark.ips do |x|
  x.report('where.first') { User.where(:login => 'admin').first }
  x.report('find_by') { User.find_by(:login => 'admin') }
end
$ bin/rails r bench-26747.rb
.
.
(snip)
.
.
Warming up --------------------------------------
         where.first    80.000  i/100ms
             find_by   165.000  i/100ms
Calculating -------------------------------------
         where.first    961.953  (± 5.5%) i/s -      4.800k in   5.005696s
             find_by      1.664k (± 4.6%) i/s -      8.415k in   5.066908s

Also available in: Atom PDF