Skip to content

Commit

Permalink
Merge pull request #32047 from kamipo/backport-31827
Browse files Browse the repository at this point in the history
5-0-stable: PERF: Recover marshaling dump/load performance
  • Loading branch information
kamipo committed Feb 18, 2018
2 parents ca97282 + 5ea2544 commit 2bbac26
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 32 deletions.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/attribute_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module ActiveRecord
class AttributeSet # :nodoc:
delegate :fetch, to: :attributes
delegate :fetch, :except, to: :attributes

def initialize(attributes)
@attributes = attributes
Expand Down
46 changes: 24 additions & 22 deletions activerecord/lib/active_record/attribute_set/builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,30 @@
module ActiveRecord
class AttributeSet # :nodoc:
class Builder # :nodoc:
attr_reader :types, :always_initialized, :default
attr_reader :types, :default_attributes

def initialize(types, always_initialized = nil, &default)
def initialize(types, default_attributes = {})
@types = types
@always_initialized = always_initialized
@default = default
@default_attributes = default_attributes
end

def build_from_database(values = {}, additional_types = {})
if always_initialized && !values.key?(always_initialized)
values[always_initialized] = nil
end

attributes = LazyAttributeHash.new(types, values, additional_types, &default)
attributes = LazyAttributeHash.new(types, values, additional_types, default_attributes)
AttributeSet.new(attributes)
end
end
end

class LazyAttributeHash # :nodoc:
delegate :transform_values, :each_key, :fetch, to: :materialize
delegate :transform_values, :each_key, :fetch, :except, to: :materialize

def initialize(types, values, additional_types, &default)
def initialize(types, values, additional_types, default_attributes, delegate_hash = {})
@types = types
@values = values
@additional_types = additional_types
@materialized = false
@delegate_hash = {}
@default = default || proc {}
@delegate_hash = delegate_hash
@default_attributes = default_attributes
end

def key?(key)
Expand Down Expand Up @@ -79,15 +74,17 @@ def ==(other)
end

def marshal_dump
materialize
[@types, @values, @additional_types, @default_attributes, @delegate_hash]
end

def marshal_load(delegate_hash)
@delegate_hash = delegate_hash
@types = {}
@values = {}
@additional_types = {}
@materialized = true
def marshal_load(values)
if values.is_a?(Hash)
empty_hash = {}.freeze
initialize(empty_hash, empty_hash, empty_hash, empty_hash, values)
@materialized = true
else
initialize(*values)
end
end

def encode_with(coder)
Expand All @@ -100,7 +97,7 @@ def init_with(coder)

protected

attr_reader :types, :values, :additional_types, :delegate_hash, :default
attr_reader :types, :values, :additional_types, :delegate_hash, :default_attributes

def materialize
unless @materialized
Expand All @@ -123,7 +120,12 @@ def assign_default_value(name)
if value_present
delegate_hash[name] = Attribute.from_database(name, value, type)
elsif types.key?(name)
delegate_hash[name] = default.call(name) || Attribute.uninitialized(name, type)
attr = default_attributes[name]
if attr
delegate_hash[name] = attr.dup
else
delegate_hash[name] = Attribute.uninitialized(name, type)
end
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions activerecord/lib/active_record/model_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -332,11 +332,11 @@ def table_exists?
end

def attributes_builder # :nodoc:
@attributes_builder ||= AttributeSet::Builder.new(attribute_types, primary_key) do |name|
unless columns_hash.key?(name)
_default_attributes[name].dup
end
unless defined?(@attributes_builder) && @attributes_builder
defaults = _default_attributes.except(*(column_names - [primary_key]))
@attributes_builder = AttributeSet::Builder.new(attribute_types, defaults)
end
@attributes_builder
end

def columns_hash # :nodoc:
Expand Down
19 changes: 18 additions & 1 deletion activerecord/test/cases/attribute_set_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ class AttributeSetTest < ActiveRecord::TestCase
end

test "the primary_key is always initialized" do
builder = AttributeSet::Builder.new({ foo: Type::Integer.new }, :foo)
defaults = { foo: Attribute.from_user(:foo, nil, nil) }
builder = AttributeSet::Builder.new({ foo: Type::Integer.new }, defaults)
attributes = builder.build_from_database

assert attributes.key?(:foo)
Expand Down Expand Up @@ -218,6 +219,22 @@ def attributes_with_uninitialized_key
assert_equal({ foo: "1" }, attributes.to_hash)
end

test "marshaling dump/load legacy materialized attribute hash" do
builder = AttributeSet::Builder.new(foo: Type::String.new)
attributes = builder.build_from_database(foo: "1")

attributes.instance_variable_get(:@attributes).instance_eval do
class << self
def marshal_dump
materialize
end
end
end

attributes = Marshal.load(Marshal.dump(attributes))
assert_equal({ foo: "1" }, attributes.to_hash)
end

test "#accessed_attributes returns only attributes which have been read" do
builder = AttributeSet::Builder.new(foo: Type::Value.new, bar: Type::Value.new)
attributes = builder.build_from_database(foo: "1", bar: "2")
Expand Down
6 changes: 2 additions & 4 deletions activerecord/test/cases/validations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -156,17 +156,15 @@ def test_validators
end

def test_numericality_validation_with_mutation
Topic.class_eval do
klass = Class.new(Topic) do
attribute :wibble, :string
validates_numericality_of :wibble, only_integer: true
end

topic = Topic.new(wibble: '123-4567')
topic = klass.new(wibble: '123-4567')
topic.wibble.gsub!('-', '')

assert topic.valid?
ensure
Topic.reset_column_information
end

def test_numericality_validation_checks_against_raw_value
Expand Down

0 comments on commit 2bbac26

Please sign in to comment.