Don't allocate new strings in compiled attribute methods

This improves memory and performance without having to use symbols which
present DoS problems. Thanks @headius and @tenderlove for the
suggestion.

This was originally committed in
f1765019ce9b6292f2264b4601dad5daaffe3a89, and then reverted in
d3494903719682abc0948bef290af0d3d7b5a440 due to it causing problems in a
real application. This second attempt should solve that.

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

3 files changed, 38 insertions, 21 deletions

diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb
index 90701938e5..3c03cce838 100644
--- a/activerecord/lib/active_record/attribute_methods/read.rb
+++ b/activerecord/lib/active_record/attribute_methods/read.rb
@@ -32,21 +32,29 @@ module ActiveRecord
 
         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) }
+            def __temp__#{safe_name}
+              read_attribute(AttrNames::ATTR_#{safe_name}) { |n| missing_attribute(n, caller) }
             end
-            alias_method '#{attr_name}', :__temp__
-            undef_method :__temp__
+            alias_method #{name.inspect}, :__temp__#{safe_name}
+            undef_method :__temp__#{safe_name}
           STR
         end
 
diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb
index fa9097db1f..cd33494cc3 100644
--- a/activerecord/lib/active_record/attribute_methods/write.rb
+++ b/activerecord/lib/active_record/attribute_methods/write.rb
@@ -9,15 +9,19 @@ module ActiveRecord
 
       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
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 957027c1ee..94c6684700 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -83,7 +83,12 @@ module ActiveRecord
         @attribute_methods_mutex = Mutex.new
 
         # force attribute methods to be higher in inheritance hierarchy than other generated methods
-        generated_attribute_methods
+        generated_attribute_methods.const_set(:AttrNames, Module.new {
+          def self.const_missing(name)
+            const_set(name, [name.to_s.sub(/ATTR_/, '')].pack('h*').freeze)
+          end
+        })
+
         generated_feature_methods
       end