From dcf43e3641d36b364d93830acfbf9dd14618f6ea Mon Sep 17 00:00:00 2001
From: Andrew Williams <awilliams@2moro.com.au>
Date: Thu, 3 Mar 2011 17:58:57 +1030
Subject: [PATCH] some speed optimisations. added 'use_dirty' configuration
 variable

---
 Gemfile.lock                                | 18 +++---
 b/couchrest.rb                              | 51 +++++++++++++---
 lib/couchrest/model/base.rb                 | 13 ++--
 lib/couchrest/model/casted_by.rb            | 23 +++++++
 lib/couchrest/model/casted_hash.rb          |  1 +
 lib/couchrest/model/casted_model.rb         |  2 +-
 lib/couchrest/model/configuration.rb        |  4 +-
 lib/couchrest/model/dirty.rb                | 18 ++++--
 lib/couchrest/model/extended_attachments.rb | 13 ++--
 lib/couchrest/model/persistence.rb          |  2 +-
 lib/couchrest/model/properties.rb           | 34 ++++-------
 lib/couchrest_model.rb                      |  6 +-
 spec/couchrest/dirty_spec.rb                | 68 ++++++++++++++++++++-
 13 files changed, 184 insertions(+), 69 deletions(-)
 create mode 100644 lib/couchrest/model/casted_by.rb

diff --git a/Gemfile.lock b/Gemfile.lock
index 312c498..8ecf864 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -12,9 +12,9 @@ GEM
   remote: http://rubygems.org/
   specs:
     abstract (1.0.0)
-    actionpack (3.0.5)
-      activemodel (= 3.0.5)
-      activesupport (= 3.0.5)
+    actionpack (3.0.4)
+      activemodel (= 3.0.4)
+      activesupport (= 3.0.4)
       builder (~> 2.1.2)
       erubis (~> 2.6.6)
       i18n (~> 0.4)
@@ -22,11 +22,11 @@ GEM
       rack-mount (~> 0.6.13)
       rack-test (~> 0.5.7)
       tzinfo (~> 0.3.23)
-    activemodel (3.0.5)
-      activesupport (= 3.0.5)
+    activemodel (3.0.4)
+      activesupport (= 3.0.4)
       builder (~> 2.1.2)
       i18n (~> 0.4)
-    activesupport (3.0.5)
+    activesupport (3.0.4)
     builder (2.1.2)
     couchrest (1.0.1)
       json (>= 1.4.6)
@@ -43,9 +43,9 @@ GEM
       rack (>= 1.0.0)
     rack-test (0.5.7)
       rack (>= 1.0)
-    railties (3.0.5)
-      actionpack (= 3.0.5)
-      activesupport (= 3.0.5)
+    railties (3.0.4)
+      actionpack (= 3.0.4)
+      activesupport (= 3.0.4)
       rake (>= 0.8.7)
       thor (~> 0.14.4)
     rake (0.8.7)
diff --git a/b/couchrest.rb b/b/couchrest.rb
index 698a6d4..7ab8fb1 100644
--- a/b/couchrest.rb
+++ b/b/couchrest.rb
@@ -20,13 +20,27 @@ class BenchmarkModel < CouchRest::Model::Base
   property :casted_list, [BenchmarkCasted]
 end
 
-begin
-  n = 50000
+# set dirty configuration, return previous configuration setting
+def set_dirty(value)
+  orig = nil
+  CouchRest::Model::Base.configure do |config|
+    orig = config.use_dirty
+    config.use_dirty = value
+  end
+  BenchmarkModel.instance_eval do
+    self.use_dirty = value
+  end
+  orig
+end
+
+def run_benchmark
+  n = 50000     # property operation count
+  db_n = 1000   # database operation count
   b = BenchmarkModel.new
 
-  Benchmark.bm(25) do |x|
+  Benchmark.bm(30) do |x|
 
-    # assigning
+    # property assigning
 
     x.report("assign string:") do
       n.times { b.string = "test" }
@@ -44,7 +58,7 @@ begin
       n.times { b.casted_list = [{ 'name' => 'test' }] }
     end
 
-    # reading
+    # property reading
 
     x.report("read string") do
       n.times { b.string }
@@ -62,13 +76,32 @@ begin
       n.times { b.casted_list }
     end
 
-    # db writing
     if ENV['BENCHMARK_DB']
-      x.report("write record") do
-        # need to make change before it will save
-        n.times { b.string = "test#{n}"; b.save }  
+      # db writing
+      x.report("write changed record to db") do
+        db_n.times { |i| b.string = "test#{i}"; b.save }  
       end
+
+      x.report("write unchanged record to db") do
+        db_n.times { b.save }  
+      end
+
+      # db reading
+      x.report("read record from db") do
+        db_n.times { BenchmarkModel.find(b.id) }
+      end
+
     end
 
   end
 end
