From 8f2bb58ba2a74975b737f7f7655247b447b7ac08 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 2 Feb 2018 07:52:33 +0900
Subject: [PATCH] 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
---
 .../lib/active_model/attribute_set/builder.rb | 20 ++++++++++---------
 activemodel/test/cases/attribute_set_test.rb  | 16 +++++++++++++++
 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")