Project

General

Profile

Actions

Patch #26747

closed

Use find_by instead of where.first to remove unnecessary sorting

Added by jwjw yy over 6 years ago. Updated over 5 years ago.

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

0%

Estimated time:

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


Files

Actions #1

Updated by Toshi MARUYAMA over 6 years ago

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

Please post by attachment patch file.

Actions #2

Updated by Toshi MARUYAMA over 6 years 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"?

Actions #3

Updated by jwjw yy over 6 years 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

Actions #4

Updated by jwjw yy almost 6 years ago

Removed advertisement -- Holger Just

Actions #5

Updated by Go MAEDA over 5 years 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]]
Actions #6

Updated by Go MAEDA over 5 years 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
Actions #7

Updated by Yuichi HARADA over 5 years ago

I changed where.first to find_by as follows.
  1. "Primary key" is specified as a condition
  2. "Unique index column" is specified as a condition
  3. Only one registration like AnonymousUser
Actions #8

Updated by Go MAEDA over 5 years ago

  • Assignee set to Go MAEDA
  • Target version changed from Candidate for next major release to 4.0.0

Committed. Thank you for improving Redmine.

Actions #9

Updated by Go MAEDA over 5 years ago

  • Status changed from New to Closed
Actions

Also available in: Atom PDF