[ruby-core:99472] [Ruby master Feature#17055] Allow suppressing uninitialized instance variable and method redefined verbose mode warnings

[merch-redmine@jeremyevans.net]

Issue #17055 has been updated by jeremyevans0 (Jeremy Evans).


tenderlovemaking (Aaron Patterson) wrote in #note-16:
> I guess I need to read the Sequel implementation.  It seems possible to design a system that is no overhead in the no-plugin case that also uses regular instance variables.  In fact it seems like defining one "ClassMethods" module and many "InstanceMethods" modules would do the trick (with no changes to Sequel).

We would have to change the default call method:

```ruby
      # class method
      def call(values)
        o = allocate
        o.instance_variable_set(:@values, values)
        o
      end
```

to something like

```ruby
      # class method
      def call(values)
        o = allocate
        o.send(:initialize_from_database, values)
        o
      end

      # instance method
      def initialize_from_database(values)
        @values = values
      end
```

I believe such an approach will always be slower, as `send(:method_name)` and setting the instance variable inside the method will slower than a call to `instance_variable_set`.

> These benchmarks were close enough that it made me wonder if setting instance variables was even the bottleneck of the program, so I deleted all instance variables from the program but left the method calls [here](https://github.com/tenderlove/sequel-benchmark/blob/methods-but-no-ivars/t.rb).

I agree here, the method calling overhead is more than the ivar setting overhead.

> btw, `super` didn't use an inline method cache in < 2.8, so the current development branch is much faster for the above case:

That's great to hear.  My libraries tend to rely heavily on module inclusion, and defining methods that call super, so that should provide a nice performance improvement.

> I'm not sure if I did the benchmarks 100% correctly, so I would appreciate a review.  The "regular ivars is slower" result makes me think I did something wrong.

I looked at your benchmark for using instance variables directly in instance methods and it looks fine to me.  If there is a bug with it, I couldn't spot it (other than init_values is public and should be private, but using send would probably decrease performance).  I'm also surprised it is slower than the `instance_variable_set` approach.  However, the two code paths are different internally, and you are running in verbose mode.  I'm not sure whether you get different results if you are not in verbose mode.

----------------------------------------
Feature #17055: Allow suppressing uninitialized instance variable and method redefined verbose mode warnings
https://bugs.ruby-lang.org/issues/17055#change-86928

* Author: jeremyevans0 (Jeremy Evans)
* Status: Open
* Priority: Normal
----------------------------------------
These two verbose mode warnings are both fairly common and have good reasons why you would not want to warn about them in specific cases.  Not initializing instance variables to nil can be much better for performance, and redefining methods without removing the method first is the only safe approach in multi-threaded code.

There are reasons that you may want to issue verbose warnings by default in these cases.  For uninitialized instance variables, it helps catch typos. For method redefinition, it could alert you that a method already exists when you didn't expect it to, such as when a file is loaded multiple times when it should only be loaded once.

I propose we keep the default behavior the same, but offer the ability to opt-out of these warnings by defining methods.  For uninitialized instance variables in verbose mode, I propose we call `expected_uninitialized_instance_variable?(iv)` on the object.  If this method doesn't exist or returns false/nil, we issue the warning.  If the method exists and returns true, we suppress the warning.  Similarly, for redefined methods, we call `expected_redefined_method?(method_name)` on the class or module.  If the method doesn't exist or returns false/nil, we issue the warning.  If the method exists and returns true, we suppress the warning.

This approach allows high performance code (uninitialized instance variables) and safe code (redefining methods without removing) to work without verbose mode warnings.

I have implemented this support in a pull request: https://github.com/ruby/ruby/pull/3371

---Files--------------------------------
t.rb (5.59 KB)


-- 
https://bugs.ruby-lang.org/