Slide 1
Slide 1 text
Fix SQL N+1 queries with
RuboCop
RubyKaigi 2023 / #rubykaigi #rubykaigiC
pixiv Inc.
sue445
2023.5.12
Slide 2
Slide 2 text
2
Hello!
Slide 3
Slide 3 text
3
My name is Go
The Go gopher was designed by Renee French. (http://reneefrench.blogspot.com/) The design is licensed under the
Creative Commons 4.0 Attributions license. Read this article for more details: https://blog.golang.org/gopher
Slide 4
Slide 4 text
4
About sue445
● Go Sueyoshi (a.k.a. sue445, sue-san)
● https://twitter.com/sue445
● https://github.com/sue445
sue445
Slide 5
Slide 5 text
5
About sue445
● Ruby: 11 years
● Golang: 7 years
● Go: 41 years (age)
sue445
Slide 6
Slide 6 text
6
sue445 and RubyKaigi
● 2019: Fukuoka
● 2020: Matsumoto
○ My proposal has been accepted, but
Kaigi was canceled…
● 2023: Matsumoto
○ again!
sue445
Slide 7
Slide 7 text
7
I’m happy!
Slide 8
Slide 8 text
8
I’m happy! So, This is Happi(法被)!
Slide 9
Slide 9 text
● https://rubygems.org/profiles/sue445
○ e.g. rubicure (Ruby implementatation of Pretty Cure)
○ https://github.com/sue445/rubicure (since 2013, 10th Anniversary!)
● Maintainer of Itamae, rspec-parameterised and more more gems
9
Gems I Implemented
Slide 10
Slide 10 text
10
About pixiv Inc.
Slide 11
Slide 11 text
11
About pixiv Inc.
https://www.pixiv.co.jp/service/
Slide 12
Slide 12 text
● We support followings!
● Ruby Association
○ https://www.ruby.or.jp/ja/sponsors/list/
pixiv
● Rails Girls Japan
○ https://railsgirls.jp/sponsors/
● International Ruby Programming Contest
○ https://www.ruby-procon.net/corp-messa
ge/
12
[*$pixiv].all? { |members| love? Ruby } #=> true
Slide 13
Slide 13 text
https://conference.pixiv.co.jp/2023/rubymusicmixin
13
RubyMusicMixin 2023
Slide 14
Slide 14 text
14
RubyMusicMixin 2023
After Patry
MusicPartyMixin 2023
Slide 15
Slide 15 text
● Infrastructure Section
● AWS and GCP
○ Organization Owner
○ Solution Architect
● Maintainer of In-house tools (GitLab, Sentry etc)
● Other many many chore works
15
Day job
Slide 16
Slide 16 text
● SQL N+1 query detection and auto-correction
● Sharing of findings from custom cop creation
16
Goal in this talk
Slide 17
Slide 17 text
● About ISUCON
● About rubocop-isucon
● The path to implementing Isucon/Mysql2/NPlusOneQuery
● Result
● Conclusion
● Appendix
17
Agenda
Slide 18
Slide 18 text
● By connecting to the database from cop, the possibilities for cop are
limitless
● Detecting N+1 queries is easy
● Limited patterns, but auto-correct of N+1 queries is possible
● ISUCON is Mixed Martial Arts
18
Conclusion
Slide 19
Slide 19 text
● https://github.com/sue445/rubocop-isucon
19
What I will talk about in this talk
Slide 20
Slide 20 text
● Other gems I made for ISUCON
● Itamae recipe I made for ISUCON
● Monitor Sinatra app with Datadog
● Implement https://github.com/tkuchiki/alp with fluentd and Datadog
dashboard
● Ruby 3.2-dev YJIT vs Ruby 3.1 YJIT
See my articles (ja)
● https://sue445.hatenablog.com/entry/2022/07/04/084915
● https://sue445.hatenablog.com/entry/2022/07/24/190412
20
What I will NOT talk about in this talk
Slide 21
Slide 21 text
21
About ISUCON
Slide 22
Slide 22 text
● ISUCON is the most famous performance tuning contest in Japan. (since
2012)
○ https://isucon.net/
○ ISUCON is a trademark or registered trademark of LINE Corporation.
● Multiple reference implementations are provided for applications
○ e.g. Go, Node.js, Perl, PHP, Python, Ruby, Rust
22
About ISUCON
Slide 23
Slide 23 text
● Regulation
○ 1-3 people /team
○ 8 hours
○ Vale Tudo
■ Except for using a servers other than the prepared server
23
About ISUCON
Slide 24
Slide 24 text
● Ruby
○ Sinatra
○ mysql2 + mysql2-cs-bind
○ Puma
● nginx
● MySQL (or MariaDB)
● 3 VMs
24
Common initial configurations
Slide 25
Slide 25 text
● Improvement targets EVERYTHING in servers
● e.g.
○ Fix application
○ Add index to table
○ Add cache
○ Upgrade pre-installed middlewares, tuning configuration
○ Use ruby-head
■ ruby-head is the fastest Ruby in the world!
25
ISUCON is Mixed Martial Arts
Slide 26
Slide 26 text
● 2021/8: ISUCON 11 failing to qualify
○ 1-man team
● 1 person is not enough
● SQL N+1 queries occur frequently in ISUCON
○ -> Let's automate!
● Compete in my area of expertise
26
ISUCON and sue445
Slide 27
Slide 27 text
27
About rubocop-isucon
Slide 28
Slide 28 text
● RuboCop plugin for ruby reference implementation of ISUCON
● Development period: 2021/8 (ISUCON 11) - 2022/7 (ISUCON 12)
● Features
○ Isucon/Sinatra department
○ Isucon/Mysql2 department
○ Isucon/Mysql2/NPlusOneQuery cop
■ Checks that there’s no N+1 query
■ Main topic of this talk
28
https://github.com/sue445/rubocop-isuco
n
Slide 29
Slide 29 text
● SQL being called inside a loop
● Model Answers: e.g. Join tables
29
About SQL N+1 queries
courses = db.xquery(
"SELECT `courses`.*" \\
" FROM `courses`" \\
" JOIN `registrations` ON `courses`.`id` = `registrations`.`course_id`" \\
" WHERE `courses`.`status` != ? AND `registrations`.`user_id` = ?",
STATUS_CLOSED, user_id,
)
courses.map do |course|
teacher = db.xquery('SELECT * FROM `users` WHERE `id` = ?', course[:teacher_id]).first
# do something
end
Slide 30
Slide 30 text
● RuboCop has a plug-in mechanism. (ordinary gem)
● I use RuboCop on a regular basis and am familiar with its use
● I have created a custom cop and tool for RuboCop in the past
○ https://github.com/sue445/rubocop-itamae
○ https://github.com/sue445/rubocop_auto_corrector
30
Q. Why RuboCop?
Slide 31
Slide 31 text
● https://github.com/rubocop/rubocop-performance
● This is also valid, but it is not enough
● In fact, only a few cases were detected when run against the last 10+ years
of source code
31
vs rubocop-performance
Slide 32
Slide 32 text
● https://github.com/xinminlabs/synvert-ruby
● Synvert specializes in automatic code rewriting
● RuboCop can detect violations and automatically correct them if possible
○ In some cases, the machine may not be able to automatically correct the
violation, so I only want to detect violations at that time
○ e.g. nginx config, database index
32
vs Synvert
Slide 33
Slide 33 text
# bad
class App < Sinatra::Base
get '/' do
content_type :html
File.read(File.join(__dir__, '..', 'public', 'index.html'))
end
end
33
e.g. Isucon/Sinatra/ServeStaticFile
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Sinatra/ServeStaticFile.html
Slide 34
Slide 34 text
# bad
class App < Sinatra::Base
get '/' do
^^^^^^^^^^ Isucon/Sinatra/ServeStaticFile: Serve static files on front server (e.g.
nginx) instead of sinatra app
content_type :html
File.read(File.join(__dir__, '..', 'public', 'index.html'))
end
end
34
e.g. Isucon/Sinatra/ServeStaticFile
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Sinatra/ServeStaticFile.html
Slide 35
Slide 35 text
# good (e.g. Serve on nginx)
location / {
try_files $uri $uri/ /index.html;
}
35
e.g. Isucon/Sinatra/ServeStaticFile
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Sinatra/ServeStaticFile.html
Slide 36
Slide 36 text
# bad
db.xquery('SELECT id, title FROM articles WHERE user_id = ?', user_id)
36
e.g. Isucon/Mysql2/WhereWithoutIndex
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Mysql2/WhereWithoutIndex.html
Slide 37
Slide 37 text
# bad (user_id is not indexed)
db.xquery('SELECT id, title FROM articles WHERE user_id = ?', user_id)
^^^^^^^^^^^
Isucon/Mysql2/WhereWithoutIndex: This where clause doesn't seem to have an index.
(e.g. `ALTER TABLE articles ADD INDEX index_user_id (user_id)`)
37
e.g. Isucon/Mysql2/WhereWithoutIndex
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Mysql2/WhereWithoutIndex.html
Slide 38
Slide 38 text
ALTER TABLE articles ADD INDEX index_user_id (user_id)
38
e.g. Isucon/Mysql2/WhereWithoutIndex
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Mysql2/WhereWithoutIndex.html
# good (user_id is indexed)
db.xquery('SELECT id, title FROM articles WHERE used_id = ?', user_id)
Slide 39
Slide 39 text
# bad (user_id is not indexed)
db.xquery('SELECT id, title FROM articles JOIN users ON users.id = articles.user_id')
^^^^^^^^^^^^^^^^
Isucon/Mysql2/JoinWithoutIndex: This join clause doesn't seem to have an index. (e.g.
`ALTER TABLE articles ADD INDEX index_user_id (user_id)`)
# good (user_id is indexed)
db.xquery('SELECT id, title FROM articles JOIN users ON users.id = articles.user_id')
39
e.g. Isucon/Mysql2/JoinWithoutIndex
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Mysql2/JoinWithoutIndex.html
Slide 40
Slide 40 text
● Get information in table using ActiveRecord
○ e.g. columns, primary key, indexes
40
Get database schema
Slide 41
Slide 41 text
● Using SELECT * is slow because it retrieves all columns and assigns them to
Ruby objects
● Only necessary columns should be passed to the SELECT clause
● All columns are output to the SELECT clause to make it easier to remove
unnecessary columns
●
●
41
e.g. Isucon/Mysql2/SelectAsterisk
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Mysql2/SelectAsterisk.html
# bad
db.xquery('SELECT * FROM users')
# good (Auto-corrected!)
db.xquery('SELECT id, name FROM users')
Slide 42
Slide 42 text
42
Tips: Share settings with cops in the same department
# .rubocop.yml
Isucon/Mysql2:
Database:
adapter: mysql2
host: localhost
database: isucon_db
username: isucon
password: isucon
encoding: utf8
port: 3306
Slide 43
Slide 43 text
● Detect SQL N+1 queries
● Auto-correct SQL N+1 queries
43
The path to implementing Isucon/Mysql2/NPlusOneQuery
Slide 44
Slide 44 text
● Performance/CollectionLiteralInLoop code in rubocop-performance is
helpful
○ https://docs.rubocop.org/rubocop-performance/cops_performance.htm
l#performancecollectionliteralinloop
○ https://github.com/rubocop/rubocop-performance/blob/v1.11.5/lib/ru
bocop/cop/performance/collection_literal_in_loop.rb
● Can be detected by AST if in the same method, but not if separate methods
are used…
44
Detect SQL N+1 queries
Slide 45
Slide 45 text
● Parse SQL string and get as SQL AST
● Make SQL AST available to RuboCop
● Find N+1 query patterns that can be mechanically modified
● Avoid false positives
45
Auto-correct SQL N+1 queries
Slide 46
Slide 46 text
● https://github.com/tenderlove/gda
○ Wrapper gem for https://gitlab.gnome.org/GNOME/libgda
● I tried a few others but could not get the expected results
○ In particular, these gems could not parse isucon10-final's complex SQL
○ https://github.com/isucon/isucon10-final/blob/e858b25/webapp/ruby
/app.rb#L250-L318
■ a.k.a. Tourist Attractions
46
Parse SQL string and get as SQL AST
Slide 47
Slide 47 text
https://github.com/isucon/isucon10-final/blob/e858b25/webapp/ruby/app.rb#L250-L318
47
Tourist Attractions
Slide 48
Slide 48 text
● libgda does not support placeholder(?) and backquotes(`), so it is removed in
rubocop-isucon
● Deleting would change the location information, so care is taken to ensure
that the number of characters does not change
○ https://github.com/sue445/rubocop-isucon/blob/v0.2.0/lib/rubocop/is
ucon/gda.rb#L19-L25
48
Tips
# Before
db.xquery('SELECT id, title FROM `articles` WHERE user_id = ?', user_id)
# After
db.xquery('SELECT id, title FROM articles WHERE user_id = 0', user_id)
Slide 49
Slide 49 text
● o
● aaaa
49
GDA gem example
require "gda"
sql = "SELECT id, name FROM users"
statement = GDA::SQL::Parser.new.parse(sql)
ast = statement.ast
ast.from.targets.map(&:table_name)
# => ["users"]
ast.expr_list.map { |select_field| select_field.expr.value }
# => ["id", "name"]
Slide 50
Slide 50 text
● o
● aaaa
50
Get SQL AST in Hash format
pp JSON.parse(statement.serialize)
{"statement"=>
{"sql"=>"SELECT id, name FROM users",
"stmt_type"=>"SELECT",
"contents"=>
{"distinct"=>"false",
"fields"=>
[{"expr"=>{"value"=>"id"}, "field_name"=>"id"},
{"expr"=>{"value"=>"name"}, "field_name"=>"name"}],
"from"=>
{"targets"=>[{"expr"=>{"value"=>"users"}, "table_name"=>"users"}]}}}}
Slide 51
Slide 51 text
● AST retrieved by libgda does not contain SQL location information
● Cannot display offences in RuboCop or rewrite any node of SQL
51
Drawbacks of GDA gem (libgda)
db.xquery(<<~SQL, jia_user_id)
SELECT * FROM `isu`
WHERE `jia_user_id` = ? ORDER BY `id` DESC
^^^^^^^^^^^^^^^^^ Isucon/Mysql2/WhereWithoutIndex: This where clause
doesn't seem to have an index. (e.g. 'ALTER TABLE `isu` ADD INDEX
`index_jia_user_id` (jia_user_id)')
SQL
Slide 52
Slide 52 text
● GDA gem provides visitor pattern for AST exploration like RuboCop
● I could calculate the node's own location during the AST search
52
Solution: Implemented myself
Slide 53
Slide 53 text
53
RuboCop::Isucon::GDA::NodePatcher
class NodePatcher < ::GDA::Visitors::Visitor
# @param node [GDA::Nodes::Operation]
def visit_GDA_Nodes_Operation(node)
return super unless node.operator
pattern = operand_pattern(node)
return super unless pattern
node.location = search_operation_location(pattern)
super
end
https://github.com/sue445/rubocop-isucon/blob/v0.2.0/lib/rubocop/isucon/gda/node_patcher.rb#L19-L29
Slide 54
Slide 54 text
● This is inspired by paser gem’s Parser::Source::Range
54
class NodeLocation
# @return [Integer]
attr_reader :begin_pos
# @return [Integer]
attr_reader :end_pos
# @return [String]
attr_reader :body
RuboCop::Isucon::GDA::NodeLocation
https://github.com/sue445/rubocop-isucon/blob/v0.2.0/lib/rubocop/isucon/gda/node_location.rb#L7-L15
Slide 55
Slide 55 text
55
spec
it "location is appended" do
subject
where_clause =
node.where_cond.to_a.
select { |node| node.instance_of?(GDA::Nodes::Operation) && node.operator }
expect(where_clause.count).to eq 3
expect(where_clause[0].location).to eq location(begin_pos: 26, end_pos: 32, body: "id = 1")
expect(where_clause[1].location).to eq location(begin_pos: 37, end_pos: 46, body: "stock > 0")
expect(where_clause[2].location).to eq location(begin_pos: 51, end_pos: 67, body: "name IS NOT NULL")
end
https://github.com/sue445/rubocop-isucon/blob/v0.2.0/spec/rubocop/isucon/gda/node_patcher_spec.rb#L10-L35
Slide 56
Slide 56 text
56
However, spec is failed!!!!
1.1) Failure/Error: expect(where_clause[0].location).to eq location(begin_pos: 26, end_pos:
32, body: "id = 1")
expected: #
got: nil
(compared using ==)
# ./spec/rubocop/isucon/gda/node_patcher_spec.rb:31:in `block (5 levels) in '
https://github.com/sue445/rubocop-isucon/blob/v0.2.0/spec/rubocop/isucon/gda/node_patcher_spec.rb#L10-L35
Slide 57
Slide 57 text
● Because each call to an AST method like where_cond creates a new instance
● Even if I embed my own instance variable(location) in the Node class in the
Visitor, I will not be able to retrieve it the next time you call the same
method
57
Why????
Slide 58
Slide 58 text
● Memoizationneeds to be planted for all AST methods
● But there are 30+ such AST methods
● AST methods are methods in the C implementation, so prepend could not be
used
58
Solution: memoization, but…
def where_cond
@where_cond ||= original_where_cond
end
Slide 59
Slide 59 text
59
Solution: alias_method_chain like patch
def where_cond_with_cache
@where_cond_with_cache ||= where_cond_without_cache
end
alias_method :where_cond_without_cache, :where_cond
alias_method :where_cond, :where_cond_with_cache
Slide 60
Slide 60 text
https://github.com/sue445/rubocop-isucon/blob/v0.2.0/lib/rubocop/isucon/gda/gda_ext.rb
60
I created MemorizeMethods
GDA::Nodes::Select.class_eval do
extend RuboCop::Isucon::MemorizeMethods
memorize :distinct_expr
memorize :expr_list
memorize :from
memorize :where_cond
memorize :group_by
memorize :having_cond
memorize :order_by
memorize :limit_count
memorize :limit_offset
end
Slide 61
Slide 61 text
Which of the following is the fastest memorize?
61
Ruby Quiz
Slide 62
Slide 62 text
62
1. define_method + instance_variable_xxx + send
def self.memorize(method_name)
define_method "#{method_name}_with_cache" do
if (ret = instance_variable_get("@#{method_name}_with_cache"))
ret
else
ret = send("#{method_name}_without_cache")
instance_variable_set("@#{method_name}_with_cache", ret)
ret
end
end
alias_method "#{method_name}_without_cache", method_name
alias_method method_name, "#{method_name}_with_cache"
end
Slide 63
Slide 63 text
63
2. define_method + Hash + send
def self.memorize(method_name)
define_method "#{method_name}_with_cache" do
@__cache ||= {}
@__cache[method_name] ||= send("#{method_name}_without_cache")
end
alias_method "#{method_name}_without_cache", method_name
alias_method method_name, "#{method_name}_with_cache"
end
Slide 64
Slide 64 text
64
3. class_eval
def self.memorize(method_name)
class_eval <<~RUBY, __FILE__, __LINE__ + 1
# def foo_with_cache
# @foo_with_cache ||= foo_without_cache
# end
def #{method_name}_with_cache
@#{method_name}_with_cache ||= #{method_name}_without_cache
end
RUBY
alias_method "#{method_name}_without_cache", method_name
alias_method method_name, "#{method_name}_with_cache"
end
Slide 65
Slide 65 text
Which of the following is the fastest memorize?
1. define_method + instance_variable_xxx + send
2. define_method + Hash + send
3. class_eval
65
Ruby Quiz
Slide 66
Slide 66 text
66
Answer
Slide 67
Slide 67 text
3. class_eval
67
Answer
Slide 68
Slide 68 text
1. define_method + instance_variable_xxx + send
● 701.502k (±13.8%) i/s
2. define_method + Hash + send
● 1.549M (± 2.3%) i/s
3. class_eval
● 4.866M (± 8.1%) i/s
68
Benchmark
● https://github.com/sue445/rubocop-isucon/tree/v0.2.0/benchmark#memorizerb
● https://github.com/sue445/rubocop-isucon/blob/v0.2.0/benchmark/memorize.rb
Slide 69
Slide 69 text
eval is evil but faster
evalは悪(evil)だが速くなる
※define_method is slower than def
69
tl;dr;
Slide 70
Slide 70 text
● This is AST’s Reincarnation into another world (異世界転生)
● Basically, just add Parser::Source::Range (Ruby AST location information)
and RuboCop::Isucon::GDA::NodeLocation (SQL AST location information)
● However, Ruby is very expressive even when writing a single string, so it
was necessary to consider many different cases.
70
Bringing SQL AST into the Ruby World
Slide 71
Slide 71 text
71
Case 1: single line
db.xquery('SELECT * FROM `isu` WHERE `jia_user_id` = ? ORDER BY `id`
DESC', jia_user_id)
Slide 72
Slide 72 text
72
Case 1: single line
'SELECT * FROM `isu` WHERE `jia_user_id` = ? ORDER BY `id` DESC'
#=> "SELECT * FROM `isu` WHERE `jia_user_id` = ? ORDER BY `id` DESC"
Slide 73
Slide 73 text
73
Case 2: Heredoc (<<, <<-)
db.xquery(<<-SQL, jia_user_id)
SELECT * FROM `isu`
WHERE `jia_user_id` = ?
ORDER BY `id` DESC
SQL
Slide 74
Slide 74 text
74
Case 2: Heredoc (<<, <<-)
<<-SQL
SELECT * FROM `isu`
WHERE `jia_user_id` = ?
ORDER BY `id` DESC
SQL
#=> " SELECT * FROM `isu`\n WHERE `jia_user_id` = ?\n ORDER BY `id`
DESC\n"
Slide 75
Slide 75 text
75
Case 3: Heredoc (<<~) since Ruby 2.3
db.xquery(<<~SQL, jia_user_id)
SELECT * FROM `isu`
WHERE `jia_user_id` = ?
ORDER BY `id` DESC
SQL
Slide 76
Slide 76 text
76
Case 3: Heredoc (<<~) since Ruby 2.3
<<~SQL
SELECT * FROM `isu`
WHERE `jia_user_id` = ?
ORDER BY `id` DESC
SQL
#=> "SELECT * FROM `isu`\nWHERE `jia_user_id` = ?\nORDER BY `id` DESC\n"
Slide 77
Slide 77 text
77
Case 4: Escape concatenation at end of
line
classes = db.xquery(
"SELECT *" \
" FROM `classes`" \
" WHERE `course_id` = ?" \
" ORDER BY `part` DESC",
course[:id]
)
Slide 78
Slide 78 text
78
Case 4: Escape concatenation at end of
line
"SELECT *" \
" FROM `classes`" \
" WHERE `course_id` = ?" \
" ORDER BY `part` DESC"
#=> "SELECT * FROM `classes` WHERE `course_id` = ? ORDER BY `part` DESC"
Slide 79
Slide 79 text
● As is the case with custom Cop in general, it is easy to just detect offenses
● However, it is very difficult to deal with auto correct of offenses
● It took me about 2 months to figure out what kind of SQL N+1 query could
be automatically modified by RuboCop
● It took me 1 month to come up with the idea and actually implement it
79
Find N+1 query patterns that can be mechanically modified
Slide 80
Slide 80 text
● I found a hint in ActiveRecord
● ActiveRecord has a function called includes to avoid N+1 and issue a query
when an association between models is defined by has_many or belongs_to
● ISUCON's reference implementation doesn’t use ActiveRecord, but plain SQL.
● However, I looked at the plain SQL and thought that an association-like SQL
and table structure could improve the N+1 query by automatically modifying
the SQL in the same way
80
How did I come up with the idea?
Slide 81
Slide 81 text
81 https://guides.rubyonrails.org/active_record_querying.html#includes
books = Book.limit(10)
books.each do |book|
puts book.author.last_name
end
Excerpt from Ruby on Rails Guides
SELECT books.* FROM books LIMIT 10
SELECT authors.* FROM authors WHERE authors.book_id = 1
SELECT authors.* FROM authors WHERE authors.book_id = 2
：
：
11 queries
Slide 82
Slide 82 text
82 https://guides.rubyonrails.org/active_record_querying.html#includes
books = Book.includes(:author).limit(10)
books.each do |book|
puts book.author.last_name
end
Excerpt from Ruby on Rails Guides
SELECT books.* FROM books LIMIT 10
SELECT authors.* FROM authors
WHERE authors.book_id IN (1,2,3,4,5,6,7,8,9,10)
2 queries
Slide 83
Slide 83 text
● To prevent false positives, I examined patterns that could be safely and
automatically corrected
83
Avoid false positives
Slide 84
Slide 84 text
● SELECT statement
● Only one table exists in SQL. (Does not include UNION, JOIN, or subqueries)
● GROUP BY clause doesn’t exist
● Aggregate functions (e.g. COUNT, MAX, MIN, SUM, AVG) doesn’t exist in the
SELECT clause
● Only one condition in the WHERE clause and the column in which it appears is
either a Primary Key or a single column Unique Key
84
Conditions for safe automatic correction
Slide 85
Slide 85 text
85
class Mysql2NPlusOneQueryCorrector
def correct
replace if correctable?
end
end
RuboCop::Cop::Isucon::Correctors::Mysql2NPlusOneQueryCorrector
https://github.com/sue445/rubocop-isucon/blob/v0.2.0/lib/rubocop/cop/isucon/correctors/n_plus_one_query_corrector.rb
#L69-L71
Slide 86
Slide 86 text
86
module CorrectableMethods
def correctable?
correctable_gda? && correctable_xquery_arg? &&
correctable_parent_receiver? && current_node.child_nodes.count == 3 &&
xquery_lvar.lvasgn_type? && %i[first last].include?(xquery_chained_method)
end
end
RuboCop::Cop::Isucon::Correctors::Mysql2NPlusOneQueryCorrector::CorrectableMethods
https://github.com/sue445/rubocop-isucon/blob/v0.2.0/lib/rubocop/cop/isucon/correctors/n_plus_one_query_corrector/co
rrectable_methods.rb#L11-L15
Slide 87
Slide 87 text
87
class Mysql2NPlusOneQueryCorrector
module ReplaceMethods
def replace
replace_where_condition_in_sql
replace_xquery_2nd_arg
replace_chained_method_to_each_with_object
replace_to_2_lines
end
end
end
RuboCop::Cop::Isucon::Correctors::Mysql2NPlusOneQueryCorrector::ReplaceMethods
https://github.com/sue445/rubocop-isucon/blob/v0.2.0/lib/rubocop/cop/isucon/correctors/n_plus_one_query_corrector/re
place_methods.rb#L10-L15
Slide 88
Slide 88 text
88
Before auto-correct
courses.map do |course|
teacher = db.xquery('SELECT * FROM `users` WHERE `id` = ?', course[:teacher_id]).first
end
Slide 89
Slide 89 text
89
Plot for auto-correct
user_ids = courses.map { |course| course[:teacher_id] }
users = db.xquery('SELECT * FROM `users` WHERE `id` IN (?)', user_ids)
users_by_id = users.each_with_object({}) do |user, hash|
hash[user[:id]] = user
end
courses.map do |course|
teacher = users_by_id[course[:teacher_id]]
end
Slide 90
Slide 90 text
90
Plot for auto-correct
user_ids = courses.map { |course| course[:teacher_id] }
users = db.xquery('SELECT * FROM `users` WHERE `id` IN (?)', user_ids)
users_by_id = users.each_with_object({}) do |user, hash|
hash[user[:id]] = user
end
courses.map do |course|
teacher = users_by_id[course[:teacher_id]]
end
Convert WHERE `id` = ? inside the loop to WHERE `id` IN
(?) outside the loop to avoid N+1 queries
Slide 91
Slide 91 text
91
Plot for auto-correct
user_ids = courses.map { |course| course[:teacher_id] }
users = db.xquery('SELECT * FROM `users` WHERE `id` IN (?)', user_ids)
users_by_id = users.each_with_object({}) do |user, hash|
hash[user[:id]] = user
end
courses.map do |course|
teacher = users_by_id[course[:teacher_id]]
end
Create a Hash with user_id as key and
user as value
Slide 92
Slide 92 text
92
After auto-correct
courses.map do |course|
@users_by_id ||= db.xquery('SELECT * FROM `users` WHERE `id` IN (?)', courses.map {
|course| course[:teacher_id] }).each_with_object({}) { |v, hash| hash[v[:id]] = v }
teacher = @users_by_id[course[:teacher_id]]
end
Slide 93
Slide 93 text
● It is difficult to split one line of processing into multiple lines in RuboCop
○ Because it is necessary to consider indentation
● I'm trying to reduce the number of lines as much as possible, so I'm using
one-liners to push method chains in and out to get more lines
● However, it was difficult to understand this case if it was completely on
one line, so I made it on two lines as a bitter effort
93
Tips
Slide 94
Slide 94 text
94
1 line ver vs 2 lines ver
# 1 line
@users_by_id ||= db.xquery('SELECT * FROM `users` WHERE `id` IN (?)', courses.map { |course|
course[:teacher_id] }).each_with_object({}) { |v, hash| hash[v[:id]] = v }; teacher =
@users_by_id[course[:teacher_id]]
# 2 lines
@users_by_id ||= db.xquery('SELECT * FROM `users` WHERE `id` IN (?)', courses.map { |course|
course[:teacher_id] }).each_with_object({}) { |v, hash| hash[v[:id]] = v }
teacher = @users_by_id[course[:teacher_id]]
Slide 95
Slide 95 text
95
Write test code for auto corrected code
it "auto-corrected code is valid" do
corrected_code = <<~RUBY
courses = [{"id" => 1, "teacher_id" => 1}]
courses.map do |course|
@users_by_id ||= db.xquery('SELECT * FROM `users` WHERE `id` IN (?)', courses.map { |course| course["teacher_id"]
}).each_with_object({}) { |v, hash| hash[v["id"]] = v }
teacher = @users_by_id[course["teacher_id"]]
teacher["name"]
end
RUBY
expect_correction(corrected_code)
teacher_names = eval(corrected_code) # rubocop:disable Security/Eval
expect(teacher_names).to eq(["user_1"])
end
https://github.com/sue445/rubocop-isucon/blob/v0.2.0/spec/rubocop/cop/isucon/mysql2/n_plus_one_query_spec.rb#L769-L793
Slide 96
Slide 96 text
● Less than 10% of all N+1 queries could be corrected automatically with this
method
● isucon12 qualify’s theme was Multi-tenant SaaS app
○ https://isucon.net/archives/56850281.html (ja)
○ https://github.com/isucon/isucon12-qualify
○ MySQL + SQLite!!!
● ISUCON is a mixed martial art, so just being able to do static analysis and
automatic modification of N+1 queries is not enough to win
96
Result
Slide 97
Slide 97 text
● By connecting to the database from cop, the possibilities for cop are
limitless
● Detecting N+1 queries is easy
● Limited patterns, but auto-correct of N+1 queries is possible
● ISUCON is Mixed Martial Arts
97
Conclusion
Slide 98
Slide 98 text
● Introduce cops included in rubocop-isucon
98
Appendix
Slide 99
Slide 99 text
● o
● aaaa
99
Isucon/Mysql2/ManyJoinTable
# bad
totals = db.xquery(
^^^^^^^^^^ Isucon/Mysql2/ManyJoinTable: Avoid SQL with lots of JOINs
"SELECT IFNULL(SUM(`submissions`.`score`), 0) AS `total_score`" \\
" FROM `users`" \\
" JOIN `registrations` ON `users`.`id` = `registrations`.`user_id`" \\
" JOIN `courses` ON `registrations`.`course_id` = `courses`.`id`" \\
" LEFT JOIN `classes` ON `courses`.`id` = `classes`.`course_id`" \\
" LEFT JOIN `submissions` ON `users`.`id` = `submissions`.`user_id` AND
`submissions`.`class_id` = `classes`.`id`" \\
" WHERE `courses`.`id` = ?" \\
" GROUP BY `users`.`id`",
course[:id]
).map { |_| _[:total_score] }
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Mysql2/ManyJoinTable.html
Slide 100
Slide 100 text
● o
● aaaa
100
Isucon/Mysql2/PrepareExecute
# bad (auto-correct is possible)
db.prepare('SELECT * FROM `users` WHERE `id` = ?').execute(
session[:user][:id]
).first
# good
require 'mysql2-cs-bind'
db.xquery('SELECT * FROM `users` WHERE `id` = ?',
session[:user][:id]
).first
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Mysql2/PrepareExecute.html
Slide 101
Slide 101 text
● o
● aaaa
101
Isucon/Sinatra/DisableLogging
# bad
class App < Sinatra::Base
enable :logging
end
# bad
class App < Sinatra::Base
end
# good
class App < Sinatra::Base
disable :logging
end
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Sinatra/DisableLogging.html
Slide 102
Slide 102 text
● o
● aaaa
102
Isucon/Sinatra/Logger
# bad
logger.error "Search condition not found"
# good
# no logging
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Sinatra/Logger.html
Slide 103
Slide 103 text
● o
● aaaa
103
Isucon/Sinatra/RackLogger
# bad
request.env['rack.logger'].warn 'drop post isu condition request'
# good
# no logging
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Sinatra/RackLogger.html
Slide 104
Slide 104 text
● o
● aaaa
104
Isucon/Shell/Backtick
# bad
def digest(src)
`printf "%s" #{Shellwords.shellescape(src)} | openssl dgst -sha512 |
sed 's/^.*= //'`.strip
end
# good
def digest(src)
OpenSSL::Digest::SHA512.hexdigest(src)
end
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Shell/Backtick.html
Slide 105
Slide 105 text
● o
● aaaa
105
Isucon/Shell/System
# bad
system("sleep 1")
# good
sleep 1
https://sue445.github.io/rubocop-isucon/RuboCop/Cop/Isucon/Shell/System.html
Slide 106
Slide 106 text
● I proposed to @koic to introduce these cops to rubocop-performance, but
rejected
106
BTW
Slide 107
Slide 107 text
● These are the same features as Isucon/Mysql2
○ Isucon/Sqlite3/JoinWithoutIndex
○ Isucon/Sqlite3/ManyJoinTable
○ Isucon/Sqlite3/NPlusOneQuery
○ Isucon/Sqlite3/SelectAsterisk
○ Isucon/Sqlite3/WhereWithoutIndex
107
Isucon/Sqlite3 department