Avoid double cast in types that only override cast
Follow-up to #44625. In #44625, the `SerializeCastValue` module was added to allow types to avoid a redundant call to `cast` when serializing a value for the database. Because it introduced a new method (`serialize_cast_value`) that was not part of the `ActiveModel::Type::Value` contract, it was designed to be opt-in. Furthermore, to guard against incompatible `serialize` and `serialize_cast_value` implementations in types that override `serialize` but (unintentionally) inherit `serialize_cast_value`, types were required to explicitly include the `SerializeCastValue` module to activate the optimization. i.e. It was not sufficient just to have `SerializeCastValue` in the ancestor chain. The `SerializeCastValue` module is not part of the public API, and there are no plans to change that, which meant user-created custom types could not benefit from this optimization. This commit changes the opt-in condition such that it is sufficient for the owner of the `serialize_cast_value` method to be the same or below the owner of the `serialize` method in the ancestor chain. This means a user-created type that only overrides `cast`, **not** `serialize`, will now benefit from the optimization. For example, a type like: ```ruby class DowncasedString < ActiveModel::Type::String def cast(value) super&.downcase end end ``` As demonstrated in the benchmark below, this commit does not change the current performance of the built-in Active Model types. However, for a simple custom type like `DowncasedString`, the performance of `value_for_database` is twice as fast. For types with more expensive `cast` operations, the improvement may be greater. **Benchmark** ```ruby # frozen_string_literal: true require "benchmark/ips" require "active_model" class DowncasedString < ActiveModel::Type::String def cast(value) super&.downcase end end ActiveModel::Type.register(:downcased_string, DowncasedString) VALUES = { my_big_integer: "123456", my_boolean: "true", my_date: "1999-12-31", my_datetime: "1999-12-31 12:34:56 UTC", my_decimal: "123.456", my_float: "123.456", my_immutable_string: "abcdef", my_integer: "123456", my_string: "abcdef", my_time: "1999-12-31T12:34:56.789-10:00", my_downcased_string: "AbcDef", } TYPES = VALUES.to_h { |name, value| [name, name.to_s.delete_prefix("my_").to_sym] } class MyModel include ActiveModel::API include ActiveModel::Attributes TYPES.each do |name, type| attribute name, type end end attribute_set = MyModel.new(VALUES).instance_variable_get(:@attributes) TYPES.each do |name, type| attribute = attribute_set[name.to_s] Benchmark.ips do |x| x.report(type.to_s) { attribute.value_for_database } end end ``` **Before** ``` big_integer 2.986M (± 1.2%) i/s - 15.161M in 5.078972s boolean 2.980M (± 1.1%) i/s - 15.074M in 5.059456s date 2.960M (± 1.1%) i/s - 14.831M in 5.011355s datetime 1.368M (± 0.9%) i/s - 6.964M in 5.092074s decimal 2.930M (± 1.2%) i/s - 14.911M in 5.089048s float 2.932M (± 1.3%) i/s - 14.713M in 5.018512s immutable_string 3.013M (± 1.3%) i/s - 15.239M in 5.058085s integer 1.603M (± 0.8%) i/s - 8.096M in 5.052046s string 2.977M (± 1.1%) i/s - 15.168M in 5.094874s time 1.338M (± 0.9%) i/s - 6.699M in 5.006046s downcased_string 1.394M (± 0.9%) i/s - 7.034M in 5.046972s ``` **After** ``` big_integer 3.016M (± 1.0%) i/s - 15.238M in 5.053005s boolean 2.965M (± 1.3%) i/s - 15.037M in 5.071921s date 2.924M (± 1.0%) i/s - 14.754M in 5.046294s datetime 1.435M (± 0.9%) i/s - 7.295M in 5.082498s decimal 2.950M (± 0.9%) i/s - 14.800M in 5.017225s float 2.964M (± 0.9%) i/s - 14.987M in 5.056405s immutable_string 2.907M (± 1.4%) i/s - 14.677M in 5.049194s integer 1.638M (± 0.9%) i/s - 8.227M in 5.022401s string 2.971M (± 1.0%) i/s - 14.891M in 5.011709s time 1.454M (± 0.9%) i/s - 7.384M in 5.079993s downcased_string 2.939M (± 0.9%) i/s - 14.872M in 5.061100s ```
This commit is contained in:
parent
110b495810
commit
28ebf3c81c
@ -1,3 +1,30 @@
|
||||
* Custom attribute types that inherit from Active Model built-in types and do
|
||||
not override the `serialize` method will now benefit from an optimization
|
||||
when serializing attribute values for the database.
|
||||
|
||||
For example, with a custom type like the following:
|
||||
|
||||
```ruby
|
||||
class DowncasedString < ActiveModel::Type::String
|
||||
def cast(value)
|
||||
super&.downcase
|
||||
end
|
||||
end
|
||||
|
||||
ActiveRecord::Type.register(:downcased_string, DowncasedString)
|
||||
|
||||
class User < ActiveRecord::Base
|
||||
attribute :email, :downcased_string
|
||||
end
|
||||
|
||||
user = User.new(email: "FooBar@example.com")
|
||||
```
|
||||
|
||||
Serializing the `email` attribute for the database will be roughly twice as
|
||||
fast. More expensive `cast` operations will likely see greater improvements.
|
||||
|
||||
*Jonathan Hefner*
|
||||
|
||||
* `has_secure_password` now supports password challenges via a
|
||||
`password_challenge` accessor and validation.
|
||||
|
||||
|
@ -21,8 +21,6 @@ module Type
|
||||
# All casting and serialization are performed in the same way as the
|
||||
# standard ActiveModel::Type::Integer type.
|
||||
class BigInteger < Integer
|
||||
include SerializeCastValue
|
||||
|
||||
def serialize_cast_value(value) # :nodoc:
|
||||
value
|
||||
end
|
||||
|
@ -10,8 +10,6 @@ module Type
|
||||
# - Empty strings are coerced to +nil+.
|
||||
# - All other values will be coerced to +true+.
|
||||
class Boolean < Value
|
||||
include SerializeCastValue
|
||||
|
||||
FALSE_VALUES = [
|
||||
false, 0,
|
||||
"0", :"0",
|
||||
@ -31,6 +29,10 @@ def serialize(value) # :nodoc:
|
||||
cast(value)
|
||||
end
|
||||
|
||||
def serialize_cast_value(value) # :nodoc:
|
||||
value
|
||||
end
|
||||
|
||||
private
|
||||
def cast_value(value)
|
||||
if value == ""
|
||||
|
@ -22,7 +22,6 @@ module Type
|
||||
# String values are parsed using the ISO 8601 date format. Any other values
|
||||
# are cast using their +to_date+ method, if it exists.
|
||||
class Date < Value
|
||||
include SerializeCastValue
|
||||
include Helpers::Timezone
|
||||
include Helpers::AcceptsMultiparameterTime.new
|
||||
|
||||
|
@ -38,19 +38,16 @@ module Type
|
||||
# attribute :start, :datetime, precision: 4
|
||||
# end
|
||||
class DateTime < Value
|
||||
include SerializeCastValue
|
||||
include Helpers::Timezone
|
||||
include Helpers::TimeValue
|
||||
include Helpers::AcceptsMultiparameterTime.new(
|
||||
defaults: { 4 => 0, 5 => 0 }
|
||||
)
|
||||
include Helpers::TimeValue
|
||||
|
||||
def type
|
||||
:datetime
|
||||
end
|
||||
|
||||
alias :serialize_cast_value :serialize_time_value # :nodoc:
|
||||
|
||||
private
|
||||
def cast_value(value)
|
||||
return apply_seconds_precision(value) unless value.is_a?(::String)
|
||||
|
@ -32,7 +32,6 @@ module Type
|
||||
# attribute :weight, :decimal, precision: 24
|
||||
# end
|
||||
class Decimal < Value
|
||||
include SerializeCastValue
|
||||
include Helpers::Numeric
|
||||
BIGDECIMAL_PRECISION = 18
|
||||
|
||||
|
@ -26,7 +26,6 @@ module Type
|
||||
# - <tt>"-Infinity"</tt> is cast to <tt>-Float::INFINITY</tt>.
|
||||
# - <tt>"NaN"</tt> is cast to <tt>Float::NAN</tt>.
|
||||
class Float < Value
|
||||
include SerializeCastValue
|
||||
include Helpers::Numeric
|
||||
|
||||
def type
|
||||
|
@ -6,7 +6,11 @@ module Helpers # :nodoc: all
|
||||
class AcceptsMultiparameterTime < Module
|
||||
module InstanceMethods
|
||||
def serialize(value)
|
||||
super(cast(value))
|
||||
serialize_cast_value(cast(value))
|
||||
end
|
||||
|
||||
def serialize_cast_value(value)
|
||||
value
|
||||
end
|
||||
|
||||
def cast(value)
|
||||
|
@ -8,6 +8,10 @@ def serialize(value)
|
||||
cast(value)
|
||||
end
|
||||
|
||||
def serialize_cast_value(value)
|
||||
value
|
||||
end
|
||||
|
||||
def cast(value)
|
||||
# Checks whether the value is numeric. Spaceship operator
|
||||
# will return nil if value is not numeric.
|
||||
|
@ -7,7 +7,7 @@ module ActiveModel
|
||||
module Type
|
||||
module Helpers # :nodoc: all
|
||||
module TimeValue
|
||||
def serialize(value)
|
||||
def serialize_cast_value(value)
|
||||
value = apply_seconds_precision(value)
|
||||
|
||||
if value.acts_like?(:time)
|
||||
@ -20,7 +20,6 @@ def serialize(value)
|
||||
|
||||
value
|
||||
end
|
||||
alias :serialize_time_value :serialize
|
||||
|
||||
def apply_seconds_precision(value)
|
||||
return value unless precision && value.respond_to?(:nsec)
|
||||
|
@ -33,8 +33,6 @@ module Type
|
||||
#
|
||||
# person.active # => "aye"
|
||||
class ImmutableString < Value
|
||||
include SerializeCastValue
|
||||
|
||||
def initialize(**args)
|
||||
@true = -(args.delete(:true)&.to_s || "t")
|
||||
@false = -(args.delete(:false)&.to_s || "f")
|
||||
@ -54,6 +52,10 @@ def serialize(value)
|
||||
end
|
||||
end
|
||||
|
||||
def serialize_cast_value(value) # :nodoc:
|
||||
value
|
||||
end
|
||||
|
||||
private
|
||||
def cast_value(value)
|
||||
case value
|
||||
|
@ -36,7 +36,6 @@ module Type
|
||||
# attribute :age, :integer, limit: 6
|
||||
# end
|
||||
class Integer < Value
|
||||
include SerializeCastValue
|
||||
include Helpers::Numeric
|
||||
|
||||
# Column storage size in bytes.
|
||||
|
@ -3,35 +3,38 @@
|
||||
module ActiveModel
|
||||
module Type
|
||||
module SerializeCastValue # :nodoc:
|
||||
def self.included(klass)
|
||||
unless klass.respond_to?(:included_serialize_cast_value)
|
||||
klass.singleton_class.attr_accessor :included_serialize_cast_value
|
||||
klass.silence_redefinition_of_method(:itself_if_class_included_serialize_cast_value)
|
||||
klass.attr_reader :itself_if_class_included_serialize_cast_value
|
||||
module DefaultImplementation
|
||||
def serialize_cast_value(value)
|
||||
value
|
||||
end
|
||||
klass.included_serialize_cast_value = true
|
||||
end
|
||||
|
||||
def self.included(klass)
|
||||
klass.include DefaultImplementation unless klass.method_defined?(:serialize_cast_value)
|
||||
end
|
||||
|
||||
def self.serialize(type, value)
|
||||
# Verify that `type.class` explicitly included SerializeCastValue.
|
||||
# Using `type.equal?(type.itself_if_...)` is a performant way to also
|
||||
# ensure that `type` is not just a DelegateClass instance (e.g.
|
||||
# ActiveRecord::Type::Serialized) unintentionally delegating
|
||||
# SerializeCastValue methods.
|
||||
if type.equal?((type.itself_if_class_included_serialize_cast_value rescue nil))
|
||||
if type.equal?((type.itself_if_serialize_cast_value_compatible rescue nil))
|
||||
type.serialize_cast_value(value)
|
||||
else
|
||||
type.serialize(value)
|
||||
end
|
||||
end
|
||||
|
||||
attr_reader :itself_if_serialize_cast_value_compatible
|
||||
|
||||
def initialize(...)
|
||||
@itself_if_class_included_serialize_cast_value = self if self.class.included_serialize_cast_value
|
||||
super
|
||||
@itself_if_serialize_cast_value_compatible = self if serialize_cast_value_compatible?
|
||||
end
|
||||
|
||||
def serialize_cast_value(value)
|
||||
value
|
||||
def serialize_cast_value_compatible?
|
||||
ancestors = self.class.ancestors
|
||||
ancestors.index(method(:serialize_cast_value).owner) <= ancestors.index(method(:serialize).owner)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -11,8 +11,6 @@ module Type
|
||||
# However, it accounts for mutable strings, so dirty tracking can properly
|
||||
# check if a string has changed.
|
||||
class String < ImmutableString
|
||||
include SerializeCastValue
|
||||
|
||||
def changed_in_place?(raw_old_value, new_value)
|
||||
if new_value.is_a?(::String)
|
||||
raw_old_value != new_value
|
||||
|
@ -38,12 +38,11 @@ module Type
|
||||
# attribute :start, :time, precision: 4
|
||||
# end
|
||||
class Time < Value
|
||||
include SerializeCastValue
|
||||
include Helpers::Timezone
|
||||
include Helpers::TimeValue
|
||||
include Helpers::AcceptsMultiparameterTime.new(
|
||||
defaults: { 1 => 2000, 2 => 1, 3 => 1, 4 => 0, 5 => 0 }
|
||||
)
|
||||
include Helpers::TimeValue
|
||||
|
||||
def type
|
||||
:time
|
||||
@ -68,8 +67,6 @@ def user_input_in_time_zone(value)
|
||||
super(value)
|
||||
end
|
||||
|
||||
alias :serialize_cast_value :serialize_time_value # :nodoc:
|
||||
|
||||
private
|
||||
def cast_value(value)
|
||||
return apply_seconds_precision(value) unless value.is_a?(::String)
|
||||
|
@ -9,49 +9,75 @@ class DoesNotIncludeModule
|
||||
def serialize(value)
|
||||
"serialize(#{value})"
|
||||
end
|
||||
|
||||
def serialize_cast_value(value)
|
||||
raise "this should never be called"
|
||||
end
|
||||
end
|
||||
|
||||
class IncludesModule < DoesNotIncludeModule
|
||||
include SerializeCastValue
|
||||
|
||||
def serialize_cast_value(value)
|
||||
super("serialize_cast_value(#{value})")
|
||||
"serialize_cast_value(#{super})"
|
||||
end
|
||||
end
|
||||
|
||||
test "calls #serialize when a class does not include SerializeCastValue" do
|
||||
assert_equal "serialize(foo)", SerializeCastValue.serialize(DoesNotIncludeModule.new, "foo")
|
||||
test "provides a default #serialize_cast_value implementation" do
|
||||
type = Class.new(DoesNotIncludeModule) { include SerializeCastValue }
|
||||
assert_equal "foo", type.new.serialize_cast_value("foo")
|
||||
end
|
||||
|
||||
test "calls #serialize_cast_value when a class directly includes SerializeCastValue" do
|
||||
assert_equal "serialize_cast_value(foo)", SerializeCastValue.serialize(IncludesModule.new, "foo")
|
||||
test "uses #serialize when a class does not include SerializeCastValue" do
|
||||
assert_serializes_using :serialize, DoesNotIncludeModule.new
|
||||
end
|
||||
|
||||
test "calls #serialize when a subclass does not directly include SerializeCastValue" do
|
||||
test "uses #serialize_cast_value when a class includes SerializeCastValue" do
|
||||
assert_serializes_using :serialize_cast_value, IncludesModule.new
|
||||
end
|
||||
|
||||
test "uses #serialize_cast_value when a subclass inherits both #serialize and #serialize_cast_value" do
|
||||
subclass = Class.new(IncludesModule)
|
||||
assert_equal "serialize(foo)", SerializeCastValue.serialize(subclass.new, "foo")
|
||||
assert_serializes_using :serialize_cast_value, subclass.new
|
||||
end
|
||||
|
||||
test "calls #serialize_cast_value when a subclass re-includes SerializeCastValue" do
|
||||
subclass = Class.new(IncludesModule)
|
||||
subclass.include SerializeCastValue
|
||||
assert_equal "serialize_cast_value(foo)", SerializeCastValue.serialize(subclass.new, "foo")
|
||||
test "uses #serialize when a subclass defines a newer #serialize implementation" do
|
||||
subclass = Class.new(IncludesModule) { def serialize(value); super; end }
|
||||
assert_serializes_using :serialize, subclass.new
|
||||
end
|
||||
|
||||
test "calls #serialize when a delegate class does not include SerializeCastValue" do
|
||||
test "uses #serialize_cast_value when a subclass defines a newer #serialize_cast_value implementation" do
|
||||
subclass = Class.new(IncludesModule) { def serialize_cast_value(value); super; end }
|
||||
assert_serializes_using :serialize_cast_value, subclass.new
|
||||
end
|
||||
|
||||
test "uses #serialize when a subclass defines a newer #serialize implementation via a module" do
|
||||
mod = Module.new { def serialize(value); super; end }
|
||||
subclass = Class.new(IncludesModule) { include mod }
|
||||
assert_serializes_using :serialize, subclass.new
|
||||
end
|
||||
|
||||
test "uses #serialize_cast_value when a subclass defines a newer #serialize_cast_value implementation via a module" do
|
||||
mod = Module.new { def serialize_cast_value(value); super; end }
|
||||
subclass = Class.new(IncludesModule) { include mod }
|
||||
assert_serializes_using :serialize_cast_value, subclass.new
|
||||
end
|
||||
|
||||
test "uses #serialize when a delegate class does not include SerializeCastValue" do
|
||||
delegate_class = DelegateClass(IncludesModule)
|
||||
assert_equal "serialize(foo)", SerializeCastValue.serialize(delegate_class.new(IncludesModule.new), "foo")
|
||||
assert_serializes_using :serialize, delegate_class.new(IncludesModule.new)
|
||||
end
|
||||
|
||||
test "calls #serialize_cast_value when a delegate class includes SerializeCastValue" do
|
||||
delegate_class = DelegateClass(IncludesModule)
|
||||
delegate_class.include SerializeCastValue
|
||||
assert_equal "serialize_cast_value(foo)", SerializeCastValue.serialize(delegate_class.new(IncludesModule.new), "foo")
|
||||
test "uses #serialize_cast_value when a delegate class prepends SerializeCastValue" do
|
||||
delegate_class = DelegateClass(IncludesModule) { prepend SerializeCastValue }
|
||||
assert_serializes_using :serialize_cast_value, delegate_class.new(IncludesModule.new)
|
||||
end
|
||||
|
||||
test "uses #serialize_cast_value when a delegate class subclass includes SerializeCastValue" do
|
||||
delegate_subclass = Class.new(DelegateClass(IncludesModule)) { include SerializeCastValue }
|
||||
assert_serializes_using :serialize_cast_value, delegate_subclass.new(IncludesModule.new)
|
||||
end
|
||||
|
||||
private
|
||||
def assert_serializes_using(method_name, type)
|
||||
assert_equal "#{method_name}(foo)", SerializeCastValue.serialize(type, "foo")
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -3,7 +3,6 @@
|
||||
module ActiveRecord
|
||||
module Type
|
||||
class Date < ActiveModel::Type::Date
|
||||
include ActiveModel::Type::SerializeCastValue
|
||||
include Internal::Timezone
|
||||
end
|
||||
end
|
||||
|
@ -3,7 +3,6 @@
|
||||
module ActiveRecord
|
||||
module Type
|
||||
class DateTime < ActiveModel::Type::DateTime
|
||||
include ActiveModel::Type::SerializeCastValue
|
||||
include Internal::Timezone
|
||||
end
|
||||
end
|
||||
|
@ -3,8 +3,6 @@
|
||||
module ActiveRecord
|
||||
module Type
|
||||
class DecimalWithoutScale < ActiveModel::Type::BigInteger # :nodoc:
|
||||
include ActiveModel::Type::SerializeCastValue
|
||||
|
||||
def type
|
||||
:decimal
|
||||
end
|
||||
|
@ -3,8 +3,6 @@
|
||||
module ActiveRecord
|
||||
module Type
|
||||
class Text < ActiveModel::Type::String # :nodoc:
|
||||
include ActiveModel::Type::SerializeCastValue
|
||||
|
||||
def type
|
||||
:text
|
||||
end
|
||||
|
@ -3,7 +3,6 @@
|
||||
module ActiveRecord
|
||||
module Type
|
||||
class Time < ActiveModel::Type::Time
|
||||
include ActiveModel::Type::SerializeCastValue
|
||||
include Internal::Timezone
|
||||
|
||||
class Value < DelegateClass(::Time) # :nodoc:
|
||||
|
@ -3,8 +3,6 @@
|
||||
module ActiveRecord
|
||||
module Type
|
||||
class UnsignedInteger < ActiveModel::Type::Integer # :nodoc:
|
||||
include ActiveModel::Type::SerializeCastValue
|
||||
|
||||
private
|
||||
def max_value
|
||||
super * 2
|
||||
|
Loading…
Reference in New Issue
Block a user