Lower the memory allocations

It prevents the repeating hashes creation during the calculation of `object` and `object_changes`.

The real-life case, discovered with a help of `memory_profiler` gem:
1. It saves `~35Mb` per bulk of 100 typical objects like `User`;
2. It saves `~875Mb` per Sidekiq process full of bulk jobs like P1,
   under default concurrency 25.

CHANGELOG.md

@@ -18,6 +18,13 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 - [#1184](/~https://github.com/paper-trail-gem/paper_trail/pull/1184) -
   No need to calculate previous values of skipped attributes
 
+- [#1188](/~https://github.com/paper-trail-gem/paper_trail/pull/1188) -
+  Optimized the memory allocations during the building of every particular
+  Version object. That can help a lot for heavy bulk processing.
+  In additional we advise to use `json[b]` DB types for `object`
+  and `object_changes` Version columns, in order to reach best possible
+  RAM performance.
+
 ## 10.2.0 (2019-01-31)
 
 ### Breaking Changes

lib/paper_trail/events/base.rb

@@ -60,15 +60,28 @@ def attribute_changed_in_latest_version?(attr_name)
 
       # @api private
       def nonskipped_attributes_before_change(is_touch)
-        record_attributes = @record.attributes.except(*@record.paper_trail_options[:skip])
+        cache_changed_attributes do
+          record_attributes = @record.attributes.except(*@record.paper_trail_options[:skip])
 
-        Hash[record_attributes.map do |k, v|
-          if @record.class.column_names.include?(k)
-            [k, attribute_in_previous_version(k, is_touch)]
-          else
-            [k, v]
+          record_attributes.each_key do |k|
+            if @record.class.column_names.include?(k)
+              record_attributes[k] = attribute_in_previous_version(k, is_touch)
+            end
           end
-        end]
+        end
+      end
+
+      # Rails 5.1 changed the API of `ActiveRecord::Dirty`.
+      # @api private
+      def cache_changed_attributes
+        if RAILS_GTE_5_1
+          # Everything works fine as it is
+          yield
+        else
+          # Any particular call to `changed_attributes` produces the huge memory allocation.
+          # Lets use the generic AR workaround for that.
+          @record.send(:cache_changed_attributes) { yield }
+        end
       end
 
       # Rails 5.1 changed the API of `ActiveRecord::Dirty`. See
@@ -110,18 +123,20 @@ def changed_and_not_ignored
 
       # @api private
       def changed_in_latest_version
-        changes_in_latest_version.keys
+        @changed_in_latest_version ||= changes_in_latest_version.keys
       end
 
       # Rails 5.1 changed the API of `ActiveRecord::Dirty`. See
       # /~https://github.com/paper-trail-gem/paper_trail/pull/899
       #
       # @api private
       def changes_in_latest_version
-        if @in_after_callback && RAILS_GTE_5_1
-          @record.saved_changes
-        else
-          @record.changes
+        @changes_in_latest_version ||= begin
+          if @in_after_callback && RAILS_GTE_5_1
+            @record.saved_changes
+          else
+            @record.changes
+          end
         end
       end
 
@@ -202,16 +217,18 @@ def notable_changes
 
       # @api private
       def notably_changed
-        only = @record.paper_trail_options[:only].dup
-        # Remove Hash arguments and then evaluate whether the attributes (the
-        # keys of the hash) should also get pushed into the collection.
-        only.delete_if do |obj|
-          obj.is_a?(Hash) &&
-            obj.each { |attr, condition|
-              only << attr if condition.respond_to?(:call) && condition.call(@record)
-            }
+        @notably_changed ||= begin
+          only = @record.paper_trail_options[:only].dup
+          # Remove Hash arguments and then evaluate whether the attributes (the
+          # keys of the hash) should also get pushed into the collection.
+          only.delete_if do |obj|
+            obj.is_a?(Hash) &&
+              obj.each { |attr, condition|
+                only << attr if condition.respond_to?(:call) && condition.call(@record)
+              }
+          end
+          only.empty? ? changed_and_not_ignored : (changed_and_not_ignored & only)
         end
-        only.empty? ? changed_and_not_ignored : (changed_and_not_ignored & only)
       end
 
       # Returns hash of attributes (with appropriate attributes serialized),
@@ -285,7 +302,7 @@ def serialize_object_changes(changes)
         AttributeSerializers::ObjectChangesAttribute.
           new(@record.class).
           serialize(changes)
-        changes.to_hash
+        changes
       end
     end
   end

lib/paper_trail/events/create.rb

@@ -13,6 +13,7 @@ class Create < Base
       # @api private
       def data
         data = {
+          item: @record,
           event: @record.paper_trail_event || "create",
           whodunnit: PaperTrail.request.whodunnit
         }

lib/paper_trail/events/update.rb

@@ -25,6 +25,7 @@ def initialize(record, in_after_callback, is_touch, force_changes)
       # @api private
       def data
         data = {
+          item: @record,
           event: @record.paper_trail_event || "update",
           whodunnit: PaperTrail.request.whodunnit
         }

lib/paper_trail/record_trail.rb

@@ -67,14 +67,25 @@ def previous_version
 
     def record_create
       return unless enabled?
-      event = Events::Create.new(@record, true)
+
+      build_version_on_create(in_after_callback: true).tap do |version|
+        version.save!
+        versions.reset
+      end
+    end
+
+    # Returns the ready-for-save Version object built for a new record.
+    # Can be useful for manual Version saving, for instance as a part of
+    # bulk processing (/~https://github.com/zdennis/activerecord-import).
+    #
+    # @api public
+    def build_version_on_create(in_after_callback:)
+      event = Events::Create.new(@record, in_after_callback)
 
       # Merge data from `Event` with data from PT-AT. We no longer use
       # `data_for_create` but PT-AT still does.
-      data = event.data.merge(data_for_create)
-
-      versions_assoc = @record.send(@record.class.versions_association_name)
-      versions_assoc.create!(data)
+      data = event.data.merge!(data_for_create)
+      @record.class.paper_trail.version_class.new(data)
     end
 
     # PT-AT extends this method to add its transaction id.
@@ -117,24 +128,36 @@ def data_for_destroy
     # @api private
     # @return - The created version object, so that plugins can use it, e.g.
     # paper_trail-association_tracking
-    def record_update(force:, in_after_callback:, is_touch:)
+    def record_update(build_version_params)
       return unless enabled?
-      event = Events::Update.new(@record, in_after_callback, is_touch, nil)
-      return unless force || event.changed_notably?
 
-      # Merge data from `Event` with data from PT-AT. We no longer use
-      # `data_for_update` but PT-AT still does.
-      data = event.data.merge(data_for_update)
+      version = build_version_on_update(build_version_params)
+      return unless version
 
-      versions_assoc = @record.send(@record.class.versions_association_name)
-      version = versions_assoc.create(data)
+      version.save
       if version.errors.any?
         log_version_errors(version, :update)
       else
+        versions.reset
         version
       end
     end
 
+    # Returns the ready-for-save Version object built for existing record.
+    # Can be useful for manual Version saving, for instance as a part of
+    # bulk processing (/~https://github.com/zdennis/activerecord-import).
+    #
+    # @api public
+    def build_version_on_update(force:, in_after_callback:, is_touch:)
+      event = Events::Update.new(@record, in_after_callback, is_touch, nil)
+      return unless force || event.changed_notably?
+
+      # Merge data from `Event` with data from PT-AT. We no longer use
+      # `data_for_update` but PT-AT still does.
+      data = event.data.merge!(data_for_update)
+      @record.class.paper_trail.version_class.new(data)
+    end
+
     # PT-AT extends this method to add its transaction id.
     #
     # @api private

lib/paper_trail/serializers/yaml.rb

@@ -13,6 +13,7 @@ def load(string)
       end
 
       def dump(object)
+        object = object.to_hash if object.is_a?(HashWithIndifferentAccess)
         ::YAML.dump object
       end
 

paper_trail.gemspec

@@ -35,6 +35,7 @@ has been destroyed.
   s.add_development_dependency "byebug", "~> 10.0"
   s.add_development_dependency "ffaker", "~> 2.8"
   s.add_development_dependency "generator_spec", "~> 0.9.4"
+  s.add_development_dependency "memory_profiler", "~> 0.9.12"
   s.add_development_dependency "mysql2", "~> 0.5.2"
   s.add_development_dependency "paper_trail-association_tracking", "~> 2.0.0"
   s.add_development_dependency "pg", "~> 1.0"

spec/models/person_spec.rb

@@ -164,15 +164,15 @@
       bicycle.update(name: "BMX 2.0")
       person.update(name: "Peter")
 
-      expect(person.reload.versions.length).to(eq(3))
+      expect(person.versions.length).to(eq(3))
 
       # These will work when PT-AT adds support for the new `item_subtype` column
       #
       # - /~https://github.com/westonganger/paper_trail-association_tracking/pull/5
       # - /~https://github.com/paper-trail-gem/paper_trail/pull/1143
       # - /~https://github.com/paper-trail-gem/paper_trail/issues/594
       #
-      # second_version = person.reload.versions.second.reify(has_one: true)
+      # second_version = person.versions.second.reify(has_one: true)
       # expect(second_version.car.name).to(eq("BMW 325"))
       # expect(second_version.bicycle.name).to(eq("BMX 1.0"))
     end

spec/models/pet_spec.rb

@@ -21,9 +21,9 @@
     cat.update(name: "Sylvester")
     person.update(name: "Peter")
 
-    expect(person.reload.versions.length).to(eq(3))
+    expect(person.versions.length).to(eq(3))
 
-    second_version = person.reload.versions.second.reify(has_many: true)
+    second_version = person.versions.second.reify(has_many: true)
     expect(second_version.pets.length).to(eq(2))
     expect(second_version.animals.length).to(eq(2))
     expect(second_version.animals.map { |a| a.class.name }).to(eq(%w[Dog Cat]))
@@ -33,7 +33,7 @@
     expect(second_version.animals.second.name).to(eq("Garfield"))
     expect(second_version.cats.first.name).to(eq("Garfield"))
 
-    last_version = person.reload.versions.last.reify(has_many: true)
+    last_version = person.versions.last.reify(has_many: true)
     expect(last_version.pets.length).to(eq(2))
     expect(last_version.animals.length).to(eq(2))
     expect(last_version.animals.map { |a| a.class.name }).to(eq(%w[Dog Cat]))

spec/paper_trail/association_reify_error_behaviour/error.rb → spec/paper_trail/association_reify_error_behaviour/error_spec.rb

@@ -24,12 +24,12 @@
       bicycle.update(name: "BMX 2.0")
       person.update(name: "Peter")
 
-      expect(person.reload.versions.length).to(eq(3))
+      expect(person.versions.length).to(eq(3))
 
       expect {
-        person.reload.versions.second.reify(has_one: true)
+        person.versions.second.reify(has_one: true)
       }.to(
-        raise_error(::PaperTrail::Reifiers::HasOne::FoundMoreThanOne) do |err|
+        raise_error(::PaperTrailAssociationTracking::Reifiers::HasOne::FoundMoreThanOne) do |err|
           expect(err.message.squish).to match(
             /Expected to find one Vehicle, but found 2/
           )

spec/paper_trail/association_reify_error_behaviour/ignore.rb → spec/paper_trail/association_reify_error_behaviour/ignore_spec.rb

@@ -24,13 +24,13 @@
       thing.update(name: "BMX 2.0")
       person.update(name: "Peter")
 
-      expect(person.reload.versions.length).to(eq(3))
+      expect(person.versions.length).to(eq(3))
 
       logger = person.versions.first.logger
 
       allow(logger).to receive(:warn)
 
-      person.reload.versions.second.reify(has_one: true)
+      person.versions.second.reify(has_one: true)
 
       expect(logger).not_to(
         have_received(:warn).with(/Unable to reify has_one association/)

spec/paper_trail/association_reify_error_behaviour/warn.rb → spec/paper_trail/association_reify_error_behaviour/warn_spec.rb

@@ -24,13 +24,13 @@
       thing.update(name: "BMX 2.0")
       person.update(name: "Peter")
 
-      expect(person.reload.versions.length).to(eq(3))
+      expect(person.versions.length).to(eq(3))
 
       logger = person.versions.first.logger
 
       allow(logger).to receive(:warn)
 
-      person.reload.versions.second.reify(has_one: true)
+      person.versions.second.reify(has_one: true)
 
       expect(logger).to(
         have_received(:warn).with(/Unable to reify has_one association/).twice

spec/paper_trail/cleaner_spec.rb

@@ -58,7 +58,7 @@ module PaperTrail
           expect(animal.paper_trail.versions_between(date, (date + 1.day)).size).to(eq(3))
           PaperTrail.clean_versions!(date: date)
           expect(PaperTrail::Version.count).to(eq(8))
-          expect(animal.versions.reload.size).to(eq(2))
+          expect(animal.versions.size).to(eq(2))
           expect(animal.versions.first.created_at.to_date).to(eq(date))
           # Why use `equal?` here instead of something less strict?
           # Doesn't `to_date` always produce a new date object?
@@ -110,7 +110,6 @@ module PaperTrail
               date = animal.versions.first.created_at.to_date
               PaperTrail.clean_versions!(date: date, keeping: 2)
               [animal, dog].each do |animal|
-                animal.versions.reload
                 expect(animal.versions.size).to(eq(3))
                 expect(animal.versions.between(date, (date + 1.day)).size).to(eq(2))
               end
@@ -122,7 +121,6 @@ module PaperTrail
             it "restrict cleaning properly" do
               date = animal.versions.first.created_at.to_date
               PaperTrail.clean_versions!(date: date, item_id: dog.id)
-              dog.versions.reload
               expect(dog.versions.size).to(eq(2))
               expect(dog.versions.between(date, (date + 1.day)).size).to(eq(1))
               expect(PaperTrail::Version.count).to(eq(9))
@@ -133,7 +131,6 @@ module PaperTrail
             it "restrict cleaning properly" do
               date = animal.versions.first.created_at.to_date
               PaperTrail.clean_versions!(date: date, item_id: dog.id, keeping: 2)
-              dog.versions.reload
               expect(dog.versions.size).to(eq(3))
               expect(dog.versions.between(date, (date + 1.day)).size).to(eq(2))
               expect(PaperTrail::Version.count).to(eq(10))