Improve performance by avoiding named block arguments. Closes #11109 …

…[adymo]

Reapplies [8865] with some fixes


git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@8957 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Perf fix: Avoid the use of named block arguments.  Closes #11109 [adymo]
+
 * PostgreSQL: support server versions 7.4 through 8.0 and the ruby-pg driver.  #11127 [jdavis]
 
 * Ensure association preloading doesn't break when an association returns nil. ##11145 [GMFlash]

activerecord/lib/active_record/associations/association_collection.rb

@@ -43,8 +43,12 @@ def delete_all
       end
 
       # Calculate sum using SQL, not Enumerable
-      def sum(*args, &block)
-        calculate(:sum, *args, &block)
+      def sum(*args)
+        if block_given?
+          calculate(:sum, *args) { |*block_args| yield(*block_args) }
+        else
+          calculate(:sum, *args)
+        end
       end
 
       # Remove +records+ from this association.  Does not destroy +records+.
@@ -121,9 +125,9 @@ def empty?
         size.zero?
       end
 
-      def any?(&block)
+      def any?
         if block_given?
-          method_missing(:any?, &block)
+          method_missing(:any?) { |*block_args| yield(*block_args) }
         else
           !empty?
         end
@@ -157,11 +161,21 @@ def replace(other_array)
 
 
       protected
-        def method_missing(method, *args, &block)
+        def method_missing(method, *args)
           if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method))
-            super
+            if block_given?
+              super { |*block_args| yield(*block_args) }
+            else
+              super
+            end
           else
-            @reflection.klass.send(:with_scope, construct_scope) { @reflection.klass.send(method, *args, &block) }
+            @reflection.klass.send(:with_scope, construct_scope) do
+              if block_given?
+                @reflection.klass.send(method, *args) { |*block_args| yield(*block_args) }
+              else
+                @reflection.klass.send(method, *args)
+              end
+            end
           end
         end
 
@@ -187,15 +201,23 @@ def find_target
 
       private
 
-        def create_record(attrs, &block)
+        def create_record(attrs)
           ensure_owner_is_not_new
           record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) { @reflection.klass.new(attrs) }
-          add_record_to_target_with_callbacks(record, &block)
+          if block_given?
+            add_record_to_target_with_callbacks(record) { |*block_args| yield(*block_args) }
+          else
+            add_record_to_target_with_callbacks(record)
+          end
         end
 
-        def build_record(attrs, &block)
+        def build_record(attrs)
           record = @reflection.klass.new(attrs)
-          add_record_to_target_with_callbacks(record, &block)
+          if block_given?
+            add_record_to_target_with_callbacks(record) { |*block_args| yield(*block_args) }
+          else
+            add_record_to_target_with_callbacks(record)
+          end
         end
 
         def add_record_to_target_with_callbacks(record)

activerecord/lib/active_record/associations/association_proxy.rb

@@ -120,9 +120,13 @@ def merge_options_from_reflection!(options)
         end
 
       private
-        def method_missing(method, *args, &block)
+        def method_missing(method, *args)
           if load_target
-            @target.send(method, *args, &block)
+            if block_given?
+              @target.send(method, *args)  { |*block_args| yield(*block_args) }
+            else
+              @target.send(method, *args)
+            end
           end
         end
 

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -113,8 +113,12 @@ def size
       end
 
       # Calculate sum using SQL, not Enumerable
-      def sum(*args, &block)
-        calculate(:sum, *args, &block)
+      def sum(*args)
+        if block_given?
+          calculate(:sum, *args) { |*block_args| yield(*block_args) }
+        else
+          calculate(:sum, *args)
+        end
       end
 
       def count(*args)
@@ -128,11 +132,21 @@ def count(*args)
       end
 
       protected
-        def method_missing(method, *args, &block)
+        def method_missing(method, *args)
           if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method))
-            super
+            if block_given?
+              super { |*block_args| yield(*block_args) }
+            else
+              super
+            end
           else
-            @reflection.klass.send(:with_scope, construct_scope) { @reflection.klass.send(method, *args, &block) }
+            @reflection.klass.send(:with_scope, construct_scope) do
+              if block_given?
+                @reflection.klass.send(method, *args) { |*block_args| yield(*block_args) }
+              else
+                @reflection.klass.send(method, *args)
+              end
+            end
           end
         end
 

activerecord/test/cases/associations/join_model_test.rb

@@ -290,6 +290,11 @@ def test_has_many_class_methods_called_by_method_missing
     assert_equal nil, authors(:david).categories.find_by_name('Technology')
   end
 
+  def test_has_many_array_methods_called_by_method_missing
+    assert true, authors(:david).categories.any? { |category| category.name == 'General' }
+    assert_nothing_raised { authors(:david).categories.sort }
+  end
+
   def test_has_many_going_through_join_model_with_custom_foreign_key
     assert_equal [], posts(:thinking).authors
     assert_equal [authors(:mary)], posts(:authorless).authors