aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2018-02-02 07:52:33 +0900
committerGitHub <noreply@github.com>2018-02-02 07:52:33 +0900
commit8f2bb58ba2a74975b737f7f7655247b447b7ac08 (patch)
treedd227ffcfbb88c51077933ea4594f80fe894c70f
parent2417f3c53f3e5fb1fbc1f8ba3a75ce65349dc11c (diff)
downloadrails-8f2bb58ba2a74975b737f7f7655247b447b7ac08.tar.gz
rails-8f2bb58ba2a74975b737f7f7655247b447b7ac08.tar.bz2
rails-8f2bb58ba2a74975b737f7f7655247b447b7ac08.zip
PERF: Recover marshaling dump/load performance (#31827)
* PERF: Recover marshaling dump/load performance This performance regression which is described in #30680 was caused by f0ddf87 due to force materialized `LazyAttributeHash`. Since 95b86e5, default proc has been removed in the class, so it is no longer needed that force materialized. Avoiding force materialized will recover marshaling dump/load performance. Benchmark: https://gist.github.com/blimmer/1360ea51cd3147bae8aeb7c6d09bff17 Before: ``` it took 0.6248569069430232 seconds to unmarshal the objects Total allocated: 38681544 bytes (530060 objects) allocated memory by class ----------------------------------- 12138848 Hash 10542384 String 7920000 ActiveModel::Attribute::Uninitialized 5600000 ActiveModel::Attribute::FromDatabase 1200000 Foo 880000 ActiveModel::LazyAttributeHash 400000 ActiveModel::AttributeSet 80 Integer 72 ActiveRecord::ConnectionAdapters::SQLite3Adapter::SQLite3Integer 40 ActiveModel::Type::String 40 ActiveRecord::Type::DateTime 40 Object 40 Range allocated objects by class ----------------------------------- 250052 String 110000 ActiveModel::Attribute::Uninitialized 70001 Hash 70000 ActiveModel::Attribute::FromDatabase 10000 ActiveModel::AttributeSet 10000 ActiveModel::LazyAttributeHash 10000 Foo 2 Integer 1 ActiveModel::Type::String 1 ActiveRecord::ConnectionAdapters::SQLite3Adapter::SQLite3Integer 1 ActiveRecord::Type::DateTime 1 Object 1 Range ``` After: ``` it took 0.1660824950085953 seconds to unmarshal the objects Total allocated: 13883811 bytes (220090 objects) allocated memory by class ----------------------------------- 5743371 String 4940008 Hash 1200000 Foo 880000 ActiveModel::LazyAttributeHash 720000 Array 400000 ActiveModel::AttributeSet 80 ActiveModel::Attribute::FromDatabase 80 Integer 72 ActiveRecord::ConnectionAdapters::SQLite3Adapter::SQLite3Integer 40 ActiveModel::Type::String 40 ActiveModel::Type::Value 40 ActiveRecord::Type::DateTime 40 Object 40 Range allocated objects by class ----------------------------------- 130077 String 50004 Hash 10000 ActiveModel::AttributeSet 10000 ActiveModel::LazyAttributeHash 10000 Array 10000 Foo 2 Integer 1 ActiveModel::Attribute::FromDatabase 1 ActiveModel::Type::String 1 ActiveModel::Type::Value 1 ActiveRecord::ConnectionAdapters::SQLite3Adapter::SQLite3Integer 1 ActiveRecord::Type::DateTime 1 Object 1 Range ``` Fixes #30680. * Keep the `@delegate_hash` to avoid to lose any mutations that have been made to the record
-rw-r--r--activemodel/lib/active_model/attribute_set/builder.rb20
-rw-r--r--activemodel/test/cases/attribute_set_test.rb16
2 files changed, 27 insertions, 9 deletions
diff --git a/activemodel/lib/active_model/attribute_set/builder.rb b/activemodel/lib/active_model/attribute_set/builder.rb
index 758eb830fc..bf2d06b48a 100644
--- a/activemodel/lib/active_model/attribute_set/builder.rb
+++ b/activemodel/lib/active_model/attribute_set/builder.rb
@@ -22,12 +22,12 @@ module ActiveModel
class LazyAttributeHash # :nodoc:
delegate :transform_values, :each_key, :each_value, :fetch, :except, to: :materialize
- def initialize(types, values, additional_types, default_attributes)
+ def initialize(types, values, additional_types, default_attributes, delegate_hash = {})
@types = types
@values = values
@additional_types = additional_types
@materialized = false
- @delegate_hash = {}
+ @delegate_hash = delegate_hash
@default_attributes = default_attributes
end
@@ -76,15 +76,17 @@ module ActiveModel
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
protected
diff --git a/activemodel/test/cases/attribute_set_test.rb b/activemodel/test/cases/attribute_set_test.rb
index 9b323dae9d..b868dba743 100644
--- a/activemodel/test/cases/attribute_set_test.rb
+++ b/activemodel/test/cases/attribute_set_test.rb
@@ -217,6 +217,22 @@ module ActiveModel
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")