Candidate fix for #258

benchmarks/find_original_method_early.rb

@@ -0,0 +1,65 @@
+$LOAD_PATH.unshift "./lib"
+require 'rspec/mocks'
+require "rspec/mocks/standalone"
+
+=begin
+This benchmark script is for troubleshooting the performance of
+#264. To use it, you have to edit the code in #264 a bit:
+wrap the call in `MethodDouble#initialize` to `find_original_method`
+in a conditional like `if $find_original`.
+
+That allows the code below to compare the perf of stubbing a method
+with the original method being found vs. not.
+=end
+
+require 'benchmark'
+
+n = 10000
+
+Foo = Class.new(Object) do
+  n.times do |i|
+    define_method "meth_#{i}" do
+    end
+  end
+end
+
+Benchmark.bmbm do |bm|
+  puts "#{n} times - ruby #{RUBY_VERSION}"
+
+  perform_report = lambda do |label, find_original, &create_object|
+    dbl = create_object.call
+    $find_original = find_original
+
+    bm.report(label) do
+      n.times do |i|
+        dbl.stub("meth_#{i}")
+      end
+    end
+
+    RSpec::Mocks.space.reset_all
+  end
+
+  perform_report.call("Find original - partial mock", true) { Foo.new }
+  perform_report.call("Don't find original - partial mock", false) { Foo.new }
+  perform_report.call("Find original - test double", true) { double }
+  perform_report.call("Don't find original - test double", false) { double }
+end
+
+=begin
+
+10000 times - ruby 1.9.3
+Rehearsal ----------------------------------------------------------------------
+Don't find original - partial mock   1.050000   0.020000   1.070000 (  1.068561)
+Don't find original - test double    1.190000   0.010000   1.200000 (  1.199815)
+Find original - partial mock         1.270000   0.010000   1.280000 (  1.282944)
+Find original - test double          1.320000   0.020000   1.340000 (  1.336136)
+------------------------------------------------------------- total: 4.890000sec
+
+                                         user     system      total        real
+Don't find original - partial mock   0.990000   0.000000   0.990000 (  1.000959)
+Don't find original - test double    0.930000   0.010000   0.940000 (  0.931871)
+Find original - partial mock         1.040000   0.000000   1.040000 (  1.046354)
+Find original - test double          0.980000   0.010000   0.990000 (  0.983577)
+
+=end
+

lib/rspec/mocks/instance_method_stasher.rb

@@ -2,24 +2,25 @@ module RSpec
   module Mocks
     # @private
     class InstanceMethodStasher
-      def initialize(klass, method)
-        @klass = klass
+      attr_reader :original_method
+
+      def initialize(object, method)
+        @object = object
         @method = method
+        @klass = (class << object; self; end)
 
-        @method_is_stashed = false
+        @original_method = nil
       end
 
       # @private
       def method_is_stashed?
-        @method_is_stashed
+        !!@original_method
       end
 
       # @private
       def stash
-        return if !method_defined_directly_on_klass? || @method_is_stashed
-
-        @klass.__send__(:alias_method, stashed_method_name, @method)
-        @method_is_stashed = true
+        return if !method_defined_directly_on_klass?
+        @original_method ||= ::RSpec::Mocks.method_handle_for(@object, @method)
       end
 
       private
@@ -61,21 +62,16 @@ def method_owned_by_klass?
 
       public
 
-      # @private
-      def stashed_method_name
-        "obfuscated_by_rspec_mocks__#{@method}"
-      end
-
       # @private
       def restore
-        return unless @method_is_stashed
+        return unless @original_method
 
         if @klass.__send__(:method_defined?, @method)
           @klass.__send__(:undef_method, @method)
         end
-        @klass.__send__(:alias_method, @method, stashed_method_name)
-        @klass.__send__(:remove_method, stashed_method_name)
-        @method_is_stashed = false
+
+        @klass.__send__(:define_method, @method, &@original_method)
+        @original_method = nil
       end
     end
   end

lib/rspec/mocks/method_double.rb

@@ -11,12 +11,18 @@ def initialize(object, method_name, proxy)
         @object = object
         @proxy = proxy
 
-        @method_stasher = InstanceMethodStasher.new(object_singleton_class, @method_name)
+        @method_stasher = InstanceMethodStasher.new(object, method_name)
         @method_is_proxied = false
         @expectations = []
         @stubs = []
       end
 
+      def original_method
+        @original_method ||= Proc.new do |*args, &block|
+          @object.__send__(:method_missing, @method_name, *args, &block)
+        end
+      end
+
       # @private
       def visibility
         if TestDouble === @object
@@ -30,119 +36,6 @@ def visibility
         end
       end
 
