Skip to content

Commit

Permalink
Avoid redundant to_s in internal attribute API
Browse files Browse the repository at this point in the history
Redundant `to_s` has a few overhead. Especially private methods are not
intend to be passed user input directly so it should be passed always
string.

Removing redundant `to_s` makes attribute methods about 10% faster.

```ruby
ActiveRecord::Schema.define do
  create_table :users, force: true do |t|
  end
end

class User < ActiveRecord::Base
  def fast_read_attribute(attr_name, &block)
    @attributes.fetch_value(attr_name, &block)
  end
end

user = User.create!

Benchmark.ips do |x|
  x.report("user._read_attribute('id')") { user._read_attribute("id") }
  x.report("user.fast_read_attribute('id')") { user.fast_read_attribute("id") }
end
```

```
Warming up --------------------------------------
user._read_attribute('id')
                       272.151k i/100ms
user.fast_read_attribute('id')
                       283.518k i/100ms
Calculating -------------------------------------
user._read_attribute('id')
                          2.699M (± 1.3%) i/s -     13.608M in   5.042846s
user.fast_read_attribute('id')
                          2.988M (± 1.2%) i/s -     15.026M in   5.029056s
```
  • Loading branch information
kamipo committed Jun 4, 2020
1 parent 95154c9 commit 7834363
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 26 deletions.
17 changes: 11 additions & 6 deletions activemodel/lib/active_model/attributes.rb
Expand Up @@ -124,20 +124,25 @@ def write_attribute(attr_name, value)
name = attr_name.to_s
name = self.class.attribute_aliases[name] || name

@attributes.write_from_user(name, value)
_write_attribute(name, value)
end

def _write_attribute(attr_name, value)
@attributes.write_from_user(attr_name, value)
value
end
alias :attribute= :_write_attribute

def attribute(attr_name)
def read_attribute(attr_name)
name = attr_name.to_s
name = self.class.attribute_aliases[name] || name

@attributes.fetch_value(name)
_read_attribute(name)
end

# Dispatch target for <tt>*=</tt> attribute methods.
def attribute=(attribute_name, value)
write_attribute(attribute_name, value)
def _read_attribute(attr_name)
@attributes.fetch_value(attr_name)
end
alias :attribute :_read_attribute
end
end
6 changes: 4 additions & 2 deletions activerecord/lib/active_record/attribute_methods.rb
Expand Up @@ -304,6 +304,7 @@ def attributes
# person.attribute_for_inspect(:tag_ids)
# # => "[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]"
def attribute_for_inspect(attr_name)
attr_name = attr_name.to_s
value = _read_attribute(attr_name)
format_for_inspect(value)
end
Expand All @@ -323,8 +324,9 @@ def attribute_for_inspect(attr_name)
# task.is_done = true
# task.attribute_present?(:title) # => true
# task.attribute_present?(:is_done) # => true
def attribute_present?(attribute)
value = _read_attribute(attribute)
def attribute_present?(attr_name)
attr_name = attr_name.to_s
value = _read_attribute(attr_name)
!value.nil? && !(value.respond_to?(:empty?) && value.empty?)
end

Expand Down
Expand Up @@ -46,7 +46,7 @@ module BeforeTypeCast
# task.read_attribute_before_type_cast('completed_on') # => "2012-10-21"
# task.read_attribute_before_type_cast(:completed_on) # => "2012-10-21"
def read_attribute_before_type_cast(attr_name)
@attributes[attr_name.to_s].value_before_type_cast
attribute_before_type_cast(attr_name.to_s)
end

# Returns a hash of attributes before typecasting and deserialization.
Expand All @@ -65,12 +65,12 @@ def attributes_before_type_cast

private
# Dispatch target for <tt>*_before_type_cast</tt> attribute methods.
def attribute_before_type_cast(attribute_name)
read_attribute_before_type_cast(attribute_name)
def attribute_before_type_cast(attr_name)
@attributes[attr_name].value_before_type_cast
end

def attribute_came_from_user?(attribute_name)
@attributes[attribute_name].came_from_user?
def attribute_came_from_user?(attr_name)
@attributes[attr_name].came_from_user?
end
end
end
Expand Down
7 changes: 2 additions & 5 deletions activerecord/lib/active_record/attribute_methods/query.rb
Expand Up @@ -31,11 +31,8 @@ def query_attribute(attr_name)
end
end

private
# Dispatch target for <tt>*?</tt> attribute methods.
def attribute?(attribute_name)
query_attribute(attribute_name)
end
alias :attribute? :query_attribute
private :attribute?
end
end
end
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/attribute_methods/read.rb
Expand Up @@ -33,7 +33,7 @@ def read_attribute(attr_name, &block)
# This method exists to avoid the expensive primary_key check internally, without
# breaking compatibility with the read_attribute API
def _read_attribute(attr_name, &block) # :nodoc
@attributes.fetch_value(attr_name.to_s, &block)
@attributes.fetch_value(attr_name, &block)
end

alias :attribute :_read_attribute
Expand Down
12 changes: 5 additions & 7 deletions activerecord/lib/active_record/attribute_methods/write.rb
Expand Up @@ -37,20 +37,18 @@ def write_attribute(attr_name, value)
# This method exists to avoid the expensive primary_key check internally, without
# breaking compatibility with the write_attribute API
def _write_attribute(attr_name, value) # :nodoc:
@attributes.write_from_user(attr_name.to_s, value)
@attributes.write_from_user(attr_name, value)
value
end

alias :attribute= :_write_attribute
private :attribute=

private
def write_attribute_without_type_cast(attr_name, value)
@attributes.write_cast_value(attr_name.to_s, value)
@attributes.write_cast_value(attr_name, value)
value
end

# Dispatch target for <tt>*=</tt> attribute methods.
def attribute=(attribute_name, value)
_write_attribute(attribute_name, value)
end
end
end
end

0 comments on commit 7834363

Please sign in to comment.