Permalink
Browse files

Remove NilClass whiners feature.

Removing this feature causes boost in performance when using Ruby 1.9.

Ruby 1.9 started to do implicit conversions using `to_ary` and `to_str`
in some STDLIB methods (like Array#join). To do such implicit conversions,
Ruby 1.9 always dispatches the method and rescues the NoMethodError exception
in case one is raised.

Therefore, since the whiners feature defined NilClass#method_missing, such
implicit conversions for nil became much, much slower. In fact, just defining
NilClass#method_missing (even without the whiners feature) already causes a
massive slow down. Here is a snippet that shows such slow down:

    require "benchmark"
    Benchmark.realtime { 1_000.times { [nil,nil,nil].join } }

    class NilClass
      def method_missing(*args)
        raise NoMethodError
      end
    end

    Benchmark.realtime { 1_000.times { [nil,nil,nil].join } }
  • Loading branch information...
1 parent 7280787 commit d1abf29e796a86cbac315da217f3fe7addb88106 @josevalim josevalim committed Dec 8, 2011
@@ -2222,8 +2222,6 @@ def populate_with_current_scope_attributes
include AutosaveAssociation, NestedAttributes
include Aggregations, Transactions, Reflection, Serialization, Store
- NilClass.add_whiner(self) if NilClass.respond_to?(:add_whiner)
-
# Returns the value of the attribute identified by <tt>attr_name</tt> after it has been typecast (for example,
# "2004-12-12" in a data column is cast to a date object, like Date.new(2004, 12, 12)).
# (Alias for the protected read_attribute method).
@@ -1,21 +1,6 @@
# Extensions to +nil+ which allow for more helpful error messages for people who
# are new to Rails.
#
-# Ruby raises NoMethodError if you invoke a method on an object that does not
-# respond to it:
-#
-# $ ruby -e nil.destroy
-# -e:1: undefined method `destroy' for nil:NilClass (NoMethodError)
-#
-# With these extensions, if the method belongs to the public interface of the
-# classes in NilClass::WHINERS the error message suggests which could be the
-# actual intended class:
-#
-# $ rails runner nil.destroy
-# ...
-# You might have expected an instance of ActiveRecord::Base.
-# ...
-#
# NilClass#id exists in Ruby 1.8 (though it is deprecated). Since +id+ is a fundamental
# method of Active Record models NilClass#id is redefined as well to raise a RuntimeError
# and warn the user. She probably wanted a model database identifier and the 4
@@ -25,36 +10,13 @@
# By default it is on in development and test modes, and it is off in production
# mode.
class NilClass
- METHOD_CLASS_MAP = Hash.new
-
def self.add_whiner(klass)
- methods = klass.public_instance_methods - public_instance_methods
- class_name = klass.name
- methods.each { |method| METHOD_CLASS_MAP[method.to_sym] = class_name }
+ ActiveSupport::Deprecation.warn "NilClass.add_whiner is deprecated and this functionality is " \
+ "removed from Rails versions as it affects Ruby 1.9 performance.", caller
end
- add_whiner ::Array
-
# Raises a RuntimeError when you attempt to call +id+ on +nil+.
def id
raise RuntimeError, "Called id for nil, which would mistakenly be #{object_id} -- if you really wanted the id of nil, use object_id", caller
end
-
- private
- def method_missing(method, *args)
- if klass = METHOD_CLASS_MAP[method]
- raise_nil_warning_for klass, method, caller
- else
- super
- end
- end
-
- # Raises a NoMethodError when you attempt to call a method on +nil+.
- def raise_nil_warning_for(class_name = nil, selector = nil, with_caller = nil)
- message = "You have a nil object when you didn't expect it!"
- message << "\nYou might have expected an instance of #{class_name}." if class_name
- message << "\nThe error occurred while evaluating nil.#{selector}" if selector
-
- raise NoMethodError, message, with_caller || caller
- end
end
@@ -1,54 +1,11 @@
-# Stub to enable testing without Active Record
-module ActiveRecord
- class Base
- def save!
- end
- end
-end
-
require 'abstract_unit'
require 'active_support/whiny_nil'
-NilClass.add_whiner ::ActiveRecord::Base
-
class WhinyNilTest < Test::Unit::TestCase
- def test_unchanged
- nil.method_thats_not_in_whiners
- rescue NoMethodError => nme
- assert_match(/nil:NilClass/, nme.message)
- end
-
- def test_active_record
- nil.save!
- rescue NoMethodError => nme
- assert_no_match(/nil:NilClass/, nme.message)
- assert_match(/nil\.save!/, nme.message)
- end
-
- def test_array
- nil.each
- rescue NoMethodError => nme
- assert_no_match(/nil:NilClass/, nme.message)
- assert_match(/nil\.each/, nme.message)
- end
-
def test_id
nil.id
rescue RuntimeError => nme
assert_no_match(/nil:NilClass/, nme.message)
assert_match(Regexp.new(nil.object_id.to_s), nme.message)
end
-
- def test_no_to_ary_coercion
- nil.to_ary
- rescue NoMethodError => nme
- assert_no_match(/nil:NilClass/, nme.message)
- assert_match(/nil\.to_ary/, nme.message)
- end
-
- def test_no_to_str_coercion
- nil.to_str
- rescue NoMethodError => nme
- assert_match(/nil:NilClass/, nme.message)
- end
end

17 comments on commit d1abf29

@plentz
Contributor
plentz commented on d1abf29 Dec 8, 2011

yey!

@sobrinho
Contributor

Nice!!!

@sobrinho
Contributor

Running your snippet here (1.9.3-p0):

0.002393
0.114014
@hugobarauna

Nice!

@codykrieger

Awesome. Every speed bump helps.

@laserlemon
Contributor

👍 for speed!

@ipoval
ipoval commented on d1abf29 Dec 8, 2011

what about config.whiny_nils option in the rails config files? are you going to remove it in a separate commit?

@carlosantoniodasilva

\o/

@timlinquist
Contributor

I say get rid of all of it. If people want a null object they should implement their own.

@josevalim
Member
@devboy
devboy commented on d1abf29 Dec 8, 2011

Good stuff!

@lucasts
Contributor
lucasts commented on d1abf29 Dec 8, 2011

0.0019240379333496094
0.07682275772094727

@wilson
wilson commented on d1abf29 Dec 8, 2011

Yay!

@tilsammans
Contributor

Amazing. Well done.

@kennyj
Contributor
kennyj commented on d1abf29 Dec 9, 2011

Awesome!

@Alexey-Lukin

Cool

@exviva
Contributor
exviva commented on d1abf29 Dec 9, 2011

Wow!

Before:

real    8m54.220s
user    7m18.483s
sys 0m50.399s

After:

real    7m46.224s
user    6m13.979s
sys 0m44.507s
Please sign in to comment.