aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/lib
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2019-06-14 21:24:54 +0900
committerRyuta Kamizono <kamipo@gmail.com>2019-06-15 21:48:12 +0900
commit85b4ba2836437ff30ff565005b2500500200611b (patch)
treeb42b9621f32ebd951f25787e9f76b441872d9fdd /activerecord/lib
parentc8a84603e2fa97822407bf4a1c8ac0308a899163 (diff)
downloadrails-85b4ba2836437ff30ff565005b2500500200611b.tar.gz
rails-85b4ba2836437ff30ff565005b2500500200611b.tar.bz2
rails-85b4ba2836437ff30ff565005b2500500200611b.zip
No allocation `Arel::Visitors::ToSql#visit`
Each `visit o, collector` allocates one extra array due to receiving args by splat array. https://github.com/rails/rails/blob/2c3332cc4c0fa77dbe2e13e8a792f80fbd8f4ad3/activerecord/lib/arel/visitors/visitor.rb#L27-L29 Currently 1,000 times `User.where(id: 1).to_sql` allocates 13,000 arrays in `visitor.accept`. This avoids receiving args by splat array, it makes `visitor.accept` no array allocation. ```ruby ObjectSpace::AllocationTracer.setup(%i{path line type}) pp ObjectSpace::AllocationTracer.trace { 1_000.times { User.where(id: 1).to_sql } }.select { |k, _| k[2] == :T_ARRAY && k[0].end_with?("visitor.rb", "to_sql.rb") } ``` Before (2c3332cc4c0fa77dbe2e13e8a792f80fbd8f4ad3): ``` {["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 18, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb", 11, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/visitor.rb", 12, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 788, :T_ARRAY]=>[3000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 794, :T_ARRAY]=>[3000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 156, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 443, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 603, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 611, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0]} ``` After (this change): ``` {} ```
Diffstat (limited to 'activerecord/lib')
-rw-r--r--activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb4
-rw-r--r--activerecord/lib/arel/visitors/depth_first.rb2
-rw-r--r--activerecord/lib/arel/visitors/to_sql.rb49
-rw-r--r--activerecord/lib/arel/visitors/visitor.rb12
4 files changed, 34 insertions, 33 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb b/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb
index 1df4dea2d8..97d74df529 100644
--- a/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb
+++ b/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb
@@ -5,7 +5,7 @@ module ActiveRecord
module DetermineIfPreparableVisitor
attr_accessor :preparable
- def accept(*)
+ def accept(object, collector)
@preparable = true
super
end
@@ -20,7 +20,7 @@ module ActiveRecord
super
end
- def visit_Arel_Nodes_SqlLiteral(*)
+ def visit_Arel_Nodes_SqlLiteral(o, collector)
@preparable = false
super
end
diff --git a/activerecord/lib/arel/visitors/depth_first.rb b/activerecord/lib/arel/visitors/depth_first.rb
index ca11d9c879..98c3f92cf1 100644
--- a/activerecord/lib/arel/visitors/depth_first.rb
+++ b/activerecord/lib/arel/visitors/depth_first.rb
@@ -9,7 +9,7 @@ module Arel # :nodoc: all
end
private
- def visit(o)
+ def visit(o, _ = nil)
super
@block.call o
end
diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb
index a67a660fed..a1aa620f9d 100644
--- a/activerecord/lib/arel/visitors/to_sql.rb
+++ b/activerecord/lib/arel/visitors/to_sql.rb
@@ -51,10 +51,14 @@ module Arel # :nodoc: all
def visit_Arel_Nodes_InsertStatement(o, collector)
collector << "INSERT INTO "
collector = visit o.relation, collector
- if o.columns.any?
- collector << " (#{o.columns.map { |x|
- quote_column_name x.name
- }.join ', '})"
+
+ unless o.columns.empty?
+ collector << " ("
+ o.columns.each_with_index do |x, i|
+ collector << ", " unless i == 0
+ collector << quote_column_name(x.name)
+ end
+ collector << ")"
end
if o.values
@@ -96,22 +100,20 @@ module Arel # :nodoc: all
def visit_Arel_Nodes_ValuesList(o, collector)
collector << "VALUES "
- len = o.rows.length - 1
- o.rows.each_with_index { |row, i|
+ o.rows.each_with_index do |row, i|
+ collector << ", " unless i == 0
collector << "("
- row_len = row.length - 1
row.each_with_index do |value, k|
+ collector << ", " unless k == 0
case value
when Nodes::SqlLiteral, Nodes::BindParam
collector = visit(value, collector)
else
collector << quote(value).to_s
end
- collector << ", " unless k == row_len
end
collector << ")"
- collector << ", " unless i == len
- }
+ end
collector
end
@@ -127,11 +129,10 @@ module Arel # :nodoc: all
unless o.orders.empty?
collector << " ORDER BY "
- len = o.orders.length - 1
- o.orders.each_with_index { |x, i|
+ o.orders.each_with_index do |x, i|
+ collector << ", " unless i == 0
collector = visit(x, collector)
- collector << ", " unless len == i
- }
+ end
end
visit_Arel_Nodes_SelectOptions(o, collector)
@@ -505,7 +506,7 @@ module Arel # :nodoc: all
def visit_Arel_Table(o, collector)
if o.table_alias
- collector << "#{quote_table_name o.name} #{quote_table_name o.table_alias}"
+ collector << quote_table_name(o.name) << " " << quote_table_name(o.table_alias)
else
collector << quote_table_name(o.name)
end
@@ -681,13 +682,12 @@ module Arel # :nodoc: all
end
def visit_Arel_Nodes_UnqualifiedColumn(o, collector)
- collector << "#{quote_column_name o.name}"
- collector
+ collector << quote_column_name(o.name)
end
def visit_Arel_Attributes_Attribute(o, collector)
join_name = o.relation.table_alias || o.relation.name
- collector << "#{quote_table_name join_name}.#{quote_column_name o.name}"
+ collector << quote_table_name(join_name) << "." << quote_column_name(o.name)
end
alias :visit_Arel_Attributes_Integer :visit_Arel_Attributes_Attribute
alias :visit_Arel_Attributes_Float :visit_Arel_Attributes_Attribute
@@ -784,14 +784,11 @@ module Arel # :nodoc: all
end
def inject_join(list, collector, join_str)
- len = list.length - 1
- list.each_with_index.inject(collector) { |c, (x, i)|
- if i == len
- visit x, c
- else
- visit(x, c) << join_str
- end
- }
+ list.each_with_index do |x, i|
+ collector << join_str unless i == 0
+ collector = visit(x, collector)
+ end
+ collector
end
def unboundable?(value)
diff --git a/activerecord/lib/arel/visitors/visitor.rb b/activerecord/lib/arel/visitors/visitor.rb
index 2d5a2b6681..d65ac820bc 100644
--- a/activerecord/lib/arel/visitors/visitor.rb
+++ b/activerecord/lib/arel/visitors/visitor.rb
@@ -7,8 +7,8 @@ module Arel # :nodoc: all
@dispatch = get_dispatch_cache
end
- def accept(object, *args)
- visit object, *args
+ def accept(object, collector = nil)
+ visit object, collector
end
private
@@ -24,9 +24,13 @@ module Arel # :nodoc: all
self.class.dispatch_cache
end
- def visit(object, *args)
+ def visit(object, collector = nil)
dispatch_method = dispatch[object.class]
- send dispatch_method, object, *args
+ if collector
+ send dispatch_method, object, collector
+ else
+ send dispatch_method, object
+ end
rescue NoMethodError => e
raise e if respond_to?(dispatch_method, true)
superklass = object.class.ancestors.find { |klass|