-      class ProcWithBlock < Struct.new(:object, :method_name)
-
-        def call(*args, &block)
-          self.object.__send__(:method_missing, self.method_name, *args, &block)
-        end
-
-      end
-
-      # @private
-      def original_method
-        if @method_stasher.method_is_stashed?
-          # Example: a singleton method defined on @object
-          ::RSpec::Mocks.method_handle_for(@object, @method_stasher.stashed_method_name)
-        elsif meth = original_unrecorded_any_instance_method
-          # Example: a method that has been mocked through
-          #   klass.any_instance.should_receive(:msg).and_call_original
-          # any_instance.should_receive(:msg) causes the method to be
-          # replaced with a proxy method, and then `and_call_original`
-          # is recorded and played back on the object instance. We need
-          # special handling here to get a handle on the original method
-          # object rather than the proxy method.
-          meth
-        else
-          # Example: an instance method defined on one of @object's ancestors.
-          original_method_from_ancestry
-        end
-      rescue NameError
-        # We have no way of knowing if the object's method_missing
-        # will handle this message or not...but we can at least try.
-        # If it's not handled, a `NoMethodError` will be raised, just
-        # like normally.
-        ProcWithBlock.new(@object,@method_name)
-      end
-
-      def original_unrecorded_any_instance_method
-        return nil unless any_instance_class_recorder_observing_method?(@object.class)
-        alias_name = ::RSpec::Mocks.any_instance_recorder_for(@object.class).build_alias_method_name(@method_name)
-        @object.method(alias_name)
-      end
-
-      def any_instance_class_recorder_observing_method?(klass)
-        return true if ::RSpec::Mocks.any_instance_recorder_for(klass).already_observing?(@method_name)
-        superklass = klass.superclass
-        return false if superklass.nil?
-        any_instance_class_recorder_observing_method?(superklass)
-      end
-
-      our_singleton_class = class << self; self; end
-      if our_singleton_class.ancestors.include? our_singleton_class
-        # In Ruby 2.1, ancestors include the correct ancestors, including the singleton classes
-        def original_method_from_ancestry
-          # Lookup in the ancestry, skipping over the singleton class itself
-          original_method_from_ancestor(object_singleton_class.ancestors.drop(1))
-        end
-      else
-        # @private
-        def original_method_from_ancestry
-          original_method_from_ancestor(object_singleton_class.ancestors)
-        rescue NameError
-          raise unless @object.respond_to?(:superclass)
-
-          # Example: a singleton method defined on @object's superclass.
-          #
-          # Note: we have to give precedence to instance methods
-          # defined on @object's class, because in a case like:
-          #
-          # `klass.should_receive(:new).and_call_original`
-          #
-          # ...we want `Class#new` bound to `klass` (which will return
-          # an instance of `klass`), not `klass.superclass.new` (which
-          # would return an instance of `klass.superclass`).
-          original_method_from_superclass
-        end
-      end
-
-      def original_method_from_ancestor(ancestors)
-        klass, *rest = ancestors
-        klass.instance_method(@method_name).bind(@object)
-      rescue NameError
-        raise if rest.empty?
-        original_method_from_ancestor(rest)
-      end
-
-      if RUBY_VERSION.to_f > 1.8
-        # @private
-        def original_method_from_superclass
-          @object.superclass.
-                  singleton_class.
-                  instance_method(@method_name).
-                  bind(@object)
-        end
-      else
-        # Our implementation for 1.9 (above) causes an error on 1.8:
-        # TypeError: singleton method bound for a different object
-        #
-        # This doesn't work quite right in all circumstances but it's the
-        # best we can do.
-        # @private
-        def original_method_from_superclass
-          ::Kernel.warn <<-WARNING.gsub(/^ +\|/, '')
-            |
-            |WARNING: On ruby 1.8, rspec-mocks is unable to bind the original
-            |`#{@method_name}` method to your partial mock object (#{@object})
-            |for `and_call_original`. The superclass's `#{@method_name}` is being
-            |used instead; however, it may not work correctly when executed due
-            |to the fact that `self` will be #{@object.superclass}, not #{@object}.
-            |
-            |Called from: #{CallerFilter.first_non_rspec_line}
-          WARNING
-
-          @object.superclass.method(@method_name)
-        end
-      end
       # @private
       def object_singleton_class
         class << @object; self; end
@@ -159,12 +52,16 @@ def configure_method
       def define_proxy_method
         return if @method_is_proxied
 
+        @original_method = @method_stasher.original_method ||
+                           @proxy.method_handle_for(method_name)
+
         object_singleton_class.class_exec(method_name, visibility) do |method_name, visibility|
           define_method(method_name) do |*args, &block|
             ::RSpec::Mocks.proxy_for(self).message_received method_name, *args, &block
           end
           self.__send__ visibility, method_name
         end
+
         @method_is_proxied = true
       end
 

lib/rspec/mocks/proxy.rb

@@ -28,6 +28,11 @@ def as_null_object
         @object
       end
 
+      # @private
+      def method_handle_for(message)
+        nil
+      end
+
       # @private
       def already_proxied_respond_to
         @already_proxied_respond_to = true
@@ -220,5 +225,28 @@ def find_almost_matching_stub(method_name, *args)
         method_double[method_name].stubs.find {|stub| stub.matches_name_but_not_args(method_name, *args)}
       end
     end
+
+    class PartialMockProxy < Proxy
+      def method_handle_for(message)
+        if any_instance_class_recorder_observing_method?(@object.class, message)
+          message = ::RSpec::Mocks.
+            any_instance_recorder_for(@object.class).
+            build_alias_method_name(message)
+        end
+
+        ::RSpec::Mocks.method_handle_for(@object, message)
+      rescue NameError
+        nil
+      end
+
+    private
+
+      def any_instance_class_recorder_observing_method?(klass, method_name)
+        return true if ::RSpec::Mocks.any_instance_recorder_for(klass).already_observing?(method_name)
+        superklass = klass.superclass
+        return false if superklass.nil?
+        any_instance_class_recorder_observing_method?(superklass, method_name)
+      end
+    end
   end
 end

lib/rspec/mocks/space.rb

@@ -53,7 +53,7 @@ def proxy_for(object)
                         when NilClass   then ProxyForNil.new
                         when TestDouble then object.__build_mock_proxy
                         else
-                          Proxy.new(object)
+                          PartialMockProxy.new(object)
                         end
         end
       end