+
+begin
+  puts "with use_dirty true"
+  set_dirty(true)
+  run_benchmark
+    
+  puts "\nwith use_dirty false"
+  set_dirty(false)
+  run_benchmark
+end
diff --git a/lib/couchrest/model/base.rb b/lib/couchrest/model/base.rb
index 9d47d1b..cb3f3ab 100644
--- a/lib/couchrest/model/base.rb
+++ b/lib/couchrest/model/base.rb
@@ -17,6 +17,7 @@ module CouchRest
       include CouchRest::Model::Associations
       include CouchRest::Model::Validations
       include CouchRest::Model::Dirty
+      include CouchRest::Model::CastedBy
 
       def self.subclasses
         @subclasses ||= []
@@ -25,6 +26,7 @@ module CouchRest
       def self.inherited(subklass)
         super
         subklass.send(:include, CouchRest::Model::Properties)
+
         subklass.class_eval <<-EOS, __FILE__, __LINE__ + 1
           def self.inherited(subklass)
             super
@@ -72,16 +74,9 @@ module CouchRest
       end
       
       ### instance methods
-      
-      # Gets a reference to the actual document in the DB
-      # Calls up to the next document if there is one,
-      # Otherwise we're at the top and we return self
-      def base_doc
-        return self if base_doc?
-        @casted_by.base_doc
-      end
-      
+
       # Checks if we're the top document
+      # (overrides base_doc? in casted_by.rb)
       def base_doc?
         !@casted_by
       end
diff --git a/lib/couchrest/model/casted_by.rb b/lib/couchrest/model/casted_by.rb
new file mode 100644
index 0000000..0f192d6
--- /dev/null
+++ b/lib/couchrest/model/casted_by.rb
@@ -0,0 +1,23 @@
+
+module CouchRest::Model
+  module CastedBy
+    extend ActiveSupport::Concern
+    included do
+      self.send(:attr_accessor, :casted_by)
+    end
+
+    # Gets a reference to the actual document in the DB
+    # Calls up to the next document if there is one,
+    # Otherwise we're at the top and we return self
+    def base_doc
+      return self if base_doc?
+      @casted_by ? @casted_by.base_doc : nil
+    end
+
+    # Checks if we're the top document
+    def base_doc?
+      false
+    end
+
+  end
+end
diff --git a/lib/couchrest/model/casted_hash.rb b/lib/couchrest/model/casted_hash.rb
index 737c91b..e1d56af 100644
--- a/lib/couchrest/model/casted_hash.rb
+++ b/lib/couchrest/model/casted_hash.rb
@@ -7,6 +7,7 @@ module CouchRest::Model
     attr_accessor :casted_by
 
     def []= index, obj
+      return super(index, obj) unless use_dirty?
       couchrest_parent_will_change! if obj != self[index]
       super(index, obj)
     end
diff --git a/lib/couchrest/model/casted_model.rb b/lib/couchrest/model/casted_model.rb
index 19b6fc9..678c8ad 100644
--- a/lib/couchrest/model/casted_model.rb
+++ b/lib/couchrest/model/casted_model.rb
@@ -21,7 +21,7 @@ module CouchRest::Model
     end
     
     def []= key, value
-      couchrest_attribute_will_change!(key) unless self[key] == value
+      couchrest_attribute_will_change!(key) if use_dirty && self[key] != value
       super(key.to_s, value)
     end
     
diff --git a/lib/couchrest/model/configuration.rb b/lib/couchrest/model/configuration.rb
index 765a4b1..25ebfcb 100644
--- a/lib/couchrest/model/configuration.rb
+++ b/lib/couchrest/model/configuration.rb
@@ -10,10 +10,12 @@ module CouchRest
       included do
         add_config :model_type_key
         add_config :mass_assign_any_attribute
-        
+        add_config :use_dirty
+
         configure do |config|
           config.model_type_key = 'couchrest-type' # 'model'?
           config.mass_assign_any_attribute = false
+          config.use_dirty = true
         end
       end
 
diff --git a/lib/couchrest/model/dirty.rb b/lib/couchrest/model/dirty.rb
index 0000fa0..3d422c9 100644
--- a/lib/couchrest/model/dirty.rb
+++ b/lib/couchrest/model/dirty.rb
@@ -10,14 +10,24 @@ module CouchRest
     # This applies to both Model::Base and Model::CastedModel
     module Dirty
       extend ActiveSupport::Concern
+      include CouchRest::Model::CastedBy  # needed for base_doc
+      include ActiveModel::Dirty
+
+      def use_dirty?
+        bdoc = base_doc
+        bdoc && !bdoc.disable_dirty && bdoc.use_dirty
+      end
 
       included do
