class_attribute: reduce reliance on define_method

Full benchmark: https://gist.github.com/casperisfine/539e310eadd0cb6
Followup: rails#37903

Instead of always redefining a bmethod on assignment,
if the reciever already has its own bmethod, we just
update the local variable.

This wasn't really considered before because class attributes
are mostly meant for configuration, so not really expected
to be changed at runtime, but it can happen.

This is 10 to 17x faster to assign a class attributes:

```
interpreter:
                 old:  1834149.6 i/s
                 new: 17722346.6 i/s - 9.66x  faster
yjit:
                 old:  2561173.8 i/s
                 new: 43004422.5 i/s - 16.79x  faster
```

The difference is even bigger if something is redefining
`method_added` events, e.g. sorbet...

All other cases are unchanged in performance, except definition
that is marginally faster.

One downside however is that generated accessors source location
will no longer point to the `class_attribute` callsite.

Author: byroot

activerecord/test/cases/test_case.rb

@@ -141,9 +141,6 @@ def with_has_many_inversing(model = ActiveRecord::Base)
       yield
     ensure
       model.has_many_inversing = old
-      if model != ActiveRecord::Base && !old
-        model.singleton_class.remove_method(:has_many_inversing) # reset the class_attribute
-      end
     end
 
     def with_automatic_scope_inversing(*reflections)

activesupport/lib/active_support.rb

@@ -62,6 +62,7 @@ module ActiveSupport
     autoload :Cache
     autoload :Callbacks
     autoload :Configurable
+    autoload :ClassAttribute
     autoload :Deprecation
     autoload :Delegation
     autoload :Digest

activesupport/lib/active_support/callbacks.rb

@@ -66,7 +66,7 @@ module Callbacks
 
     included do
       extend ActiveSupport::DescendantsTracker
-      class_attribute :__callbacks, instance_writer: false, default: {}
+      class_attribute :__callbacks, instance_writer: false, instance_predicate: false, default: {}
     end
 
     CALLBACK_FILTER_TYPES = [:before, :after, :around].freeze

activesupport/lib/active_support/class_attribute.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module ActiveSupport
+  module ClassAttribute # :nodoc:
+    class << self
+      def redefine(owner, name, value)
+        if owner.singleton_class?
+          owner.redefine_method(name) { value }
+          owner.send(:public, name)
+        end
+
+        owner.redefine_singleton_method(name) { value }
+        owner.singleton_class.send(:public, name)
+
+        owner.redefine_singleton_method("#{name}=") do |new_value|
+          if owner.equal?(self)
+            value = new_value
+          else
+            ::ActiveSupport::ClassAttribute.redefine(self, name, new_value)
+          end
+        end
+        owner.singleton_class.send(:public, "#{name}=")
+      end
+    end
+  end
+end

activesupport/lib/active_support/core_ext/class/attribute.rb

@@ -91,24 +91,16 @@ def class_attribute(*attrs, instance_accessor: true,
         raise TypeError, "#{name.inspect} is not a symbol nor a string"
       end
 
-      class_methods << <<~RUBY # In case the method exists and is not public
-        silence_redefinition_of_method def #{name}
-        end
-      RUBY
-
-      methods << <<~RUBY if instance_reader
-        silence_redefinition_of_method def #{name}
-          defined?(@#{name}) ? @#{name} : self.class.#{name}
-        end
-      RUBY
+      name = name.to_sym
+      ::ActiveSupport::ClassAttribute.redefine(self, name, default)
 
-      class_methods << <<~RUBY
-        silence_redefinition_of_method def #{name}=(value)
-          redefine_method(:#{name}) { value } if singleton_class?
-          redefine_singleton_method(:#{name}) { value }
-          value
-        end
-      RUBY
+      unless singleton_class?
+        methods << <<~RUBY if instance_reader
+          silence_redefinition_of_method def #{name}
+            defined?(@#{name}) ? @#{name} : self.class.#{name}
+          end
+        RUBY
+      end
 
       methods << <<~RUBY if instance_writer
         silence_redefinition_of_method(:#{name}=)
@@ -125,7 +117,5 @@ def class_attribute(*attrs, instance_accessor: true,
 
     location = caller_locations(1, 1).first
     class_eval(["class << self", *class_methods, "end", *methods].join(";").tr("\n", ";"), location.path, location.lineno)
-
-    attrs.each { |name| public_send("#{name}=", default) }
   end
 end

activesupport/test/core_ext/class/attribute_test.rb

@@ -8,6 +8,7 @@ def setup
     @klass = Class.new do
       class_attribute :setting
       class_attribute :timeout, default: 5
+      class_attribute :system # private kernel method
     end
 
     @sub = Class.new(@klass)
@@ -30,11 +31,14 @@ def setup
   test "overridable" do
     @sub.setting = 1
     assert_nil @klass.setting
+    assert_equal 1, @sub.setting
 
     @klass.setting = 2
+    assert_equal 2, @klass.setting
     assert_equal 1, @sub.setting
 
     assert_equal 1, Class.new(@sub).setting
+    assert_equal 2, Class.new(@klass).setting
   end
 
   test "predicate method" do
@@ -97,8 +101,34 @@ def setup
 
   test "works well with singleton classes" do
     object = @klass.new
+
     object.singleton_class.setting = "foo"
     assert_equal "foo", object.setting
+    assert_nil @klass.setting
+
+    object.singleton_class.setting = "bar"
+    assert_equal "bar", object.setting
+    assert_nil @klass.setting
+
+    @klass.setting = "plop"
+    assert_equal "bar", object.setting
+    assert_equal "plop", @klass.setting
+  end
+
+  test "when defined in a class's singleton" do
+    @klass = Class.new do
+      class << self
+        class_attribute :__callbacks, default: 1
+      end
+    end
+
+    assert_equal 1, @klass.__callbacks
+    assert_equal 1, @klass.singleton_class.__callbacks
+
+    # I honestly think this is a bug, but that's how it used to behave
+    @klass.__callbacks = 4
+    assert_equal 1, @klass.__callbacks
+    assert_equal 1, @klass.singleton_class.__callbacks
   end
 
   test "works well with module singleton classes" do
@@ -115,4 +145,16 @@ class << self
     val = @klass.public_send(:setting=, 1)
     assert_equal 1, val
   end
+
+  test "works when overriding private methods from an ancestor" do
+    assert_nil @klass.system
+    @klass.system = 1
+    assert_equal 1, @klass.system
+
+    instance = @klass.new
+    assert_equal 1, instance.system
+    assert_predicate @klass.new, :system?
+    instance.system = 2
+    assert_equal 2, instance.system
+  end
 end