Skip to content

Commit

Permalink
Don't allocate new strings in compiled attribute methods
Browse files Browse the repository at this point in the history
This improves memory and performance without having to use symbols which
present DoS problems. Thanks @headius and @tenderlove for the
suggestion.

Benchmark
---------

require 'active_record'
require 'benchmark/ips'

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database:
':memory:')

class Post < ActiveRecord::Base
  connection.create_table :posts, force: true do |t|
    t.string :name
  end
end

post = Post.create name: 'omg'

Benchmark.ips do |r|
  r.report('Post.new')          { Post.new name: 'omg' }
  r.report('post.name')         { post.name }
  r.report('post.name=')        { post.name = 'omg' }
  r.report('Post.find(1).name') { Post.find(1).name }
end

Before
------

Calculating -------------------------------------
            Post.new      1419 i/100ms
           post.name      7538 i/100ms
          post.name=      3024 i/100ms
   Post.find(1).name       243 i/100ms
-------------------------------------------------
            Post.new    20637.6 (±12.7%) i/s -     102168 in   5.039578s
           post.name  1167897.7 (±18.2%) i/s -    5186144 in   4.983077s
          post.name=    64305.6 (±9.6%) i/s -     317520 in   4.998720s
   Post.find(1).name     2678.8 (±10.8%) i/s -      13365 in   5.051265s

After
-----

Calculating -------------------------------------
            Post.new      1431 i/100ms
           post.name      7790 i/100ms
          post.name=      3181 i/100ms
   Post.find(1).name       245 i/100ms
-------------------------------------------------
            Post.new    21308.8 (±12.2%) i/s -     105894 in   5.053879s
           post.name  1534103.8 (±2.1%) i/s -    7634200 in   4.979405s
          post.name=    67441.0 (±7.5%) i/s -     337186 in   5.037871s
   Post.find(1).name     2681.9 (±10.6%) i/s -      13475 in   5.084511s
  • Loading branch information
jonleighton committed Oct 12, 2012
1 parent 9e5f7cc commit f176501
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 20 deletions.
39 changes: 27 additions & 12 deletions activerecord/lib/active_record/attribute_methods/read.rb
Expand Up @@ -35,21 +35,36 @@ def cache_attribute?(attr_name)

protected

# We want to generate the methods via module_eval rather than define_method,
# because define_method is slower on dispatch and uses more memory (because it
# creates a closure).
# We want to generate the methods via module_eval rather than
# define_method, because define_method is slower on dispatch and
# uses more memory (because it creates a closure).
#
# But sometimes the database might return columns with characters that are not
# allowed in normal method names (like 'my_column(omg)'. So to work around this
# we first define with the __temp__ identifier, and then use alias method to
# rename it to what we want.
def define_method_attribute(attr_name)
# But sometimes the database might return columns with
# characters that are not allowed in normal method names (like
# 'my_column(omg)'. So to work around this we first define with
# the __temp__ identifier, and then use alias method to rename
# it to what we want.
#
# We are also defining a constant to hold the frozen string of
# the attribute name. Using a constant means that we do not have
# to allocate an object on each call to the attribute method.
# Making it frozen means that it doesn't get duped when used to
# key the @attributes_cache in read_attribute.
def define_method_attribute(name)
safe_name = name.unpack('h*').first
generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
def __temp__
read_attribute('#{attr_name}') { |n| missing_attribute(n, caller) }
module AttrNames
unless defined? ATTR_#{safe_name}
ATTR_#{safe_name} = #{name.inspect}.freeze
end
end
alias_method '#{attr_name}', :__temp__
undef_method :__temp__
def __temp__#{safe_name}
read_attribute(AttrNames::ATTR_#{safe_name}) { |n| missing_attribute(n, caller) }
end
alias_method #{name.inspect}, :__temp__#{safe_name}
undef_method :__temp__#{safe_name}
STR
end

Expand Down
20 changes: 12 additions & 8 deletions activerecord/lib/active_record/attribute_methods/write.rb
Expand Up @@ -9,15 +9,19 @@ module Write

module ClassMethods
protected
def define_method_attribute=(attr_name)
if attr_name =~ ActiveModel::AttributeMethods::NAME_COMPILABLE_REGEXP
generated_attribute_methods.module_eval("def #{attr_name}=(new_value); write_attribute('#{attr_name}', new_value); end", __FILE__, __LINE__)
else
generated_attribute_methods.send(:define_method, "#{attr_name}=") do |new_value|
write_attribute(attr_name, new_value)
end

# See define_method_attribute in read.rb for an explanation of
# this code.
def define_method_attribute=(name)
safe_name = name.unpack('h*').first
generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
def __temp__#{safe_name}=(value)
write_attribute(AttrNames::ATTR_#{safe_name}, value)
end
end
alias_method #{(name + '=').inspect}, :__temp__#{safe_name}=
undef_method :__temp__#{safe_name}=
STR
end
end

# Updates the attribute identified by <tt>attr_name</tt> with the
Expand Down

0 comments on commit f176501

Please sign in to comment.