-        include ActiveModel::Dirty
+        # internal dirty setting - overrides global setting.
+        # this is used to temporarily disable dirty tracking when setting 
+        # attributes directly, for performance reasons.
+        self.send(:attr_accessor, :disable_dirty)
       end
 
       def couchrest_attribute_will_change!(attr)
-        return if attr.nil?
-        self.send("#{attr}_will_change!")
+        return if attr.nil? || !use_dirty?
+        attribute_will_change!(attr)
         couchrest_parent_will_change!
       end
       
@@ -29,7 +39,7 @@ module CouchRest
       
       # return the attribute name this object is referenced by in the parent
       def casted_by_attribute
-        return @casted_by_attribute if @casted_by_attribute_set
+        return @casted_by_attribute if @casted_by_attribute
         attr = @casted_by.attributes
         @casted_by_attribute = attr.keys.detect { |k| attr[k] == self }
       end
diff --git a/lib/couchrest/model/extended_attachments.rb b/lib/couchrest/model/extended_attachments.rb
index c2a5ffc..d36488f 100644
--- a/lib/couchrest/model/extended_attachments.rb
+++ b/lib/couchrest/model/extended_attachments.rb
@@ -2,11 +2,6 @@ module CouchRest
   module Model
     module ExtendedAttachments
       extend ActiveSupport::Concern
-      include ActiveModel::Dirty
-      included do
-        # for _attachments_will_change!
-        define_attribute_methods [:_attachments]
-      end
 
       # Add a file attachment to the current document. Expects
       # :file and :name to be included in the arguments.
@@ -41,8 +36,10 @@ module CouchRest
       # deletes a file attachment from the current doc
       def delete_attachment(attachment_name)
         return unless attachments
-        _attachments_will_change! if attachments.include?(attachment_name)
-        attachments.delete attachment_name
+        if attachments.include?(attachment_name)
+          attribute_will_change!("_attachments")
+          attachments.delete attachment_name
+        end
       end
 
       # returns true if attachment_name exists
@@ -74,7 +71,7 @@ module CouchRest
           content_type = args[:content_type] ? args[:content_type] : get_mime_type(args[:file].path)
           content_type ||= (get_mime_type(args[:name]) || 'text/plain')
 
