diff options
author | Ryuta Kamizono <kamipo@gmail.com> | 2019-06-14 21:24:54 +0900 |
---|---|---|
committer | Ryuta Kamizono <kamipo@gmail.com> | 2019-06-15 21:48:12 +0900 |
commit | 85b4ba2836437ff30ff565005b2500500200611b (patch) | |
tree | b42b9621f32ebd951f25787e9f76b441872d9fdd /activerecord/lib | |
parent | c8a84603e2fa97822407bf4a1c8ac0308a899163 (diff) | |
download | rails-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')
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| |