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

[eregontp@gmail.com]

Issue #17055 has been updated by Eregon (Benoit Daloze).


I tried on TruffleRuby and there I can't see a significant difference (at least for the noplugin case):
https://gist.github.com/eregon/e552bf55d42ce128a9d89f41d57b637f

TruffleRuby uses inline caches for almost all metaprogramming operations.
And so TruffleRuby optimizes `instance_variable_set :@ivar, value` to be essentially no different than `@ivar = value` when the ivar name is always the same.

So this might mean a good way to speed this case and remove this difference is to have an inline cache for `instance_variable_set` on CRuby too.

Regarding method call overhead, I think this is biased by CRuby without JIT having no inlining.
The method call overhead is something I would expect any Ruby JIT to eventually remove to at least some degree (there is essentially no overhead in TruffleRuby when inlined, the same as if manually inlining the method).
I think MJIT can inline some pure-Ruby methods (not sure if in `master` yet or not).

If you wanted to optimize to the extreme for the CRuby interpreter and no JIT, then one could use tricks like [OptCarrot `--opt`](https://github.com/mame/optcarrot#optimized-mode), for instance in this case to generate a single `#init_with` method with all assignments.
I don't think that's a good idea though, and I hope it shows my point we shouldn't over-optimize for one specific way of running Ruby.

jeremyevans0 (Jeremy Evans) wrote in #note-13:
> > So, for this case, how about using `def foo; @foo ||= nil; end`? That would avoid the warning and still let you assign the `@ivar` lazily, no?
> 
> Then you need a method call instead of an ivar access, which is also slower.  It also would require more changes across the library.

Not necessarily, you could (manually) inline it if that's a concern (but as mentioned above, I think a JIT would easily inline it for you).
It's probably worth a method though if > 1 usage and a non-trivial default value.
I actually found a few of those in Sequel, all related to this benchmark:

```ruby
defined?(@new) ? @new : (@new = false)
@errors ||= errors_class.new
@changed_columns ||= []
@associations ||= {}
```
(these 4 are all in their own method)

Initializing those to `nil` is redundant in the benchmark.

I think the last 3 make a lot of sense to initialize lazily since they involve additional allocations and might never be used.

For the first one, it's too bad that `@new ||= false` will repetitively assign to `false`, but `defined?()` is indeed a workaround for that.

In other words, that approach is already used, works well on existing versions, and has no overhead. It's not very pretty, but this is a micro-optimization. For pretty/readable, eager initialization seems better.

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

* 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/