-          _attachments_will_change!
+          attribute_will_change!("_attachments")
           attachments[args[:name]] = {
             'content_type' => content_type,
             'data'         => args[:file].read
diff --git a/lib/couchrest/model/persistence.rb b/lib/couchrest/model/persistence.rb
index f4c7970..fc1fb4f 100644
--- a/lib/couchrest/model/persistence.rb
+++ b/lib/couchrest/model/persistence.rb
@@ -30,7 +30,7 @@ module CouchRest
       def update(options = {})
         raise "Calling #{self.class.name}#update on document that has not been created!" if self.new?
         return false unless perform_validations(options)
-        return true unless self.changed?
+        return true if use_dirty? && !self.changed?
         _run_update_callbacks do
           _run_save_callbacks do
             result = database.save_doc(self)
diff --git a/lib/couchrest/model/properties.rb b/lib/couchrest/model/properties.rb
index a065092..152db8a 100644
--- a/lib/couchrest/model/properties.rb
+++ b/lib/couchrest/model/properties.rb
@@ -3,7 +3,6 @@ module CouchRest
   module Model
     module Properties
       extend ActiveSupport::Concern
-      include ActiveModel::Dirty
 
       included do
         extlib_inheritable_accessor(:properties) unless self.respond_to?(:properties)
@@ -48,11 +47,14 @@ module CouchRest
       def write_attribute_dirty(property, value)
         prop = find_property!(property)
         value = prop.is_a?(String) ? value : prop.cast(self, value)
-        self.send("#{prop}_will_change!") unless self[prop.to_s] == value
-        write_attribute(property, value)
+        propname = prop.to_s
+        attribute_will_change!(propname) if use_dirty? && self[propname] != value
+        self[propname] = value
       end
 
       def []=(key,value)
+        return super(key,value) unless use_dirty?
+
         has_changes = self.changed?
         if !has_changes && self.respond_to?(:get_unique_id)
           check_id_change = true
@@ -82,14 +84,8 @@ module CouchRest
       end
       alias :attributes= :update_attributes_without_saving
 
-      # needed for Dirty
-      def attributes
-        ret = {}
-        self.class.properties.each do |property|
-          ret[property.name] = read_attribute(property)
-        end
-        ret
-      end
+      # 'attributes' needed for Dirty
+      alias :attributes :properties_with_values
 
       def find_property(property)
         property.is_a?(Property) ? property : self.class.properties.detect {|p| p.to_s == property.to_s}
@@ -123,18 +119,10 @@ module CouchRest
       # Set all the attributes and return a hash with the attributes
       # that have not been accepted.
       def directly_set_attributes(hash, options = {})
-        hash.reject do |attribute_name, attribute_value|
+        self.disable_dirty = !options[:dirty]
+        ret = hash.reject do |attribute_name, attribute_value|
           if self.respond_to?("#{attribute_name}=")
-            if find_property(attribute_name)
-              if options[:dirty]
-                self.write_attribute_dirty(attribute_name, attribute_value)
-              else
-                # set attribute without updating dirty status
-                self.write_attribute(attribute_name, attribute_value)
-              end
-            else
-              self.send("#{attribute_name}=", attribute_value)
-            end
+            self.send("#{attribute_name}=", attribute_value)
             true
           elsif mass_assign_any_attribute # config option
             self[attribute_name] = attribute_value
@@ -143,6 +131,8 @@ module CouchRest
             false
           end
         end
+        self.disable_dirty = false
+        ret
       end
 
       def directly_set_read_only_attributes(hash)
diff --git a/lib/couchrest_model.rb b/lib/couchrest_model.rb
index 4d3e7c6..2a9850d 100644
--- a/lib/couchrest_model.rb
+++ b/lib/couchrest_model.rb
@@ -27,10 +27,11 @@ require 'couchrest/model'
 require 'couchrest/model/errors'
 require "couchrest/model/persistence"
 require "couchrest/model/typecast"
+require "couchrest/model/casted_by"
+require "couchrest/model/dirty"
 require "couchrest/model/property"
 require "couchrest/model/property_protection"
 require "couchrest/model/properties"
-require "couchrest/model/dirty"
 require "couchrest/model/casted_array"
 require "couchrest/model/casted_hash"
 require "couchrest/model/casted_model"
@@ -43,7 +44,7 @@ require "couchrest/model/extended_attachments"
 require "couchrest/model/class_proxy"
 require "couchrest/model/collection"
 require "couchrest/model/associations"
-require "couchrest/model/configuration"
+require 'couchrest/model/configuration'
 
 # Monkey patches applied to couchrest
 require "couchrest/model/support/couchrest"
@@ -52,7 +53,6 @@ require "couchrest/model/support/hash"
 # Base libraries
 require "couchrest/model/casted_model"
 require "couchrest/model/base"
-
 # Add rails support *after* everything has loaded
 
 require "couchrest/railtie"
diff --git a/spec/couchrest/dirty_spec.rb b/spec/couchrest/dirty_spec.rb
index 9184edc..098fa70 100644
--- a/spec/couchrest/dirty_spec.rb
+++ b/spec/couchrest/dirty_spec.rb
@@ -11,7 +11,7 @@ class WithCastedModelMixin < Hash
   property :name
   property :details, Object, :default => {}
   property :casted_attribute, WithCastedModelMixin
-end
+end 
 
 class DummyModel < CouchRest::Model::Base
   use_database TEST_SERVER.default_database
@@ -24,8 +24,72 @@ class DummyModel < CouchRest::Model::Base
   end
 end
 
+# set dirty configuration, return previous configuration setting
+def set_dirty(value)
+  orig = nil
+  CouchRest::Model::Base.configure do |config|
+    orig = config.use_dirty
+    config.use_dirty = value
+  end
+  Card.instance_eval do
+    self.use_dirty = value
+  end
+  orig
+end
 
-describe "Dirty" do
+describe "With use_dirty(off)" do
+
+  before(:all) do
+    @use_dirty_orig = set_dirty(false)
+  end
+
+  # turn dirty back to default
+  after(:all) do
+    set_dirty(@use_dirty_orig)
+  end
+
+  describe "changes" do
+    
+    it "should not respond to the changes method" do
+      @card = Card.new
+      @card.first_name = "andrew"
+      @card.changes.should == {}
+    end
+
+  end
+
+  describe "changed?" do
+
+    it "should not record changes" do
+      @card = Card.new
+      @card.first_name = "andrew"
+      @card.changed?.should be_false
+    end
+  end
+
+  describe "save" do
+    
+    it "should save unchanged records" do
+      @card = Card.create!(:first_name => "matt")
+      @card = Card.find(@card.id)
+      @card.database.should_receive(:save_doc).and_return({"ok" => true})
+      @card.save
+    end
+
+  end
+
+end
+
+describe "With use_dirty(on)" do
+
+  before(:all) do
+    @use_dirty_orig = set_dirty(true)
+  end
+
+  # turn dirty back to default
+  after(:all) do
+    set_dirty(@use_dirty_orig)
+  end
 
   describe "changes" do