Skip to content

Commit

Permalink
Unify shapes of ActiveModel::Attributes
Browse files Browse the repository at this point in the history
`if defined?(@ivar)` a performance anti-pattern in Ruby 3.2+
even more so with YJIT.

This pattern cause the object shape to be inconsistent which slow
downs instance variable access.

```
ruby 3.2.1 (2023-02-08 revision 31819e82c8) [arm64-darwin22]
       #value (orig):  1811240.2 i/s
        #value (opt):  2045238.9 i/s - 1.13x  faster
```

```
ruby 3.2.1 (2023-02-08 revision 31819e82c8) +YJIT [arm64-darwin22]
       #value (orig):  4379180.3 i/s
        #value (opt):  6347280.7 i/s - 1.45x  faster
```

Benchmark: https://gist.github.com/casperisfine/872f0a486b5ccdf90d9feb830c76d9ad
  • Loading branch information
byroot committed Mar 31, 2023
1 parent 6879f89 commit d9713d6
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 40 deletions.
157 changes: 117 additions & 40 deletions activemodel/lib/active_model/attribute.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

module ActiveModel
class Attribute # :nodoc:
# We can't use an anonymous object token here, because Attribute instances may be
# serialized with Marshal, so it would restore a different object.
# We use a very specific symbol instead.
UNDEF = :__ActiveModel__Attribute__UNDEF__ # :nodoc:

@optimization_enabled = false
class << self
def from_database(name, value_before_type_cast, type, value = nil)
FromDatabase.new(name, value_before_type_cast, type, nil, value)
Expand All @@ -24,24 +30,122 @@ def null(name)
def uninitialized(name, type)
Uninitialized.new(name, type)
end

attr_reader :optimization_enabled

# This optimization requires to change the internal representation
# of the object, so we can't enable it unless Active Record new serialization format
# is enabled too.
def optimization_enabled=(enabled)
return if @optimization_enabled == enabled

if enabled
ModernMethods.instance_methods(false).each do |method_name|
define_method(method_name, ModernMethods.instance_method(method_name))
end
else
ModernMethods.instance_methods(false).each do |method_name|
remove_method(method_name) if method_defined?(method_name, false)
end
end

@optimization_enabled = enabled
end
end

attr_reader :name, :value_before_type_cast, :type

# This method should not be called directly.
# Use #from_database or #from_user
def initialize(name, value_before_type_cast, type, original_attribute = nil, value = nil)
@name = name
@value_before_type_cast = value_before_type_cast
@type = type
@original_attribute = original_attribute
@value = value unless value.nil?
module LegacyMethods # :nodoc:
# This method should not be called directly.
# Use #from_database or #from_user
def initialize(name, value_before_type_cast, type, original_attribute = nil, value = nil)
@name = name
@value_before_type_cast = value_before_type_cast
@type = type
@original_attribute = original_attribute
@value = value unless value.nil?
end

def value
# `defined?` is cheaper than `||=` when we get back falsy values
@value = type_cast(value_before_type_cast) unless defined?(@value)
@value
end

def has_been_read?
defined?(@value)
end

def value_for_database
if !defined?(@value_for_database) || type.changed_in_place?(@value_for_database, value)
@value_for_database = _value_for_database
end
@value_for_database
end

def init_with(coder)
@name = coder["name"]
@value_before_type_cast = coder["value_before_type_cast"]
@type = coder["type"]
@original_attribute = coder["original_attribute"]
@value = coder["value"] if coder.map.key?("value")
end

def encode_with(coder)
coder["name"] = name
coder["value_before_type_cast"] = value_before_type_cast unless value_before_type_cast.nil?
coder["type"] = type if type
coder["original_attribute"] = original_attribute if original_attribute
coder["value"] = value if defined?(@value)
end
end

def value
# `defined?` is cheaper than `||=` when we get back falsy values
@value = type_cast(value_before_type_cast) unless defined?(@value)
@value
include LegacyMethods

module ModernMethods # :nodoc:
# This method should not be called directly.
# Use #from_database or #from_user
def initialize(name, value_before_type_cast, type, original_attribute = nil, value = nil)
@name = name
@value_before_type_cast = value_before_type_cast
@type = type
@original_attribute = original_attribute
@value = value.nil? ? UNDEF : value
@value_for_database = UNDEF
end

def value
@value = type_cast(value_before_type_cast) if UNDEF == @value
@value
end

def has_been_read?
UNDEF == @value
end

def value_for_database
if UNDEF == @value_for_database || type.changed_in_place?(@value_for_database, value)
@value_for_database = _value_for_database
end
@value_for_database
end

def init_with(coder)
@name = coder["name"]
@value_before_type_cast = coder["value_before_type_cast"]
@type = coder["type"]
@original_attribute = coder["original_attribute"]
@value = coder.map.key?("value") ? coder["value"] : UNDEF
@value_for_database = UNDEF
end

def encode_with(coder)
coder["name"] = name
coder["value_before_type_cast"] = value_before_type_cast unless value_before_type_cast.nil?
coder["type"] = type if type
coder["original_attribute"] = original_attribute if original_attribute
coder["value"] = value unless UNDEF.equal?(@value)
end
end

def original_value
Expand All @@ -52,13 +156,6 @@ def original_value
end
end

def value_for_database
if !defined?(@value_for_database) || type.changed_in_place?(@value_for_database, value)
@value_for_database = _value_for_database
end
@value_for_database
end

def serializable?(&block)
type.serializable?(value, &block)
end
Expand Down Expand Up @@ -108,10 +205,6 @@ def came_from_user?
false
end

def has_been_read?
defined?(@value)
end

def ==(other)
self.class == other.class &&
name == other.name &&
Expand All @@ -124,22 +217,6 @@ def hash
[self.class, name, value_before_type_cast, type].hash
end

def init_with(coder)
@name = coder["name"]
@value_before_type_cast = coder["value_before_type_cast"]
@type = coder["type"]
@original_attribute = coder["original_attribute"]
@value = coder["value"] if coder.map.key?("value")
end

def encode_with(coder)
coder["name"] = name
coder["value_before_type_cast"] = value_before_type_cast unless value_before_type_cast.nil?
coder["type"] = type if type
coder["original_attribute"] = original_attribute if original_attribute
coder["value"] = value if defined?(@value)
end

def original_value_for_database
if assigned?
original_attribute.original_value_for_database
Expand All @@ -153,7 +230,7 @@ def original_value_for_database
alias :assigned? :original_attribute

def initialize_dup(other)
if defined?(@value) && @value.duplicable?
if has_been_read? && @value.duplicable?
@value = @value.dup
end
end
Expand Down
2 changes: 2 additions & 0 deletions activerecord/lib/active_record/marshalling.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ def format_version=(version)
case version
when 6.1
Methods.remove_method(:marshal_dump) if Methods.method_defined?(:marshal_dump)
ActiveModel::Attribute.optimization_enabled = false
when 7.1
Methods.alias_method(:marshal_dump, :_marshal_dump_7_1)
ActiveModel::Attribute.optimization_enabled = true
else
raise ArgumentError, "Unknown marshalling format: #{version.inspect}"
end
Expand Down

0 comments on commit d9713d6

Please sign in to comment.